From 74bd1dbb3bb2ae60ca91755003c816bb1710ad19 Mon Sep 17 00:00:00 2001 From: Moritz Mack Date: Mon, 13 Oct 2025 15:14:23 +0200 Subject: [PATCH 1/5] Deny usage of metric attributes known to cause issues. Due to their mapping or cardinality, certain metric attributes are likely to cause issues. Such attribute names are denied by a new assertion Relates to ES-13074 --- modules/apm/NAMING.md | 2 ++ .../apm/internal/MetricNameValidator.java | 33 +++++++++++++++++++ .../apm/internal/metrics/OtelHelper.java | 2 ++ .../metrics/AsyncCountersAdapterTests.java | 26 ++++++++++++++- .../internal/metrics/GaugeAdapterTests.java | 24 ++++++++++++++ 5 files changed, 86 insertions(+), 1 deletion(-) diff --git a/modules/apm/NAMING.md b/modules/apm/NAMING.md index 31cad34d0470a..5224a3541ccd7 100644 --- a/modules/apm/NAMING.md +++ b/modules/apm/NAMING.md @@ -64,6 +64,8 @@ Examples : ### Attributes +Do not use **high cardinality** attributes, this might result in the APM Java agent dropping events. + Attribute names should follow the same rules. In particular, these rules apply to attributes too: * elements and separators * hierarchy/namespaces diff --git a/modules/apm/src/main/java/org/elasticsearch/telemetry/apm/internal/MetricNameValidator.java b/modules/apm/src/main/java/org/elasticsearch/telemetry/apm/internal/MetricNameValidator.java index bc5665bedcf12..3ebe2aafc3ce8 100644 --- a/modules/apm/src/main/java/org/elasticsearch/telemetry/apm/internal/MetricNameValidator.java +++ b/modules/apm/src/main/java/org/elasticsearch/telemetry/apm/internal/MetricNameValidator.java @@ -9,8 +9,13 @@ package org.elasticsearch.telemetry.apm.internal; +import org.apache.logging.log4j.LogManager; +import org.elasticsearch.common.regex.Regex; + +import java.util.Map; import java.util.Objects; import java.util.Set; +import java.util.function.Predicate; import java.util.regex.Matcher; import java.util.regex.Pattern; import java.util.stream.Collectors; @@ -40,6 +45,21 @@ public class MetricNameValidator { "security-crypto" ); + // forbidden attributes known to cause issues due to mapping conflicts or high cardinality + static final Predicate FORBIDDEN_ATTRIBUTE_NAMES = Regex.simpleMatcher( + "index", + "*_id", + "*.timestamp", + "*_timestamp", + "created", + "*.created", + "*.creation_date", + "ingested", + "*.ingested", + "*.start", + "*.end" + ); + private MetricNameValidator() {} /** @@ -65,6 +85,19 @@ public static String validate(String metricName) { return metricName; } + public static boolean validateAttributeNames(Map attributes) { + if (attributes == null && attributes.isEmpty()) { + return true; + } + for (String attribute : attributes.keySet()) { + if (FORBIDDEN_ATTRIBUTE_NAMES.test(attribute)) { + LogManager.getLogger(MetricNameValidator.class).warn("Attribute name [{}] is forbidden", attribute); + return false; + } + } + return true; + } + /** * Due to backwards compatibility some metric names would have to skip validation. * This is for instance where a threadpool name is too long, or contains `-` 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 index ea2a921c06fb6..f46bd76883de1 100644 --- 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 @@ -15,6 +15,7 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; +import org.elasticsearch.telemetry.apm.internal.MetricNameValidator; import org.elasticsearch.telemetry.metric.DoubleWithAttributes; import org.elasticsearch.telemetry.metric.LongWithAttributes; @@ -30,6 +31,7 @@ static Attributes fromMap(Map attributes) { if (attributes == null || attributes.isEmpty()) { return Attributes.empty(); } + assert MetricNameValidator.validateAttributeNames(attributes) : "invalid metric attributes"; var builder = Attributes.builder(); attributes.forEach((k, v) -> { if (v instanceof String value) { 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 index 74b361c8e7a9e..4e59ef4e0d34b 100644 --- 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 @@ -101,7 +101,31 @@ public void testDoubleAsyncAdapter() throws Exception { assertThat(metrics, hasSize(0)); } - public void testNullGaugeRecord() throws Exception { + public void testLongWithInvalidAttribute() { + registry.registerLongAsyncCounter( + "es.test.name.total", + "desc", + "unit", + () -> new LongWithAttributes(1, Map.of("high_cardinality_id", "27932451")) + ); + + AssertionError error = assertThrows(AssertionError.class, otelMeter::collectMetrics); + assertThat(error.getMessage(), equalTo("invalid metric attributes")); + } + + public void testDoubleWithInvalidAttribute() { + registry.registerDoubleAsyncCounter( + "es.test.name.total", + "desc", + "unit", + () -> new DoubleWithAttributes(1.0, Map.of("has_timestamp", "false")) + ); + + AssertionError error = assertThrows(AssertionError.class, otelMeter::collectMetrics); + assertThat(error.getMessage(), equalTo("invalid metric attributes")); + } + + public void testNullRecord() throws Exception { DoubleAsyncCounter dcounter = registry.registerDoubleAsyncCounter( "es.test.name.total", "desc", 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 index 49ba9de1aaed3..6e043e8707d9a 100644 --- 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 @@ -118,4 +118,28 @@ public void testNullGaugeRecord() throws Exception { metrics = otelMeter.getRecorder().getMeasurements(lgauge); assertThat(metrics, hasSize(0)); } + + public void testLongGaugeWithInvalidAttribute() { + registry.registerLongGauge( + "es.test.name.total", + "desc", + "unit", + () -> new LongWithAttributes(1, Map.of("high_cardinality_id", "27932451")) + ); + + AssertionError error = assertThrows(AssertionError.class, otelMeter::collectMetrics); + assertThat(error.getMessage(), equalTo("invalid metric attributes")); + } + + public void testDoubleGaugeWithInvalidAttribute() { + registry.registerDoubleGauge( + "es.test.name.total", + "desc", + "unit", + () -> new DoubleWithAttributes(1.0, Map.of("has_timestamp", "false")) + ); + + AssertionError error = assertThrows(AssertionError.class, otelMeter::collectMetrics); + assertThat(error.getMessage(), equalTo("invalid metric attributes")); + } } From 5fa8d39d685bdb340028baef6daf495f30bd81b7 Mon Sep 17 00:00:00 2001 From: Moritz Mack Date: Mon, 13 Oct 2025 15:15:52 +0200 Subject: [PATCH 2/5] log message --- .../telemetry/apm/internal/MetricNameValidator.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/modules/apm/src/main/java/org/elasticsearch/telemetry/apm/internal/MetricNameValidator.java b/modules/apm/src/main/java/org/elasticsearch/telemetry/apm/internal/MetricNameValidator.java index 3ebe2aafc3ce8..c6e94753443cf 100644 --- a/modules/apm/src/main/java/org/elasticsearch/telemetry/apm/internal/MetricNameValidator.java +++ b/modules/apm/src/main/java/org/elasticsearch/telemetry/apm/internal/MetricNameValidator.java @@ -91,7 +91,8 @@ public static boolean validateAttributeNames(Map attributes) { } for (String attribute : attributes.keySet()) { if (FORBIDDEN_ATTRIBUTE_NAMES.test(attribute)) { - LogManager.getLogger(MetricNameValidator.class).warn("Attribute name [{}] is forbidden", attribute); + LogManager.getLogger(MetricNameValidator.class) + .warn("Attribute name [{}] is forbidden due to potential mapping conflicts or assumed high cardinality", attribute); return false; } } From 39ce633629ce93d2454b078b7ef0d93f6c739b99 Mon Sep 17 00:00:00 2001 From: Moritz Mack Date: Mon, 13 Oct 2025 16:11:57 +0200 Subject: [PATCH 3/5] remove *_id --- .../telemetry/apm/internal/MetricNameValidator.java | 1 - 1 file changed, 1 deletion(-) diff --git a/modules/apm/src/main/java/org/elasticsearch/telemetry/apm/internal/MetricNameValidator.java b/modules/apm/src/main/java/org/elasticsearch/telemetry/apm/internal/MetricNameValidator.java index c6e94753443cf..91b86be1a7725 100644 --- a/modules/apm/src/main/java/org/elasticsearch/telemetry/apm/internal/MetricNameValidator.java +++ b/modules/apm/src/main/java/org/elasticsearch/telemetry/apm/internal/MetricNameValidator.java @@ -48,7 +48,6 @@ public class MetricNameValidator { // forbidden attributes known to cause issues due to mapping conflicts or high cardinality static final Predicate FORBIDDEN_ATTRIBUTE_NAMES = Regex.simpleMatcher( "index", - "*_id", "*.timestamp", "*_timestamp", "created", From 7d3ae93736304db0d051f300a430e54c1e995a59 Mon Sep 17 00:00:00 2001 From: Moritz Mack Date: Tue, 14 Oct 2025 08:21:35 +0200 Subject: [PATCH 4/5] fix tests after removing *_id --- .../telemetry/apm/internal/MetricNameValidator.java | 2 ++ .../apm/internal/metrics/AsyncCountersAdapterTests.java | 2 +- .../telemetry/apm/internal/metrics/GaugeAdapterTests.java | 2 +- 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/modules/apm/src/main/java/org/elasticsearch/telemetry/apm/internal/MetricNameValidator.java b/modules/apm/src/main/java/org/elasticsearch/telemetry/apm/internal/MetricNameValidator.java index 91b86be1a7725..10133157426cb 100644 --- a/modules/apm/src/main/java/org/elasticsearch/telemetry/apm/internal/MetricNameValidator.java +++ b/modules/apm/src/main/java/org/elasticsearch/telemetry/apm/internal/MetricNameValidator.java @@ -48,6 +48,8 @@ public class MetricNameValidator { // forbidden attributes known to cause issues due to mapping conflicts or high cardinality static final Predicate FORBIDDEN_ATTRIBUTE_NAMES = Regex.simpleMatcher( "index", + // below field names are typically mapped to a timestamp risking mapping errors at ingest time + // if values are not valid timestamps (which would be of high cardinality, and not desired either) "*.timestamp", "*_timestamp", "created", 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 index 4e59ef4e0d34b..bd33b44c28b84 100644 --- 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() { "es.test.name.total", "desc", "unit", - () -> new LongWithAttributes(1, Map.of("high_cardinality_id", "27932451")) + () -> new LongWithAttributes(1, Map.of("index", "index1")) ); AssertionError error = assertThrows(AssertionError.class, otelMeter::collectMetrics); 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 index 6e043e8707d9a..d01fbc8bd262f 100644 --- 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() { "es.test.name.total", "desc", "unit", - () -> new LongWithAttributes(1, Map.of("high_cardinality_id", "27932451")) + () -> new LongWithAttributes(1, Map.of("index", "index1")) ); AssertionError error = assertThrows(AssertionError.class, otelMeter::collectMetrics); From 51cb0f143e078e9fc294a2674bfdd4a1def2500b Mon Sep 17 00:00:00 2001 From: elasticsearchmachine Date: Tue, 14 Oct 2025 06:28:39 +0000 Subject: [PATCH 5/5] [CI] Auto commit changes from spotless --- .../apm/internal/metrics/AsyncCountersAdapterTests.java | 7 +------ .../telemetry/apm/internal/metrics/GaugeAdapterTests.java | 7 +------ 2 files changed, 2 insertions(+), 12 deletions(-) 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 index bd33b44c28b84..7f1bf676e90db 100644 --- 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 @@ -102,12 +102,7 @@ public void testDoubleAsyncAdapter() throws Exception { } public void testLongWithInvalidAttribute() { - registry.registerLongAsyncCounter( - "es.test.name.total", - "desc", - "unit", - () -> new LongWithAttributes(1, Map.of("index", "index1")) - ); + 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(), equalTo("invalid metric attributes")); 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 index d01fbc8bd262f..cac36c9038ea7 100644 --- 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 @@ -120,12 +120,7 @@ public void testNullGaugeRecord() throws Exception { } public void testLongGaugeWithInvalidAttribute() { - registry.registerLongGauge( - "es.test.name.total", - "desc", - "unit", - () -> new LongWithAttributes(1, Map.of("index", "index1")) - ); + 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(), equalTo("invalid metric attributes"));