Skip to content
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

Cumulative ExponentialHistogram resets start time each collection #3931

Closed
aabmass opened this issue Jun 20, 2023 · 2 comments · Fixed by #3934
Closed

Cumulative ExponentialHistogram resets start time each collection #3931

aabmass opened this issue Jun 20, 2023 · 2 comments · Fixed by #3934
Assignees
Labels
bug Something isn't working priority:p2 Bugs and spec inconsistencies which cause telemetry to be incomplete or incorrect

Comments

@aabmass
Copy link
Member

aabmass commented Jun 20, 2023

What happened?

Cumulative ExponentialHistogram points have their startTime reset each collection. E.g.

// current incorrect behavior

t0-------t1                   first collection
         t1-------t2          second collection
--------------------------> t

// instead of correct behavior

t0-------t1                    first collection
t0-----------------t2          second collection
--------------------------> t

Steps to Reproduce

#3930

Expected Result

The startTime should be the same each MetricReader collection for the same metric. The tests in linked PR draft should pass.

Actual Result

The startTime resets each collection (like a delta temporality) while the value continues to grow cumulatively. The linked PR test fails.

Additional Details

Looks like the merge function is not merging the start times: https://github.com/open-telemetry/opentelemetry-js/blob/74393ac6397dcb130b9d5dae35354a2bf9a0d2c3/packages/sdk-metrics/src/aggregator/ExponentialHistogram.ts#LL219C33-L219C33

OpenTelemetry Setup Code

No response

package.json

No response

Relevant log output

No response

@aabmass aabmass added bug Something isn't working triage labels Jun 20, 2023
@aabmass
Copy link
Member Author

aabmass commented Jun 21, 2023

I'm happy to submit a PR for this btw, I think i already found the fix.

@pichlermarc pichlermarc added priority:p2 Bugs and spec inconsistencies which cause telemetry to be incomplete or incorrect and removed triage labels Jun 21, 2023
@pichlermarc
Copy link
Member

Hi @aabmass, thanks for reporting this.
A PR to fix this would be very welcome 🙂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working priority:p2 Bugs and spec inconsistencies which cause telemetry to be incomplete or incorrect
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants