Refactoring skip lists for metric name validation to be metric specific (#140643)

mosche · web-flow · commit 705a330ae688 · 2026-01-16T07:56:50.000+01:00
diff --git a/modules/apm/src/main/java/org/elasticsearch/telemetry/apm/internal/MetricValidator.java b/modules/apm/src/main/java/org/elasticsearch/telemetry/apm/internal/MetricValidator.java
diff --git a/modules/apm/src/main/java/org/elasticsearch/telemetry/apm/internal/metrics/DoubleAsyncCounterAdapter.java b/modules/apm/src/main/java/org/elasticsearch/telemetry/apm/internal/metrics/DoubleAsyncCounterAdapter.java
@@ -52,7 +52,7 @@ public ObservableDoubleCounter build(Meter meter) {
                 .setDescription(description)
                 .setUnit(unit)
                 .ofDoubles()
-                .buildWithCallback(OtelHelper.doubleMeasurementCallback(observer));
+                .buildWithCallback(OtelHelper.doubleMeasurementCallback(name, observer));
         }
     }
 }
diff --git a/modules/apm/src/main/java/org/elasticsearch/telemetry/apm/internal/metrics/DoubleCounterAdapter.java b/modules/apm/src/main/java/org/elasticsearch/telemetry/apm/internal/metrics/DoubleCounterAdapter.java
@@ -40,7 +40,7 @@ public void incrementBy(double inc) {
     @Override
     public void incrementBy(double inc, Map<String, Object> attributes) {
         assert inc >= 0;
-        getInstrument().add(inc, OtelHelper.fromMap(attributes));
+        getInstrument().add(inc, OtelHelper.fromMap(getName(), attributes));
     }
 
     private static class Builder extends AbstractInstrument.Builder<DoubleCounter> {
diff --git a/modules/apm/src/main/java/org/elasticsearch/telemetry/apm/internal/metrics/DoubleGaugeAdapter.java b/modules/apm/src/main/java/org/elasticsearch/telemetry/apm/internal/metrics/DoubleGaugeAdapter.java
@@ -55,7 +55,7 @@ public ObservableDoubleGauge build(Meter meter) {
                 .gaugeBuilder(name)
                 .setDescription(description)
                 .setUnit(unit)
-                .buildWithCallback(OtelHelper.doubleMeasurementCallback(observer));
+                .buildWithCallback(OtelHelper.doubleMeasurementCallback(name, observer));
         }
     }
 }
diff --git a/modules/apm/src/main/java/org/elasticsearch/telemetry/apm/internal/metrics/DoubleHistogramAdapter.java b/modules/apm/src/main/java/org/elasticsearch/telemetry/apm/internal/metrics/DoubleHistogramAdapter.java
@@ -35,7 +35,7 @@ public void record(double value) {
 
     @Override
     public void record(double value, Map<String, Object> attributes) {
-        getInstrument().record(value, OtelHelper.fromMap(attributes));
+        getInstrument().record(value, OtelHelper.fromMap(getName(), attributes));
     }
 
     private static class Builder extends AbstractInstrument.Builder<DoubleHistogram> {
diff --git a/modules/apm/src/main/java/org/elasticsearch/telemetry/apm/internal/metrics/DoubleUpDownCounterAdapter.java b/modules/apm/src/main/java/org/elasticsearch/telemetry/apm/internal/metrics/DoubleUpDownCounterAdapter.java
@@ -35,7 +35,7 @@ public void add(double inc) {
 
     @Override
     public void add(double inc, Map<String, Object> attributes) {
-        getInstrument().add(inc, OtelHelper.fromMap(attributes));
+        getInstrument().add(inc, OtelHelper.fromMap(getName(), attributes));
     }
 
     private static class Builder extends AbstractInstrument.Builder<DoubleUpDownCounter> {
diff --git a/modules/apm/src/main/java/org/elasticsearch/telemetry/apm/internal/metrics/LongAsyncCounterAdapter.java b/modules/apm/src/main/java/org/elasticsearch/telemetry/apm/internal/metrics/LongAsyncCounterAdapter.java
@@ -51,7 +51,7 @@ public ObservableLongCounter build(Meter meter) {
                 .counterBuilder(name)
                 .setDescription(description)
                 .setUnit(unit)
-                .buildWithCallback(OtelHelper.longMeasurementCallback(observer));
+                .buildWithCallback(OtelHelper.longMeasurementCallback(name, observer));
         }
     }
 }
diff --git a/modules/apm/src/main/java/org/elasticsearch/telemetry/apm/internal/metrics/LongCounterAdapter.java b/modules/apm/src/main/java/org/elasticsearch/telemetry/apm/internal/metrics/LongCounterAdapter.java
@@ -39,7 +39,7 @@ public void incrementBy(long inc) {
     @Override
     public void incrementBy(long inc, Map<String, Object> attributes) {
         assert inc >= 0;
-        getInstrument().add(inc, OtelHelper.fromMap(attributes));
+        getInstrument().add(inc, OtelHelper.fromMap(getName(), attributes));
     }
 
     private static class Builder extends AbstractInstrument.Builder<LongCounter> {
diff --git a/modules/apm/src/main/java/org/elasticsearch/telemetry/apm/internal/metrics/LongGaugeAdapter.java b/modules/apm/src/main/java/org/elasticsearch/telemetry/apm/internal/metrics/LongGaugeAdapter.java
@@ -47,7 +47,7 @@ public ObservableLongGauge build(Meter meter) {
                 .ofLongs()
                 .setDescription(description)
                 .setUnit(unit)
-                .buildWithCallback(OtelHelper.longMeasurementCallback(observer));
+                .buildWithCallback(OtelHelper.longMeasurementCallback(name, observer));
         }
     }
 }
diff --git a/modules/apm/src/main/java/org/elasticsearch/telemetry/apm/internal/metrics/LongHistogramAdapter.java b/modules/apm/src/main/java/org/elasticsearch/telemetry/apm/internal/metrics/LongHistogramAdapter.java
@@ -32,7 +32,7 @@ public void record(long value) {
 
     @Override
     public void record(long value, Map<String, Object> attributes) {
-        getInstrument().record(value, OtelHelper.fromMap(attributes));
+        getInstrument().record(value, OtelHelper.fromMap(getName(), attributes));
     }
 
     private static class Builder extends AbstractInstrument.Builder<LongHistogram> {
diff --git a/modules/apm/src/main/java/org/elasticsearch/telemetry/apm/internal/metrics/LongUpDownCounterAdapter.java b/modules/apm/src/main/java/org/elasticsearch/telemetry/apm/internal/metrics/LongUpDownCounterAdapter.java
@@ -35,7 +35,7 @@ public void add(long inc) {
 
     @Override
     public void add(long inc, Map<String, Object> attributes) {
-        getInstrument().add(inc, OtelHelper.fromMap(attributes));
+        getInstrument().add(inc, OtelHelper.fromMap(getName(), attributes));
     }
 
     private static class Builder extends AbstractInstrument.Builder<LongUpDownCounter> {
diff --git a/modules/apm/src/main/java/org/elasticsearch/telemetry/apm/internal/metrics/OtelHelper.java b/modules/apm/src/main/java/org/elasticsearch/telemetry/apm/internal/metrics/OtelHelper.java
@@ -27,12 +27,12 @@
 class OtelHelper {
     private static final Logger logger = LogManager.getLogger(OtelHelper.class);
 
-    static Attributes fromMap(Map<String, Object> attributes) {
+    static Attributes fromMap(String metricName, Map<String, Object> attributes) {
         if (attributes == null || attributes.isEmpty()) {
             return Attributes.empty();
         }
 
-        MetricValidator.assertValidAttributeNames(attributes);
+        MetricValidator.assertValidAttributeNames(metricName, attributes);
 
         var builder = Attributes.builder();
         attributes.forEach((k, v) -> {
@@ -59,7 +59,10 @@ static Attributes fromMap(Map<String, Object> attributes) {
         return builder.build();
     }
 
-    static Consumer<ObservableDoubleMeasurement> doubleMeasurementCallback(Supplier<Collection<DoubleWithAttributes>> observer) {
+    static Consumer<ObservableDoubleMeasurement> doubleMeasurementCallback(
+        String metricName,
+        Supplier<Collection<DoubleWithAttributes>> observer
+    ) {
         return measurement -> {
             Collection<DoubleWithAttributes> observations;
             try {
@@ -74,13 +77,16 @@ static Consumer<ObservableDoubleMeasurement> doubleMeasurementCallback(Supplier<
             }
             for (DoubleWithAttributes observation : observations) {
                 if (observation != null) {
-                    measurement.record(observation.value(), OtelHelper.fromMap(observation.attributes()));
+                    measurement.record(observation.value(), OtelHelper.fromMap(metricName, observation.attributes()));
                 }
             }
         };
     }
 
-    static Consumer<ObservableLongMeasurement> longMeasurementCallback(Supplier<Collection<LongWithAttributes>> observer) {
+    static Consumer<ObservableLongMeasurement> longMeasurementCallback(
+        String metricName,
+        Supplier<Collection<LongWithAttributes>> observer
+    ) {
         return measurement -> {
             Collection<LongWithAttributes> observations;
             try {
@@ -95,7 +101,7 @@ static Consumer<ObservableLongMeasurement> longMeasurementCallback(Supplier<Coll
             }
             for (LongWithAttributes observation : observations) {
                 if (observation != null) {
-                    measurement.record(observation.value(), OtelHelper.fromMap(observation.attributes()));
+                    measurement.record(observation.value(), OtelHelper.fromMap(metricName, observation.attributes()));
                 }
             }
         };
diff --git a/modules/apm/src/test/java/org/elasticsearch/telemetry/apm/internal/metrics/AsyncCountersAdapterTests.java b/modules/apm/src/test/java/org/elasticsearch/telemetry/apm/internal/metrics/AsyncCountersAdapterTests.java
@@ -106,7 +106,7 @@ public void testLongWithInvalidAttribute() {
         registry.registerLongAsyncCounter("es.test.name.total", "desc", "unit", () -> new LongWithAttributes(1, Map.of("index", "index1")));
 
         AssertionError error = assertThrows(AssertionError.class, otelMeter::collectMetrics);
-        assertThat(error.getMessage(), containsString("Attribute name [index] is forbidden"));
+        assertThat(error.getMessage(), containsString("Attribute [index] of [es.test.name.total] is forbidden"));
     }
 
     public void testDoubleWithInvalidAttribute() {
@@ -118,7 +118,7 @@ public void testDoubleWithInvalidAttribute() {
         );
 
         AssertionError error = assertThrows(AssertionError.class, otelMeter::collectMetrics);
-        assertThat(error.getMessage(), containsString("Attribute name [es_has_timestamp] is forbidden"));
+        assertThat(error.getMessage(), containsString("Attribute [es_has_timestamp] of [es.test.name.total] is forbidden"));
     }
 
     public void testNullRecord() throws Exception {
diff --git a/modules/apm/src/test/java/org/elasticsearch/telemetry/apm/internal/metrics/GaugeAdapterTests.java b/modules/apm/src/test/java/org/elasticsearch/telemetry/apm/internal/metrics/GaugeAdapterTests.java
@@ -124,7 +124,7 @@ public void testLongGaugeWithInvalidAttribute() {
         registry.registerLongGauge("es.test.name.total", "desc", "unit", () -> new LongWithAttributes(1, Map.of("index", "index1")));
 
         AssertionError error = assertThrows(AssertionError.class, otelMeter::collectMetrics);
-        assertThat(error.getMessage(), containsString("Attribute name [index] is forbidden"));
+        assertThat(error.getMessage(), containsString("Attribute [index] of [es.test.name.total] is forbidden"));
     }
 
     public void testDoubleGaugeWithInvalidAttribute() {
@@ -136,6 +136,6 @@ public void testDoubleGaugeWithInvalidAttribute() {
         );
 
         AssertionError error = assertThrows(AssertionError.class, otelMeter::collectMetrics);
-        assertThat(error.getMessage(), containsString("Attribute name [es_has_timestamp] is forbidden"));
+        assertThat(error.getMessage(), containsString("Attribute [es_has_timestamp] of [es.test.name.total] is forbidden"));
     }
 }

Original file line number	Diff line number	Diff line change
`@@ -52,7 +52,7 @@ public ObservableDoubleCounter build(Meter meter) {`
`52`	`52`	`.setDescription(description)`
`53`	`53`	`.setUnit(unit)`
`54`	`54`	`.ofDoubles()`
`55`		`- .buildWithCallback(OtelHelper.doubleMeasurementCallback(observer));`
	`55`	`+ .buildWithCallback(OtelHelper.doubleMeasurementCallback(name, observer));`
`56`	`56`	`}`
`57`	`57`	`}`
`58`	`58`	`}`
Original file line number	Diff line number	Diff line change
`@@ -40,7 +40,7 @@ public void incrementBy(double inc) {`
`40`	`40`	`@Override`
`41`	`41`	`public void incrementBy(double inc, Map<String, Object> attributes) {`
`42`	`42`	`assert inc >= 0;`
`43`		`- getInstrument().add(inc, OtelHelper.fromMap(attributes));`
	`43`	`+ getInstrument().add(inc, OtelHelper.fromMap(getName(), attributes));`
`44`	`44`	`}`
`45`	`45`
`46`	`46`	`private static class Builder extends AbstractInstrument.Builder<DoubleCounter> {`
Original file line number	Diff line number	Diff line change
`@@ -55,7 +55,7 @@ public ObservableDoubleGauge build(Meter meter) {`
`55`	`55`	`.gaugeBuilder(name)`
`56`	`56`	`.setDescription(description)`
`57`	`57`	`.setUnit(unit)`
`58`		`- .buildWithCallback(OtelHelper.doubleMeasurementCallback(observer));`
	`58`	`+ .buildWithCallback(OtelHelper.doubleMeasurementCallback(name, observer));`
`59`	`59`	`}`
`60`	`60`	`}`
`61`	`61`	`}`
Original file line number	Diff line number	Diff line change
`@@ -35,7 +35,7 @@ public void record(double value) {`
`35`	`35`
`36`	`36`	`@Override`
`37`	`37`	`public void record(double value, Map<String, Object> attributes) {`
`38`		`- getInstrument().record(value, OtelHelper.fromMap(attributes));`
	`38`	`+ getInstrument().record(value, OtelHelper.fromMap(getName(), attributes));`
`39`	`39`	`}`
`40`	`40`
`41`	`41`	`private static class Builder extends AbstractInstrument.Builder<DoubleHistogram> {`
Original file line number	Diff line number	Diff line change
`@@ -35,7 +35,7 @@ public void add(double inc) {`
`35`	`35`
`36`	`36`	`@Override`
`37`	`37`	`public void add(double inc, Map<String, Object> attributes) {`
`38`		`- getInstrument().add(inc, OtelHelper.fromMap(attributes));`
	`38`	`+ getInstrument().add(inc, OtelHelper.fromMap(getName(), attributes));`
`39`	`39`	`}`
`40`	`40`
`41`	`41`	`private static class Builder extends AbstractInstrument.Builder<DoubleUpDownCounter> {`
Original file line number	Diff line number	Diff line change
`@@ -51,7 +51,7 @@ public ObservableLongCounter build(Meter meter) {`
`51`	`51`	`.counterBuilder(name)`
`52`	`52`	`.setDescription(description)`
`53`	`53`	`.setUnit(unit)`
`54`		`- .buildWithCallback(OtelHelper.longMeasurementCallback(observer));`
	`54`	`+ .buildWithCallback(OtelHelper.longMeasurementCallback(name, observer));`
`55`	`55`	`}`
`56`	`56`	`}`
`57`	`57`	`}`
Original file line number	Diff line number	Diff line change
`@@ -39,7 +39,7 @@ public void incrementBy(long inc) {`
`39`	`39`	`@Override`
`40`	`40`	`public void incrementBy(long inc, Map<String, Object> attributes) {`
`41`	`41`	`assert inc >= 0;`
`42`		`- getInstrument().add(inc, OtelHelper.fromMap(attributes));`
	`42`	`+ getInstrument().add(inc, OtelHelper.fromMap(getName(), attributes));`
`43`	`43`	`}`
`44`	`44`
`45`	`45`	`private static class Builder extends AbstractInstrument.Builder<LongCounter> {`
Original file line number	Diff line number	Diff line change
`@@ -47,7 +47,7 @@ public ObservableLongGauge build(Meter meter) {`
`47`	`47`	`.ofLongs()`
`48`	`48`	`.setDescription(description)`
`49`	`49`	`.setUnit(unit)`
`50`		`- .buildWithCallback(OtelHelper.longMeasurementCallback(observer));`
	`50`	`+ .buildWithCallback(OtelHelper.longMeasurementCallback(name, observer));`
`51`	`51`	`}`
`52`	`52`	`}`
`53`	`53`	`}`
Original file line number	Diff line number	Diff line change
`@@ -32,7 +32,7 @@ public void record(long value) {`
`32`	`32`
`33`	`33`	`@Override`
`34`	`34`	`public void record(long value, Map<String, Object> attributes) {`
`35`		`- getInstrument().record(value, OtelHelper.fromMap(attributes));`
	`35`	`+ getInstrument().record(value, OtelHelper.fromMap(getName(), attributes));`
`36`	`36`	`}`
`37`	`37`
`38`	`38`	`private static class Builder extends AbstractInstrument.Builder<LongHistogram> {`