perf: scan metric name validity once per metric family - #2443
Draft
david-mollitor-db wants to merge 1 commit into
Draft
perf: scan metric name validity once per metric family#2443david-mollitor-db wants to merge 1 commit into
david-mollitor-db wants to merge 1 commit into
Conversation
The Prometheus, OpenMetrics, and OpenMetrics 2 text writers call PrometheusNaming.isValidLegacyMetricName(name) for every data point line - once to choose bare-vs-braced metric syntax and again inside writeName(). The metric name is constant across all data points of a family, so this re-scans the same string ~2N times per scrape for a family with N series. Scan the name once per family and pass the boolean result down through writeNameAndLabels()/writeName(). Appending a suffix built from legacy characters (_total, _bucket, _count, _created, ...) does not change legacy validity, so the base-name result applies to the suffixed names too. No behavior change: exposition output is byte-identical (the full exposition-format test suite, including the golden-output ExpositionFormatsTest across all formats and escaping schemes, passes unchanged). This is a CPU-only optimization; per-scrape allocation is unchanged. On a high-cardinality scrape+serialize benchmark (one counter family, 100000 series) throughput improved ~7% (6.9 -> 7.3 ops/s) with allocation unchanged. Signed-off-by: David Mollitor <david.mollitor@databricks.com>
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
1 participant
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 Prometheus, OpenMetrics, and OpenMetrics 2 text writers validate the metric
name with
PrometheusNaming.isValidLegacyMetricName(name)on every data pointline — once to choose bare-vs-braced metric syntax, and again inside
writeName(). The metric name is constant across all data points of a family,so for a family with N series this re-scans the same string ~2N times per scrape.
This change scans the name once per family and threads the boolean result
down through
writeNameAndLabels()/writeName()(a newwriteName(writer, name, nameType, boolean validLegacyName)overload trusts theprecomputed value). A suffix built from legacy characters (
_total,_bucket,_count,_created, …) never changes legacy validity, so the base-name resultapplies to the suffixed names too.
No behavior change
Output is byte-identical — the full exposition-format test suite passes
unchanged, including the golden-output
ExpositionFormatsTestacross everyformat and escaping scheme,
OpenMetrics2TextFormatWriterTest, andPrometheusNamingTest. This is a CPU-only optimization; per-scrape allocation isunchanged.
Benchmark
A local high-cardinality scrape+serialize benchmark (one counter family, 100,000
series; JMH
-f 1 -wi 4 -i 5, GC profiler):~+7% throughput on this path, allocation unchanged. The effect scales with the
number of series and the metric-name length.
Notes
The per-label-name validity check is a similar (and larger, O(N × labels))
per-scrape rescan and is a natural follow-up; this PR intentionally scopes to the
metric name only.
This pull request and its description were written by Isaac.