Skip to content

metrics/prometheus: rename histogram timings from _ns to _seconds#4546

Open
squiidz wants to merge 1 commit into
mainfrom
fix-prometheus-histogram-timing-seconds
Open

metrics/prometheus: rename histogram timings from _ns to _seconds#4546
squiidz wants to merge 1 commit into
mainfrom
fix-prometheus-histogram-timing-seconds

Conversation

@squiidz

@squiidz squiidz commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

When use_histogram_timing is true, timing metrics are recorded in seconds but were still exported under their _ns name. This both misreported the unit and, in a fleet with mixed use_histogram_timing settings, caused the same metric name to be sent as two Prometheus kinds (summary vs histogram). Remote-write targets such as Vector and Mimir reject the whole batch in that case ("multiple metric kinds given for metric name"), dropping data and adding backpressure.

Rewrite a trailing _ns suffix to _seconds in histogram mode so the histogram series (e.g. processor_latency_seconds) is distinct from the summary series (processor_latency_ns) and the unit suffix is correct.

Fixes INC-1095.

@claude

claude Bot commented Jun 23, 2026

Copy link
Copy Markdown

Commits
LGTM

Review
Small, focused fix that rewrites the _ns suffix to _seconds for timing metrics when use_histogram_timing is enabled, correcting the unit label and keeping histogram series distinct from summary series to avoid Prometheus remote-write "multiple metric kinds" rejections. The rename is correctly scoped to the histogram path only, applied after metric-name validation, and covered by tests for both histogram and summary modes (including inverse NotContains assertions). Docs and changelog are updated consistently.

LGTM

When use_histogram_timing is true, timing metrics are recorded in
seconds but were still exported under their _ns name. This both
misreported the unit and, in a fleet with mixed use_histogram_timing
settings, caused the same metric name to be sent as two Prometheus
kinds (summary vs histogram). Remote-write targets such as Vector and
Mimir reject the whole batch in that case ("multiple metric kinds
given for metric name"), dropping data and adding backpressure.

Rewrite a trailing _ns suffix to _seconds in histogram mode so the
histogram series (e.g. processor_latency_seconds) is distinct from the
summary series (processor_latency_ns) and the unit suffix is correct.

Fixes INC-1095.
@squiidz squiidz force-pushed the fix-prometheus-histogram-timing-seconds branch from a0fff16 to 8525612 Compare June 29, 2026 17:35
@claude

claude Bot commented Jun 29, 2026

Copy link
Copy Markdown

Commits
LGTM

Review
This PR rewrites a _ns timing-metric suffix to _seconds when use_histogram_timing is enabled, correcting the unit label and keeping the histogram series name distinct from the summary series (avoiding Prometheus remote-write "multiple metric kinds" rejections in mixed fleets).

The change is correct and well-scoped: metric-name validation in NewTimerCtor runs on the original path before the rename, the rename only appends a valid suffix, and the renamed path is used consistently as both the timersHist map key and the histogram Name. Unit tests cover both histogram and summary modes (including negative assertions), and the docs/CHANGELOG are updated.

LGTM


Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants