Skip to content

prometheusreceiver: Append.*CTZeroSample methods don't covert milliseconds to seconds #39912

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
ridwanmsharif opened this issue May 6, 2025 · 3 comments · Fixed by #39913
Closed
Labels
bug Something isn't working receiver/prometheus Prometheus receiver

Comments

@ridwanmsharif
Copy link
Member

Component(s)

receiver/prometheus

What happened?

Description

When being called Append.*CTZeroSample is called with ctMs in milliseconds.

This inturn calls addCreationTimestamp in milliseconds. But that assumes and sets mg.created as though it is called with seconds.

This causes the resulting metrics to have wildly incorrect start times when it is set here:

point.SetStartTimestamp(timestampFromFloat64(mg.created))

since no conversion is done.

This was introduced in this PR: #36660

And this bug was present ever since.

Steps to Reproduce

scrape metrics that have the _created metrics and see if the start time is set appropriately when converted to OTLP

Steps to fix

I'll be sending a PR that makes sure that when mg.created is set, it is set in seconds as expected here: https://github.com/prometheus/OpenMetrics/blob/main/specification/OpenMetrics.md#counter

Collector version

0.120.0 to latest

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

@ridwanmsharif ridwanmsharif added bug Something isn't working needs triage New item requiring triage labels May 6, 2025
@github-actions github-actions bot added the receiver/prometheus Prometheus receiver label May 6, 2025
Copy link
Contributor

github-actions bot commented May 6, 2025

Pinging code owners:

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

ridwanmsharif added a commit to ridwanmsharif/opentelemetry-collector-contrib that referenced this issue May 6, 2025
ridwanmsharif added a commit to ridwanmsharif/opentelemetry-collector-contrib that referenced this issue May 6, 2025
ridwanmsharif added a commit to ridwanmsharif/opentelemetry-collector-contrib that referenced this issue May 6, 2025
@ArthurSens
Copy link
Member

Nice find!

@ArthurSens ArthurSens removed the needs triage New item requiring triage label May 7, 2025
@ridwanmsharif
Copy link
Member Author

When trying to hunt this bug down there were other things I noticed. We also look for the _created metrics in the receiver:

case metricName == mf.metadata.Metric+metricSuffixCreated:
mg.created = v
and try to update the timestamp there as well (in addition to the Append.*CTZeroSample call which I think achieves the same thing already?)

If I'm right, we can remove and clean some of that up now that Append.*CTZeroSample is fully implemented. Is my understanding correct here? Happy to help with some clean up too :)

@atoulme atoulme closed this as completed in 0dbec36 May 8, 2025
dragonlord93 pushed a commit to dragonlord93/opentelemetry-collector-contrib that referenced this issue May 23, 2025
…en-telemetry#39913)

<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue.
Ex. Adding a feature - Explain what this achieves.-->
#### Description
Set the start time appropriately when Append.*CTZeroSample is called.
Also rename so units are clearer in the definition

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

<!--Describe what testing was performed and which tests were added.-->
#### Testing
Added more clarity to the unit tests.

<!--Describe the documentation added.-->
#### Documentation
N/A

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

---------

Signed-off-by: Ridwan Sharif <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working receiver/prometheus Prometheus receiver
Projects
None yet
2 participants