fix(internal_metrics source): emit increments for counters and histograms - #26080
Draft
gwenaskell wants to merge 1 commit into
Draft
fix(internal_metrics source): emit increments for counters and histograms#26080gwenaskell wants to merge 1 commit into
gwenaskell wants to merge 1 commit into
Conversation
…rams `internal_metrics` emitted every metric as absolute. Sinks that require incremental metrics discard the first observation of an absolute series to avoid reporting an accumulated counter as one enormous delta, and then filter histograms whose delta is empty. A histogram observed only within a single scrape interval therefore established a baseline, produced zero deltas forever after, and was never sent. Counters lost one scrape interval. The internal registry starts at zero within the process, so the first value observed for a series is already a correct increment and nothing needs to be discarded. Track the previous absolute value per series in the source and emit counters and histograms as increments. Gauges stay absolute, as does `internal_metrics_cardinality_total`, which is declared a counter but reports the current series count and so is not monotonic. The source also scrapes once while being built, so adding it to an already-running Vector, or reloading the configuration, reports the change since that point rather than the whole accumulated registry. Series that expire from the registry are dropped from the tracked state via a per-scrape generation stamp, bounding the map. Component validation is adjusted as a consequence: `sum_counters` overwrote its accumulator for absolute metrics instead of adding, so multi-series counters silently reported one arbitrary series rather than their total. It now always sums. This makes `component_errors_total` correctly report that `http_server` and `splunk_hec` record two errors for a single malformed request, so test cases can declare `errors_per_failure`.
gwenaskell
force-pushed
the
yoenn.burban/OPA-5040-internal-metrics-increments
branch
from
August 11, 2026 10:12
4eeb562 to
b9f157e
Compare
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.
Summary
The
internal_metricssource emitted every metric asAbsolute. Sinks that require incrementalmetrics discard the first observation of an absolute series — to avoid reporting an accumulated
counter as one enormous delta after a restart — and then filter histograms whose delta is empty.
The consequence is that a histogram observed only within a single scrape interval established a
baseline, produced zero deltas forever after, and was never sent at all. Counters lost one
scrape interval. This affects every metric recorded via
histogram!(), includingcomponent_latency_seconds,http_server_handler_duration_seconds, andhttp_client_rtt_seconds.That first-observation guard is meaningless for
internal_metrics: registry handles are created atzero within the process, so the first value observed for a series is already a correct increment
and nothing needs discarding.
This is the "Fix B" option from OPA-5040.
What changed
internal_metricsnow tracks the previous absolute value per series and emits counters andhistograms as
Incremental. Falling out of that, without special-casing:MetricValue::subtractis guard-checked and bails without mutating.MetricSeriesis cloned only on insert.Gauges stay
Absolute, as does the deprecatedinternal_metrics_cardinality_total, which isdeclared a counter but reports the current series count and so is not monotonic.
The source also scrapes the registry once while being built, so adding
internal_metricsto analready-running Vector — or reloading the configuration, which rebuilds sources — reports the change
since that point instead of the entire accumulated registry as one increment.
State is bounded by a per-scrape generation stamp: series that expire from the registry via
expire_metrics_secsare dropped. That pass is already O(n) per scrape, so it adds no asymptoticcost.
Downstream effects
datadog_metrics—make_incrementalnow passes metrics through untouched. This is the fix.prometheus_exporter/prometheus_remote_write—make_absolutere-accumulates from zero,reproducing the values the registry itself reports. No visible change.
Idle counters now emit
Incremental{0}each scrape, the same event volume as the constant absolutevalue they emitted before. Idle histograms stop being sent, filtered by the sink's pre-existing
is_empty()check.Vector configuration
Before this change, a histogram whose observations all land inside one scrape interval never
reaches the sink. After it, the first scrape carries the full observation.
How did you test this PR?
src/sources/internal_metrics/delta.rscovering first sighting, subsequentdeltas, idle zero-delta, histogram bucket differencing, counter reset, gauge and
cardinality-counter pass-through, seeding, and expired-series eviction.
src/sources/internal_metrics/mod.rs: the OPA-5040 scenario (allobservations inside a single interval still emit), gauges staying absolute, and the build-time
scrape excluding prior activity.
removed (disabling the seed yields
15instead of5; bypassing the conversion yieldsAbsoluteinstead of
Incremental).make check-clippy,cargo fmt --check,make check-markdown,make check-generated-docs,cargo vdev check changelog-fragments— all clean.Two pre-existing failures were observed and confirmed identical on
master:sources::http_client::tests::request_query_vrl_applied(VRLmd5unavailable under this featureset) and
sources::internal_logs::tests::receives_logs.Is this a breaking change?
The metric kind of internal counters and histograms changes from
absolutetoincremental. Sinksare unaffected, but a transform reading the kind — a
remapgating on it, or a consumer ofmetric_to_logoutput reading thekindfield — now seesincremental. See the changelog fragmentfor migration notes.
Does this PR include user facing changes?
no-changeloglabel to this PR.Added
changelog.d/internal_metrics_increments.breaking.md.Reviewer notes
The component-validation change is a required consequence, not scope creep.
sum_countersoverwrote its accumulator for absolute metrics (
sum = *value) instead of adding, so a multi-seriescounter silently reported one arbitrary series rather than its total — it only ever appeared correct
because "last one wins" happens to recover the absolute value of a single series. Now that
counters arrive incrementally it always sums, which is order-independent and correct.
That surfaced two things worth flagging:
http_serverandsplunk_hecrecord twocomponent_errors_totalincrements for a singlemalformed request — a decode/parse error plus the 400 rejection it causes, as two distinct
series. The framework hardcoded "one error per failed input" (with a standing
TODO: this assumption may need to be made configurable), which masked it. Test cases can nowdeclare
errors_per_failure, set to2for these two. Worth deciding separately whether thedouble count is intended, since it inflates that metric ~2x for these sources in real
deployments. It is pre-existing and unrelated to this PR.
Downstream projects embedding Vector that maintain their own validation test cases should
expect to update expected values for multi-series counters (anything tagged with
output,source/serviceviaTaggedEventsSent, orerror_type). Single-series metrics such ascomponent_sent_bytes_totalare unaffected.References