Skip to content

feat: move suffix handling to scrape time#1955

Open
zeitlinger wants to merge 2 commits intomainfrom
scrape-time-suffix-core
Open

feat: move suffix handling to scrape time#1955
zeitlinger wants to merge 2 commits intomainfrom
scrape-time-suffix-core

Conversation

@zeitlinger
Copy link
Member

@zeitlinger zeitlinger commented Mar 17, 2026

Summary

Moves metric name suffix handling (_total, _info, unit suffixes)
from creation time to scrape time. Closes #1941, part of #1942.

  • OM1: smart-appends suffixes (skips if already present)
  • Registry: detects cross-format name collisions at registration
    time

Key changes

  • Remove all reserved metric name suffixes from PrometheusNaming
  • Store original user-provided name separately from exposition base
    name in MetricMetadata (originalName vs expositionBaseName)
  • Smart-append logic in OM1/protobuf writers for _total and _info
  • Two-layer collision detection in PrometheusRegistry
    (base name + exposition names)

Key table

User provides OM1
Counter("events") events_total
Counter("events_total") events_total
Counter("req").unit(BYTES) req_bytes_total
Counter("req_bytes").unit(BYTES) req_bytes_total
Gauge("events_total") events_total

PR stack

  1. This PR — core model + OM1/protobuf writers
  2. OTel preserve_names (stacked on this)
  3. OM2 writer no-suffix (stacked on this)

Test plan

  • mise run compile passes
  • New tests for MetricMetadata 5-arg constructor and
    field accessors
  • New PrometheusRegistryTest covers collision detection
  • Existing snapshot tests updated

Part of #1912.

@jaydeluca
Copy link
Collaborator

jaydeluca commented Mar 17, 2026

i accidentally commented on this one because it has a lot of the same changes as #1956 , is this separate?

edit: just noticed the "blocked by" on the other PR, so I guess i did them out of order

Move metric name suffix handling (_total, _info, unit suffixes) from
creation time to scrape time. Each format writer now owns its suffix
conventions:

- OM1: smart-appends suffixes (skip if already present)
- Registry detects cross-format name collisions at registration time

Key changes:
- Remove all reserved metric name suffixes from PrometheusNaming
- Store original user-provided name separately from exposition base
  name in MetricMetadata (originalName vs expositionBaseName)
- Smart-append logic in OM1/protobuf writers for _total and _info
- Two-layer collision detection in PrometheusRegistry

Closes #1941
Signed-off-by: Gregor Zeitlinger <gregor.zeitlinger@grafana.com>
@zeitlinger zeitlinger force-pushed the scrape-time-suffix-core branch from e63ef43 to 0787661 Compare March 18, 2026 08:03
@zeitlinger
Copy link
Member Author

i accidentally commented on this one because it has a lot of the same changes as #1956 , is this separate?

edit: just noticed the "blocked by" on the other PR, so I guess i did them out of order

no worries - I'll apply the feedback to the correct pr

- Fix race condition: use putIfAbsent for atomic exposition name claiming
  with proper rollback on failure
- Extract claimExpositionNames/releaseExpositionNames for single rollback path
- Rename Info test methods to reflect that _info suffix is now allowed
Signed-off-by: Gregor Zeitlinger <gregor.zeitlinger@grafana.com>
@zeitlinger
Copy link
Member Author

i accidentally commented on this one because it has a lot of the same changes as #1956 , is this separate?
edit: just noticed the "blocked by" on the other PR, so I guess i did them out of order

no worries - I'll apply the feedback to the correct pr

did that now

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Move suffix handling from creation time to scrape time (current release)

2 participants