[prometheusreceiver] Make use of created timestamp from prometheus #36660
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Florian Bacher <[email protected]>
Signed-off-by: Florian Bacher <[email protected]>
Signed-off-by: Florian Bacher <[email protected]>
Signed-off-by: Florian Bacher <[email protected]>
Signed-off-by: Florian Bacher <[email protected]>
Signed-off-by: Florian Bacher <[email protected]>
Signed-off-by: Florian Bacher <[email protected]>
Signed-off-by: Florian Bacher <[email protected]>
|
I think we might need the prometheus update for this to work: #36642 |
thanks for the link to the PR - I was wondering if I should also include the dependency update in my PR but in this case I will wait for the dependency update PR to get merged. One other thing: As of now, it seems like the |
|
This PR was marked stale due to lack of activity. It will be closed in 14 days. |
Signed-off-by: Florian Bacher <[email protected]>
dashpole
approved these changes
Feb 19, 2025
MovieStoreGuy
approved these changes
Feb 20, 2025
| @@ -425,6 +425,11 @@ func (mf *metricFamily) addSeries(seriesRef uint64, metricName string, ls labels | |||
| return nil | |||
| } | |||
|
|
|||
| func (mf *metricFamily) addCreationTimestamp(seriesRef uint64, ls labels.Labels, atMs, created int64) { | |||
| mg := mf.loadMetricGroupOrCreate(seriesRef, ls, atMs) | |||
| mg.created = float64(created) | |||
There was a problem hiding this comment.
This should be setting seconds but is setting milliseconds.
This was referenced May 23, 2025
gracewehner
added a commit
to Azure/prometheus-collector
that referenced
this pull request
May 28, 2025
Change the `EnableCreatedTimestampZeroIngestion` setting in the prometheus scrape manager to use the otelcollector feature flag that has the same name and is disabled by default. This had introduced new parsing during a scrape loop that resulted in very high CPU at higher metric volumes. Reverts this PR: open-telemetry/opentelemetry-collector-contrib#36660 so that the functionality for this is the same as before v0.121.0``
songy23
pushed a commit
that referenced
this pull request
Jun 5, 2025
…us setting to fix high CPU for openmetrics (#40355) <!--Ex. Fixing a bug - Describe the bug and how this fixes the issue. Ex. Adding a feature - Explain what this achieves.--> #### Description The PR #36660 enabled the setting in the scrape manager `EnableCreatedTimestampZeroIngestion` by default. This enabled the [Prometheus feature flag](https://prometheus.io/docs/prometheus/latest/feature_flags/#created-timestamps-zero-injection) by the same name. In Prometheus, when this is set to true, [a call to a new function](https://github.com/prometheus/prometheus/blob/1d9dfde9894f0c82b809b70aa5f9cecab25b8fea/scrape/scrape.go#L1770-L1799) CreatedTimestamp() is run. This currently isn't implemented for the Prometheus parser but is for the OpenMetrics parser and [has a comment](https://github.com/prometheus/prometheus/blob/main/model/textparse/openmetricsparse.go#L287) that it may use additional CPU and memory resources. We are seeing a 10x increase in CPU usage when there's a high volume of metrics scraped with this scenario. This PR puts the setting in the scrape manager behind a feature gate that allows it to be disabled. <!-- Issue number (e.g. #1234) or full URL to issue, if applicable. --> #### Link to tracking issue Fixes #40245 <!--Describe what testing was performed and which tests were added.--> #### Testing Tested with a custom build with this change. <!--Describe the documentation added.--> #### Documentation Documentation about the feature gate exists [here](https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/main/receiver/prometheusreceiver/README.md#getting-started). <!--Please delete paragraphs that you did not use before submitting.--> --------- Co-authored-by: Arthur Silva Sens <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This PR implements the
AppendCTZeroSampleandAppendHistogramCTZeroSamplemethods which can be called by prometheus to pass through the creation timestamp of a metricLink to tracking issue
Fixes #36473
Testing
Added unit tests