openmetrics: fix native transformer locking to first observed metric type#24299
Draft
lucia-sb wants to merge 4 commits into
Draft
openmetrics: fix native transformer locking to first observed metric type#24299lucia-sb wants to merge 4 commits into
lucia-sb wants to merge 4 commits into
Conversation
…ric type OpenMetrics strips the `_total` suffix from counter family names, so a gauge and its `_total` counter counterpart (e.g. go_memstats_alloc_bytes) collapse into the same metric name. The default `native` transformer cached whichever type it saw first and reused it forever, so one of the two samples always got submitted under the wrong type. `native` now shares native_dynamic's per-type caching, so both samples are always submitted correctly regardless of order. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Contributor
|
…ge/_total-counter collisions The base-check fix in this PR (native transformer no longer locking to the first observed metric type) now correctly emits the counter side of a gauge/_total-counter collision that was previously silently dropped. Update metadata.csv and test metric lists to reflect the newly-correct metrics: go.memstats.alloc_bytes.count (coredns, istio, krakend, kuma), the mesh-agent variant (istio), and gitaly.go.memstats_alloc_bytes.count plus a second, previously unreported collision, gitaly.catfile_processes.count (gitlab). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
It only forwarded to get_native_dynamic_transformer. Point the 'native' TRANSFORMERS entry at get_native_dynamic_transformer directly instead of calling one factory from the other. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Contributor
Validation ReportAll 21 validations passed. Show details
|
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
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.
What does this PR do?
Fixes the default
nativeOpenMetrics V2 transformer so it no longer locks onto whichever metric type it sees first for a given metric name. It now reusesnative_dynamic's per-type caching, so a metric name that legitimately reports more than one type (e.g. a gauge/counter pair colliding after OpenMetrics strips the_totalsuffix) gets each sample routed to the correct transformer regardless of arrival order.Adds a regression test (
test_v2/test_transformers/test_native.py) covering both the plain single-type case and the gauge/_total-counter collision.Motivation
AI-6931: several integrations (krakend, kuma, coredns v2, istio, gitlab; argocd has a separate broken custom transformer) mis-submit
go_memstats_alloc_bytesbecause Go exposes it as both a gauge and a_totalcounter. OpenMetrics strips the_totalsuffix from counter family names, so both samples collapse into the same metric name. The defaultnativetransformer cached the first sample's type (transformer = None, set once) and reused it forever, so the gauge's value gets silently overwritten by the counter's large cumulative value, and the.countmetric is never emitted.native_dynamicalready handles this correctly by caching a transformer per observed type instead of a single slot. This PR makesnative— the default type for every metric not given an explicittype— share that same per-type caching, fixing the collision at the base-check level instead of requiring anative_dynamicannotation in every affected integration.Review checklist (to be filled by reviewers)
qa/requiredif this PR needs QA validation, orqa/skip-qaif it does not. Exactly one of the two is required.backport/<branch-name>label to the PR and it will automatically open a backport PR once this one is merged🤖 Generated with Claude Code