[Connector/Servicegraph] use span time instead of flush time

[alburthoffman]

Component(s)

connector/servicegraph

What happened?

Description

When generating metrics, the connector uses flush time at present https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/v0.123.0/connector/servicegraphconnector/connector.go#L496 . But normally the metrics were generated buffered spans and it would take sometime to complete the edges.

Steps to Reproduce

https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/v0.123.0/connector/servicegraphconnector/connector.go#L496

Expected Result

use span flush time

Actual Result

Collector version

v1.230

Environment information

Environment

OS: (e.g., "Ubuntu 20.04")
Compiler(if manually compiled): (e.g., "go 14.2")

OpenTelemetry Collector configuration

Log output

Additional context

No response

[github-actions]

Pinging code owners:

• connector/servicegraph: @mapno @JaredTan95

See Adding Labels via Comments if you do not have permissions to add labels yourself.

[mapno]

Can you describe a bit more how this would work with the metric itself?

Metrics are collected on a ticker, so in each collection there are multiple batches of spans/traces that are being processed. It's not possible to retroactively change the value of a past data point. If a trace arrives with 10min delay, it's not possible to change the value of the metric from 10min ago.

Something that other implementations do is to filter out traces based on their timestamp, but it's a trade-off.

[bacherfl]

(Triage): Removing needs-triage as code owner already responded. Adding wait-for-author since more information is requested

[alburthoffman]

what we tried to solve is our component sends spans (like 10M ago) as a batch to service graph connector. And then the connector generates the metrics with flush time. Then the metrics actually shows the status of 10m ago.

This is misleading. I'm thinking it is also good to allow a offset value for the timestamp.

Let's say if setting the offset to 10m, then the metrics will have timestamp as (flushtime-10m), then this can also help.

[mapno]

Using an offset sounds more reasonable 👍

[alburthoffman]

thx, will raise a PR for this.

[alburthoffman]

please review the PR. thx