Skip to content

[connector/spanmetrics] Incorrect starttimestamps for subspans. #35994

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
shivanthzen opened this issue Oct 25, 2024 · 7 comments · Fixed by #36718
Closed

[connector/spanmetrics] Incorrect starttimestamps for subspans. #35994

shivanthzen opened this issue Oct 25, 2024 · 7 comments · Fixed by #36718
Assignees
Labels
bug Something isn't working connector/spanmetrics

Comments

@shivanthzen
Copy link
Contributor

shivanthzen commented Oct 25, 2024

Component(s)

connector/spanmetrics

What happened?

Description

Currently while generating metrics out of traces, the start timestamp of a metric is currently tied to the root span. When a "new" subspan appears for trace (eg : unhappy path on an api call which results in a new subspan), the time starttimestamp for the new metric for the new subspan is that of the root span(which can be well in the past).

Steps to Reproduce

  1. Setup otelcol with traces and spanmetricsconnector to generate metrics.
  2. Generate periodic traces to the collector with consistent spans for a long duration.
  3. Generate a trace that contains a "new" span for the same parent span.

We store start-timestamp at resource level rawmetrics in the spanconnector. This starttimestamp is used regardless of when the "new" subspan appeared.

Expected Result

The metric generated for the new span has a timestamp of the first occurance of the "new" span.

Actual Result

The metric generated for the new span has the time stamp corresponding start of the periodic trace

Collector version

v0.112.0

Environment information

Environment

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

OpenTelemetry Collector configuration

No response

Log output

No response

Additional context

No response

@shivanthzen shivanthzen added bug Something isn't working needs triage New item requiring triage labels Oct 25, 2024
@shivanthzen shivanthzen changed the title SpanmetricsConnector: Incorrect starttimestamps for subspans. [connector/spanmetrics]: Incorrect starttimestamps for subspans. Oct 25, 2024
@shivanthzen shivanthzen changed the title [connector/spanmetrics]: Incorrect starttimestamps for subspans. [connector/spanmetrics] Incorrect starttimestamps for subspans. Oct 25, 2024
@shivanthzen
Copy link
Contributor Author

cc: @portertech @Frapschen

@shivanthzen
Copy link
Contributor Author

Reopening

@shivanthzen shivanthzen reopened this Oct 29, 2024
shivanthzen added a commit to shivanthzen/opentelemetry-collector-contrib that referenced this issue Nov 4, 2024
… of a metric is currently tied to the root span. When a "new" child span appears for trace (eg : unhappy path on an api call which results in a new subspan or an async process that was triggered much later), the time starttimestamp for the new metric for the new child span is that of the parent span(which can be well in the past).

This MR moves the start timestamp from resource level (root span) to metric level(child span)  . (Doesn't consider delta-temporality as of now).
References: open-telemetry#35994
Upstream Fix: open-telemetry#36019
shivanthzen added a commit to shivanthzen/opentelemetry-collector-contrib that referenced this issue Nov 4, 2024
Currently while generating metrics out of traces, the start timestamp of a metric is currently tied to the root span. When a "new" child span  appears for trace (eg : unhappy path on an api call which results in a new subspan or an async process that was triggered much later), the time starttimestamp for the new metric for the new child span  is that of the parent span(which can be well in the past).
This MR moves the start timestamp from resource level (root span) to metric level(child span)  . (Doesn't consider delta-temporality as of now).
References: open-telemetry#35994
Upstream Fix: open-telemetry#36019
@shivanthzen
Copy link
Contributor Author

shivanthzen commented Nov 4, 2024

Ping @Frapschen @portertech
The POC fix has been validated on production.

Copy link
Contributor

github-actions bot commented Jan 6, 2025

This issue has been inactive for 60 days. It will be closed in 60 days if there is no activity. To ping code owners by adding a component label, see Adding Labels via Comments, or if you are unsure of which component this issue relates to, please ping @open-telemetry/collector-contrib-triagers. If this issue is still relevant, please ping the code owners or leave a comment explaining why it is still relevant. Otherwise, please close it.

@github-actions github-actions bot added the Stale label Jan 6, 2025
@shivanthzen
Copy link
Contributor Author

@open-telemetry/collector-contrib-triagers

@portertech
Copy link
Contributor

Hey @shivanthzen, thanks for the issue, and sorry for the radio silence. I'm pulling your PR to give it a go.

Copy link
Contributor

Pinging code owners for connector/spanmetrics: @portertech @Frapschen. See Adding Labels via Comments if you do not have permissions to add labels yourself. For example, comment '/label priority:p2 -needs-triaged' to set the priority and remove the needs-triaged label.

@atoulme atoulme removed the needs triage New item requiring triage label Mar 8, 2025
akshays-19 pushed a commit to akshays-19/opentelemetry-collector-contrib that referenced this issue Apr 23, 2025
…ry#36718)

<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue.
Ex. Adding a feature - Explain what this achieves.-->
#### Description
Currently, metric start timestamps are associated with the parent span
(resource level). This means that all child spans, even those that occur
asynchronously or infrequently, inherit the same start timestamp. This
can lead to inaccurate data.

This merge request proposes moving the start timestamp (and last seen
timestamp) from the parent span level to the individual child span
(metric) level. This will ensure that each metric has its own accurate
start and last seen timestamps, regardless of its relationship to other
spans.


<!-- Issue number (e.g. open-telemetry#1234) or full URL to issue, if applicable. -->
#### Link to tracking issue
Fixes
open-telemetry#35994

<!--Describe what testing was performed and which tests were added.-->
#### Testing

<!--Describe the documentation added.-->
#### Documentation

<!--Please delete paragraphs that you did not use before submitting.-->

---------

Co-authored-by: Antoine Toulme <[email protected]>
Co-authored-by: Murphy Chen <[email protected]>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: opentelemetrybot <[email protected]>
Co-authored-by: Yang Song <[email protected]>
Fiery-Fenix pushed a commit to Fiery-Fenix/opentelemetry-collector-contrib that referenced this issue Apr 24, 2025
…ry#36718)

<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue.
Ex. Adding a feature - Explain what this achieves.-->
#### Description
Currently, metric start timestamps are associated with the parent span
(resource level). This means that all child spans, even those that occur
asynchronously or infrequently, inherit the same start timestamp. This
can lead to inaccurate data.

This merge request proposes moving the start timestamp (and last seen
timestamp) from the parent span level to the individual child span
(metric) level. This will ensure that each metric has its own accurate
start and last seen timestamps, regardless of its relationship to other
spans.


<!-- Issue number (e.g. open-telemetry#1234) or full URL to issue, if applicable. -->
#### Link to tracking issue
Fixes
open-telemetry#35994

<!--Describe what testing was performed and which tests were added.-->
#### Testing

<!--Describe the documentation added.-->
#### Documentation

<!--Please delete paragraphs that you did not use before submitting.-->

---------

Co-authored-by: Antoine Toulme <[email protected]>
Co-authored-by: Murphy Chen <[email protected]>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: opentelemetrybot <[email protected]>
Co-authored-by: Yang Song <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment