Allow metrics with the same name different labels#1800
Allow metrics with the same name different labels#1800
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.
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.
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.
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
Outdated
Show resolved
Hide resolved
...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
...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
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 37 out of 38 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
...sition-textformats/src/main/java/io/prometheus/metrics/expositionformats/TextFormatUtil.java
Show resolved
Hide resolved
...metrics-model/src/test/java/io/prometheus/metrics/model/registry/PrometheusRegistryTest.java
Show resolved
Hide resolved
...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
Outdated
Show resolved
Hide resolved
...eus-metrics-model/src/main/java/io/prometheus/metrics/model/registry/PrometheusRegistry.java
Show resolved
Hide resolved
Signed-off-by: Jay DeLuca <jaydeluca4@gmail.com>
Closes #696
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