-
Notifications
You must be signed in to change notification settings - Fork 831
Allow metrics with the same name different labels #1800
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
7203084 to
2359244
Compare
Signed-off-by: Jay DeLuca <jaydeluca4@gmail.com>
Signed-off-by: Jay DeLuca <jaydeluca4@gmail.com>
Signed-off-by: Jay DeLuca <jaydeluca4@gmail.com>
Signed-off-by: Jay DeLuca <jaydeluca4@gmail.com>
Signed-off-by: Jay DeLuca <jaydeluca4@gmail.com>
Signed-off-by: Jay DeLuca <jaydeluca4@gmail.com>
2359244 to
9289e4a
Compare
Signed-off-by: Jay DeLuca <jaydeluca4@gmail.com>
I would opt for registration only. What does that mean for clients that only work on snapshots like the OTel SDK? |
Signed-off-by: Jay DeLuca <jaydeluca4@gmail.com>
Signed-off-by: Jay DeLuca <jaydeluca4@gmail.com>
62b9cbc to
02c0db0
Compare
|
still need to incorporate checks for the help/unit |
Signed-off-by: Jay DeLuca <jaydeluca4@gmail.com>
Signed-off-by: Jay DeLuca <jaydeluca4@gmail.com>
Signed-off-by: Jay DeLuca <jaydeluca4@gmail.com>
Signed-off-by: Jay DeLuca <jaydeluca4@gmail.com>
b5396a5 to
1e897dc
Compare
Should be no change in behavior, I've added some tests that emulate the same type of interactions |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Enables registering multiple metrics with the same name when they differ by label schema, while ensuring exposition formats emit a single metric family per name.
Changes:
- Adds registration-time validation in
PrometheusRegistryfor metric type consistency, help/unit consistency, and duplicate label schemas (when collectors provide the new metadata methods). - Allows
MetricSnapshotsto contain duplicate names (same snapshot type), and merges duplicate-name snapshots during exposition writing. - Introduces integration/unit tests and sample apps covering duplicate-name behavior across text/OpenMetrics/protobuf exporters.
Reviewed changes
Copilot reviewed 35 out of 36 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
| prometheus-metrics-model/src/test/java/io/prometheus/metrics/model/registry/PrometheusRegistryTest.java | Expands registry tests for duplicate names, label schemas, help/unit consistency, and unregister behavior. |
| prometheus-metrics-model/src/test/java/io/prometheus/metrics/model/registry/OpenTelemetryExporterRegistryCompatibilityTest.java | Adds regression tests for OTel-style MultiCollector usage (no names/types/labels metadata). |
| prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/snapshots/MetricSnapshots.java | Relaxes duplicate-name constraint; only rejects conflicting snapshot types for same name. |
| prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/registry/PrometheusRegistry.java | Implements registration-time validation and tracking for per-name label schemas/type/help/unit. |
| prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/registry/MultiCollector.java | Adds optional per-metric-name metadata methods (getMetricType, getLabelNames, getMetadata). |
| prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/registry/MetricType.java | Introduces a metric-type enum used for registration-time validation. |
| prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/registry/Collector.java | Adds optional metadata methods (getMetricType, getLabelNames, getMetadata) for registration-time validation. |
| prometheus-metrics-exposition-textformats/src/test/java/io/prometheus/metrics/expositionformats/TextFormatUtilTest.java | Adds unit tests for merging duplicate-name snapshots. |
| prometheus-metrics-exposition-textformats/src/test/java/io/prometheus/metrics/expositionformats/DuplicateNamesExpositionTest.java | Adds text/OpenMetrics exposition tests demonstrating valid output with duplicate names. |
| prometheus-metrics-exposition-textformats/src/main/java/io/prometheus/metrics/expositionformats/TextFormatUtil.java | Adds duplicate-name merging utility used by writers. |
| prometheus-metrics-exposition-textformats/src/main/java/io/prometheus/metrics/expositionformats/PrometheusTextFormatWriter.java | Merges duplicate-name snapshots before writing Prometheus text format. |
| prometheus-metrics-exposition-textformats/src/main/java/io/prometheus/metrics/expositionformats/OpenMetricsTextFormatWriter.java | Merges duplicate-name snapshots before writing OpenMetrics text format. |
| prometheus-metrics-exposition-formats/src/test/java/io/prometheus/metrics/expositionformats/DuplicateNamesProtobufTest.java | Adds protobuf exposition tests for duplicate-name snapshots being merged into one family. |
| prometheus-metrics-exposition-formats/src/main/java/io/prometheus/metrics/expositionformats/internal/PrometheusProtobufWriterImpl.java | Merges duplicate-name snapshots before protobuf serialization. |
| prometheus-metrics-exposition-formats/generate-protobuf.sh | Updates generation script to try to support macOS tooling and mise-provided protoc. |
| prometheus-metrics-core/src/test/java/io/prometheus/metrics/core/metrics/CounterTest.java | Adds test for label normalization affecting registration-time label schema validation. |
| prometheus-metrics-core/src/main/java/io/prometheus/metrics/core/metrics/SummaryWithCallback.java | Implements getMetricType() for registration-time validation. |
| prometheus-metrics-core/src/main/java/io/prometheus/metrics/core/metrics/Summary.java | Implements getMetricType() for registration-time validation. |
| prometheus-metrics-core/src/main/java/io/prometheus/metrics/core/metrics/StateSet.java | Implements getMetricType() for registration-time validation. |
| prometheus-metrics-core/src/main/java/io/prometheus/metrics/core/metrics/MetricWithFixedMetadata.java | Exposes metadata via Collector.getMetadata() and implements normalized getLabelNames(). |
| prometheus-metrics-core/src/main/java/io/prometheus/metrics/core/metrics/Info.java | Implements getMetricType() for registration-time validation. |
| prometheus-metrics-core/src/main/java/io/prometheus/metrics/core/metrics/Histogram.java | Implements getMetricType() for registration-time validation. |
| prometheus-metrics-core/src/main/java/io/prometheus/metrics/core/metrics/GaugeWithCallback.java | Implements getMetricType() for registration-time validation. |
| prometheus-metrics-core/src/main/java/io/prometheus/metrics/core/metrics/Gauge.java | Implements getMetricType() for registration-time validation. |
| prometheus-metrics-core/src/main/java/io/prometheus/metrics/core/metrics/CounterWithCallback.java | Implements getMetricType() for registration-time validation. |
| prometheus-metrics-core/src/main/java/io/prometheus/metrics/core/metrics/Counter.java | Implements getMetricType() for registration-time validation. |
| mise.toml | Adjusts Java toolchain version. |
| integration-tests/it-exporter/pom.xml | Adds a new integration-test sample module for duplicate metrics. |
| integration-tests/it-exporter/it-exporter-test/src/test/java/io/prometheus/metrics/it/exporter/test/DuplicateMetricsIT.java | Adds end-to-end exporter tests validating merged duplicate metrics output. |
| integration-tests/it-exporter/it-exporter-duplicate-metrics-sample/src/main/java/io/prometheus/metrics/it/exporter/duplicatemetrics/DuplicateMetricsSample.java | Adds sample app emitting same-name metrics with different label sets. |
| integration-tests/it-exporter/it-exporter-duplicate-metrics-sample/pom.xml | Maven module for the duplicate-metrics integration sample. |
| integration-tests/it-common/src/test/java/io/prometheus/client/it/common/ExporterTest.java | Makes the container field protected for reuse by new ITs. |
| docs/content/internals/model.md | Documents behavior for collectors that don’t provide type/label metadata. |
| docs/content/getting-started/registry.md | Documents registration-time-only validation and optional metadata methods. |
| docs/content/getting-started/metric-types.md | Adds guidance about avoiding duplicate time series for unvalidated custom collectors. |
| .gitignore | Ignores macOS .DS_Store files. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
...eus-metrics-model/src/main/java/io/prometheus/metrics/model/registry/PrometheusRegistry.java
Outdated
Show resolved
Hide resolved
...eus-metrics-model/src/main/java/io/prometheus/metrics/model/registry/PrometheusRegistry.java
Show resolved
Hide resolved
...eus-metrics-model/src/main/java/io/prometheus/metrics/model/registry/PrometheusRegistry.java
Show resolved
Hide resolved
...ormats/src/main/java/io/prometheus/metrics/expositionformats/PrometheusTextFormatWriter.java
Show resolved
Hide resolved
...eus-metrics-model/src/main/java/io/prometheus/metrics/model/registry/PrometheusRegistry.java
Show resolved
Hide resolved
...sition-textformats/src/main/java/io/prometheus/metrics/expositionformats/TextFormatUtil.java
Show resolved
Hide resolved
...ormats/src/test/java/io/prometheus/metrics/expositionformats/DuplicateNamesProtobufTest.java
Show resolved
Hide resolved
...ormats/src/test/java/io/prometheus/metrics/expositionformats/DuplicateNamesProtobufTest.java
Show resolved
Hide resolved
…ix build order to avoid local issues Signed-off-by: Jay DeLuca <jaydeluca4@gmail.com>
…imestamps logic includes merging Signed-off-by: Jay DeLuca <jaydeluca4@gmail.com>
e9410e8 to
73b47bd
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 38 out of 39 changed files in this pull request and generated 7 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
...eus-metrics-model/src/main/java/io/prometheus/metrics/model/registry/PrometheusRegistry.java
Outdated
Show resolved
Hide resolved
...eus-metrics-model/src/main/java/io/prometheus/metrics/model/registry/PrometheusRegistry.java
Show resolved
Hide resolved
prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/registry/Collector.java
Outdated
Show resolved
Hide resolved
prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/registry/MultiCollector.java
Outdated
Show resolved
Hide resolved
...theus-metrics-model/src/main/java/io/prometheus/metrics/model/snapshots/MetricSnapshots.java
Show resolved
Hide resolved
...ormats/src/test/java/io/prometheus/metrics/expositionformats/DuplicateNamesProtobufTest.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 37 out of 38 changed files in this pull request and generated 7 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
...eus-metrics-model/src/main/java/io/prometheus/metrics/model/registry/PrometheusRegistry.java
Show resolved
Hide resolved
...eus-metrics-model/src/main/java/io/prometheus/metrics/model/registry/PrometheusRegistry.java
Show resolved
Hide resolved
...eus-metrics-model/src/main/java/io/prometheus/metrics/model/registry/PrometheusRegistry.java
Show resolved
Hide resolved
...eus-metrics-model/src/main/java/io/prometheus/metrics/model/registry/PrometheusRegistry.java
Show resolved
Hide resolved
...sition-textformats/src/main/java/io/prometheus/metrics/expositionformats/TextFormatUtil.java
Show resolved
Hide resolved
...mats/src/test/java/io/prometheus/metrics/expositionformats/DuplicateNamesExpositionTest.java
Show resolved
Hide resolved
...ormats/src/test/java/io/prometheus/metrics/expositionformats/DuplicateNamesProtobufTest.java
Show resolved
Hide resolved
Signed-off-by: Jay DeLuca <jaydeluca4@gmail.com>
Signed-off-by: Jay DeLuca <jaydeluca4@gmail.com>
Signed-off-by: Jay DeLuca <jaydeluca4@gmail.com>
c5907ce to
28a8514
Compare
Alternate approach to #1728
Validation occurs at registration time:
collect()at registration in order to introspect existing metadata and that seemed not great due to potential side effects