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..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 @@ -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,22 @@ 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", + // 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", + "*.created", + "*.creation_date", + "ingested", + "*.ingested", + "*.start", + "*.end" + ); + private MetricNameValidator() {} /** @@ -65,6 +86,20 @@ 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 due to potential mapping conflicts or assumed high cardinality", 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..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 @@ -101,7 +101,26 @@ 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("index", "index1"))); + + 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..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 @@ -118,4 +118,23 @@ 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("index", "index1"))); + + 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")); + } }