Deny usage of metric attributes known to cause issues. (elastic#136478)

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

Author: mosche

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

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<String> 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<String, Object> 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 `-`

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<String, Object> 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) {

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",

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"));
+    }
 }