Skip to content

fix(internal_metrics source): emit increments for counters and histograms - #26080

Draft
gwenaskell wants to merge 1 commit into
masterfrom
yoenn.burban/OPA-5040-internal-metrics-increments
Draft

fix(internal_metrics source): emit increments for counters and histograms#26080
gwenaskell wants to merge 1 commit into
masterfrom
yoenn.burban/OPA-5040-internal-metrics-increments

Conversation

@gwenaskell

Copy link
Copy Markdown
Contributor

Summary

The internal_metrics source 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 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!(), including
component_latency_seconds, http_server_handler_duration_seconds, and http_client_rtt_seconds.

That first-observation guard is meaningless for internal_metrics: registry handles are created at
zero 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_metrics now tracks the previous absolute value per series and emits counters and
histograms as Incremental. Falling out of that, without special-casing:

  • A first sighting emits its full value — the registry counts from zero. This is the actual fix.
  • A registry reset (expiry then re-registration) emits the full new value, because
    MetricValue::subtract is guard-checked and bails without mutating.
  • Steady state allocates nothing per series; MetricSeries is cloned only on insert.

Gauges stay Absolute, as does the deprecated internal_metrics_cardinality_total, which is
declared 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_metrics to an
already-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_secs are dropped. That pass is already O(n) per scrape, so it adds no asymptotic
cost.

Downstream effects

  • datadog_metricsmake_incremental now passes metrics through untouched. This is the fix.
  • prometheus_exporter / prometheus_remote_writemake_absolute re-accumulates from zero,
    reproducing the values the registry itself reports. No visible change.
  • Gauges — untouched.

Idle counters now emit Incremental{0} each scrape, the same event volume as the constant absolute
value they emitted before. Idle histograms stop being sent, filtered by the sink's pre-existing
is_empty() check.

Vector configuration

sources:
  internal_metrics:
    type: internal_metrics

sinks:
  dd:
    type: datadog_metrics
    inputs: [internal_metrics]
    default_api_key: "${DD_API_KEY}"

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?

  • New unit tests in src/sources/internal_metrics/delta.rs covering first sighting, subsequent
    deltas, idle zero-delta, histogram bucket differencing, counter reset, gauge and
    cardinality-counter pass-through, seeding, and expired-series eviction.
  • New integration tests in src/sources/internal_metrics/mod.rs: the OPA-5040 scenario (all
    observations inside a single interval still emit), gauges staying absolute, and the build-time
    scrape excluding prior activity.
  • Verified the new tests are not vacuous — each was confirmed to fail when its mechanism is
    removed (disabling the seed yields 15 instead of 5; bypassing the conversion yields Absolute
    instead of Incremental).
  • make check-clippy, cargo fmt --check, make check-markdown, make check-generated-docs,
    cargo vdev check changelog-fragments — all clean.
  • Component validation suite: 7/7 pass.

Two pre-existing failures were observed and confirmed identical on master:
sources::http_client::tests::request_query_vrl_applied (VRL md5 unavailable under this feature
set) and sources::internal_logs::tests::receives_logs.

Is this a breaking change?

  • Yes
  • No

The metric kind of internal counters and histograms changes from absolute to incremental. Sinks
are unaffected, but a transform reading the kind — a remap gating on it, or a consumer of
metric_to_log output reading the kind field — now sees incremental. See the changelog fragment
for migration notes.

Does this PR include user facing changes?

  • Yes. Please add a changelog fragment based on our guidelines.
  • No. A maintainer will apply the no-changelog label 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_counters
overwrote its accumulator for absolute metrics (sum = *value) instead of adding, so a multi-series
counter 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:

  1. http_server and splunk_hec record two component_errors_total increments for a single
    malformed 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 now
    declare errors_per_failure, set to 2 for these two. Worth deciding separately whether the
    double count is intended, since it inflates that metric ~2x for these sources in real
    deployments. It is pre-existing and unrelated to this PR.

  2. 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/service via TaggedEventsSent, or error_type). Single-series metrics such as
    component_sent_bytes_total are unaffected.

References

  • Closes: OPA-5040

@github-actions github-actions Bot added domain: sources Anything related to the Vector's sources domain: core Anything related to core crates i.e. vector-core, core-common, etc labels Aug 11, 2026
…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
gwenaskell force-pushed the yoenn.burban/OPA-5040-internal-metrics-increments branch from 4eeb562 to b9f157e Compare August 11, 2026 10:12
@gwenaskell gwenaskell changed the title fix(internal_metrics source)!: emit increments for counters and histograms fix(internal_metrics source): emit increments for counters and histograms Aug 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

domain: core Anything related to core crates i.e. vector-core, core-common, etc domain: sources Anything related to the Vector's sources

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant