Skip to content

Conversation

@jaydeluca
Copy link
Collaborator

@jaydeluca jaydeluca commented Jan 24, 2026

Alternate approach to #1728

Validation occurs at registration time:

  • Metrics with the same name, type, unit, and help - but different labels, are allowed to be registered
  • Custom Collectors that do not implement the required fields (prometheus name, metric type) will skip validation and could risk creating duplicate series that will cause issues at scrape time.
    • I looked into adding a fallback, but it requires calling collect() at registration in order to introspect existing metadata and that seemed not great due to potential side effects

@jaydeluca jaydeluca force-pushed the duplicate-names-registration-validation branch from 7203084 to 2359244 Compare January 24, 2026 16:10
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>
@jaydeluca jaydeluca force-pushed the duplicate-names-registration-validation branch from 2359244 to 9289e4a Compare January 24, 2026 16:10
Signed-off-by: Jay DeLuca <jaydeluca4@gmail.com>
@zeitlinger
Copy link
Member

I still also have scrape time validation occurring along with registration time, but we can remove that if we think the registration time validation is sufficient protection.

I would opt for registration only.

What does that mean for clients that only work on snapshots like the OTel SDK?

https://github.com/open-telemetry/opentelemetry-java/blob/da310cc1fcd06e606426649f6c8db96958d9bb12/exporters/prometheus/src/main/java/io/opentelemetry/exporter/prometheus/PrometheusMetricReader.java#L104

Signed-off-by: Jay DeLuca <jaydeluca4@gmail.com>
Signed-off-by: Jay DeLuca <jaydeluca4@gmail.com>
Signed-off-by: Jay DeLuca <jaydeluca4@gmail.com>
@jaydeluca jaydeluca force-pushed the duplicate-names-registration-validation branch from 62b9cbc to 02c0db0 Compare January 29, 2026 20:03
@jaydeluca
Copy link
Collaborator Author

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>
Signed-off-by: Jay DeLuca <jaydeluca4@gmail.com>
Signed-off-by: Jay DeLuca <jaydeluca4@gmail.com>
@jaydeluca jaydeluca force-pushed the duplicate-names-registration-validation branch from b5396a5 to 1e897dc Compare January 30, 2026 17:53
@jaydeluca jaydeluca changed the title Allow metrics with the same name (Option #2) Allow metrics with the same name different labels Jan 30, 2026
@jaydeluca
Copy link
Collaborator Author

What does that mean for clients that only work on snapshots like the OTel SDK?

Should be no change in behavior, I've added some tests that emulate the same type of interactions

Copy link
Contributor

Copilot AI left a 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 PrometheusRegistry for metric type consistency, help/unit consistency, and duplicate label schemas (when collectors provide the new metadata methods).
  • Allows MetricSnapshots to 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.

Comment on lines +198 to +202
List<String> prometheusNamesList = collector.getPrometheusNames();
List<MultiCollectorRegistration> registrations = new ArrayList<>();

for (String prometheusName : prometheusNamesList) {
MetricType metricType = collector.getMetricType(prometheusName);
Copy link

Copilot AI Jan 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PrometheusRegistry.register(MultiCollector) mutates shared state incrementally while iterating prometheusNamesList. If registration throws for a later name, earlier per-name updates can remain even though the MultiCollector itself was never added, leaving orphaned RegistrationInfo entries. Wrap the loop in try/catch and rollback any successful per-name registrations on failure (or pre-validate all names before mutating shared state).

Copilot uses AI. Check for mistakes.
Comment on lines +34 to +38
CollectorRegistration(String prometheusName, @Nullable Set<String> labelNames) {
this.prometheusName = prometheusName;
this.labelNames =
(labelNames == null || labelNames.isEmpty()) ? Collections.emptySet() : labelNames;
}
Copy link

Copilot AI Jan 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CollectorRegistration stores the Set returned by Collector.getLabelNames() directly. Because these Sets are later used as keys in labelSchemas and for unregistration, mutating the returned Set after registration can break duplicate detection and/or prevent cleanup on unregister. Consider defensively copying/normalizing to an immutable Set (e.g., Set.copyOf(normalized)) at registration time.

Copilot uses AI. Check for mistakes.
Comment on lines +49 to +53
MultiCollectorRegistration(String prometheusName, @Nullable Set<String> labelNames) {
this.prometheusName = prometheusName;
this.labelNames =
(labelNames == null || labelNames.isEmpty()) ? Collections.emptySet() : labelNames;
}
Copy link

Copilot AI Jan 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MultiCollectorRegistration also stores the Set returned by MultiCollector.getLabelNames(name) directly. If that Set is mutable and later changes, unregisterLabelSchema may fail to remove the schema and the registered map entry can leak. Prefer storing an immutable defensive copy (e.g., Set.copyOf(normalized)).

Copilot uses AI. Check for mistakes.
Comment on lines +10 to +11
if [[ "$OSTYPE" == "darwin"* ]] && command -v gsed >/dev/null 2>&1; then
SED='gsed'
Copy link

Copilot AI Jan 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On macOS, this script falls back to BSD sed when gsed is not installed, but later uses "$SED -i". BSD sed requires an argument for -i (e.g., -i ''), so the fallback will fail. Either hard-require gsed on macOS with a clear error message, or use a portable in-place edit strategy for both GNU/BSD sed.

Suggested change
if [[ "$OSTYPE" == "darwin"* ]] && command -v gsed >/dev/null 2>&1; then
SED='gsed'
if [[ "$OSTYPE" == "darwin"* ]]; then
if command -v gsed >/dev/null 2>&1; then
SED='gsed'
else
echo "Error: gsed (GNU sed) is required on macOS. Please install it, e.g. with 'brew install gnu-sed'." >&2
exit 1
fi

Copilot uses AI. Check for mistakes.
Comment on lines +118 to 122
MetricSnapshots merged = TextFormatUtil.mergeDuplicates(metricSnapshots);
for (MetricSnapshot s : merged) {
MetricSnapshot snapshot = escapeMetricSnapshot(s, scheme);
if (!snapshot.getDataPoints().isEmpty()) {
if (snapshot instanceof CounterSnapshot) {
Copy link

Copilot AI Jan 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PrometheusTextFormatWriter now merges duplicate metric snapshots for the main output, but the optional created-timestamp pass still uses the original MetricSnapshots. With duplicate names enabled this can emit repeated HELP/TYPE blocks and potentially duplicate _created series. Consider iterating over the merged snapshots (or otherwise merging per metric family) for the created-timestamp section too.

Copilot uses AI. Check for mistakes.
Comment on lines +16 to +20
if [[ "$OSTYPE" == "darwin"* ]] && command -v ggrep >/dev/null 2>&1; then
GREP='ggrep'
else
GREP='grep'
fi
Copy link

Copilot AI Jan 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On macOS, falling back to BSD grep will break later when using "$GREP -oP" because BSD grep does not support -P. Either hard-require ggrep on macOS with a clear error message, or replace the -P usage with a portable alternative (e.g., awk/sed/perl).

Copilot uses AI. Check for mistakes.
Comment on lines +94 to +98
void validateMetadata(@Nullable String newHelp, @Nullable Unit newUnit) {
if (help != null && newHelp != null && !Objects.equals(help, newHelp)) {
throw new IllegalArgumentException(
"Conflicting help strings. Existing: \"" + help + "\", new: \"" + newHelp + "\"");
}
Copy link

Copilot AI Jan 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

RegistrationInfo.validateMetadata() only checks conflicts when both existing and new help/unit are non-null, but never records newly observed non-null help/unit when the stored value is null. If the first registration omits help/unit and later ones provide differing values, conflicts will never be detected and exposition output may become nondeterministic. Consider capturing the first non-null help/unit so subsequent registrations can be validated consistently.

Copilot uses AI. Check for mistakes.
Comment on lines +247 to +249
return new HistogramSnapshot(
first.getMetadata(),
(Collection<HistogramSnapshot.HistogramDataPointSnapshot>) (Object) allDataPoints);
Copy link

Copilot AI Jan 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TextFormatUtil.mergeSnapshots() creates a new HistogramSnapshot using the constructor that defaults isGaugeHistogram to false. This drops the gaugeHistogram flag when merging duplicate histogram snapshots (and can change OpenMetrics output from "gaugehistogram" to "histogram"). Preserve (and validate consistency of) the gaugeHistogram flag when merging HistogramSnapshot instances (e.g., use the boolean constructor and ensure all snapshots in the group agree).

Suggested change
return new HistogramSnapshot(
first.getMetadata(),
(Collection<HistogramSnapshot.HistogramDataPointSnapshot>) (Object) allDataPoints);
boolean isGaugeHistogram = ((HistogramSnapshot) first).isGaugeHistogram();
for (MetricSnapshot snapshot : snapshots) {
boolean snapshotIsGaugeHistogram = ((HistogramSnapshot) snapshot).isGaugeHistogram();
if (snapshotIsGaugeHistogram != isGaugeHistogram) {
throw new IllegalArgumentException(
"Cannot merge HistogramSnapshots with different gaugeHistogram flags");
}
}
return new HistogramSnapshot(
first.getMetadata(),
(Collection<HistogramSnapshot.HistogramDataPointSnapshot>) (Object) allDataPoints,
isGaugeHistogram);

Copilot uses AI. Check for mistakes.
}

assertThat(counterFamily).isNotNull();
assertThat(counterFamily.getType()).isEqualTo(Metrics.MetricType.COUNTER);
Copy link

Copilot AI Jan 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Variable counterFamily may be null at this access because of this assignment.

Copilot uses AI. Check for mistakes.
assertThat(counterFamily.getMetric(0).getCounter().getValue()).isEqualTo(100.0);

assertThat(gaugeFamily).isNotNull();
assertThat(gaugeFamily.getType()).isEqualTo(Metrics.MetricType.GAUGE);
Copy link

Copilot AI Jan 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Variable gaugeFamily may be null at this access because of this assignment.

Copilot uses AI. Check for mistakes.
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.

3 participants