Validate low cardinality keys (#6713)

Observations with the same name should have the same set of
low cardinality keys

Author: jonatan-ivanov

micrometer-observation-test/src/main/java/io/micrometer/observation/tck/InvalidObservationException.java

@@ -19,6 +19,7 @@
 import io.micrometer.observation.Observation.Context;
 
 import java.util.Arrays;
+import java.util.Collections;
 import java.util.List;
 import java.util.stream.Collectors;
 
@@ -37,6 +38,10 @@ public class InvalidObservationException extends RuntimeException {
 
     private final List<HistoryElement> history;
 
+    InvalidObservationException(String message, Context context) {
+        this(message, context, Collections.emptyList());
+    }
+
     InvalidObservationException(String message, Context context, List<HistoryElement> history) {
         super(message);
         this.context = context;
@@ -53,8 +58,13 @@ public List<HistoryElement> getHistory() {
 
     @Override
     public String toString() {
-        return super.toString() + "\n"
-                + history.stream().map(HistoryElement::toString).collect(Collectors.joining("\n"));
+        if (history.isEmpty()) {
+            return super.toString();
+        }
+        else {
+            return super.toString() + "\n"
+                    + history.stream().map(HistoryElement::toString).collect(Collectors.joining("\n"));
+        }
     }
 
     public static class HistoryElement {

micrometer-observation-test/src/main/java/io/micrometer/observation/tck/ObservationValidator.java

@@ -15,6 +15,8 @@
  */
 package io.micrometer.observation.tck;
 
+import io.micrometer.common.KeyValue;
+import io.micrometer.common.KeyValues;
 import io.micrometer.observation.NullObservation.NullContext;
 import io.micrometer.observation.Observation.Context;
 import io.micrometer.observation.Observation.Event;
@@ -23,11 +25,13 @@
 import io.micrometer.observation.tck.InvalidObservationException.HistoryElement;
 import org.jspecify.annotations.Nullable;
 
-import java.util.ArrayList;
-import java.util.Collections;
-import java.util.List;
+import java.util.*;
 import java.util.function.Consumer;
 import java.util.function.Predicate;
+import java.util.stream.Collectors;
+
+import static io.micrometer.observation.tck.TestObservationRegistry.Capability;
+import static io.micrometer.observation.tck.TestObservationRegistry.Capability.OBSERVATIONS_WITH_THE_SAME_NAME_SHOULD_HAVE_THE_SAME_SET_OF_LOW_CARDINALITY_KEYS;
 
 /**
  * An {@link ObservationHandler} that validates the order of events of an Observation (for
@@ -43,17 +47,15 @@ class ObservationValidator implements ObservationHandler<Context> {
 
     private final Predicate<Context> supportsContextPredicate;
 
-    ObservationValidator() {
-        this(ObservationValidator::throwInvalidObservationException);
-    }
+    private final Map<String, Set<String>> lowCardinalityKeys;
 
-    ObservationValidator(Consumer<ValidationResult> consumer) {
-        this(consumer, context -> !(context instanceof NullContext));
-    }
+    private final Set<Capability> capabilities;
 
-    ObservationValidator(Consumer<ValidationResult> consumer, Predicate<Context> supportsContextPredicate) {
-        this.consumer = consumer;
-        this.supportsContextPredicate = supportsContextPredicate;
+    ObservationValidator(Set<Capability> capabilities) {
+        this.consumer = ObservationValidator::throwInvalidObservationException;
+        this.supportsContextPredicate = context -> !(context instanceof NullContext);
+        this.lowCardinalityKeys = new HashMap<>();
+        this.capabilities = capabilities;
     }
 
     @Override
@@ -109,6 +111,9 @@ public void onStop(Context context) {
         if (status != null) {
             status.markStopped();
         }
+        if (capabilities.contains(OBSERVATIONS_WITH_THE_SAME_NAME_SHOULD_HAVE_THE_SAME_SET_OF_LOW_CARDINALITY_KEYS)) {
+            checkIfObservationsWithTheSameNameHaveTheSameSetOfLowCardinalityKeys(context);
+        }
     }
 
     @Override
@@ -141,8 +146,34 @@ private void addHistoryElement(Context context, EventName eventName) {
         return status;
     }
 
+    private void checkIfObservationsWithTheSameNameHaveTheSameSetOfLowCardinalityKeys(Context context) {
+        if (lowCardinalityKeys.containsKey(context.getName())) {
+            Set<String> existingKeys = lowCardinalityKeys.get(context.getName());
+            Set<String> currentKeys = getLowCardinalityKeys(context);
+            if (!existingKeys.equals(currentKeys)) {
+                String message = "Metrics backends may require that all observations with the same name have the same"
+                        + " set of low cardinality keys. There is already an existing observation named '"
+                        + context.getName() + "' containing keys [" + String.join(", ", existingKeys)
+                        + "]. The observation you are attempting to register" + " has keys ["
+                        + String.join(", ", currentKeys) + "].";
+                throw new InvalidObservationException(message, context);
+            }
+        }
+        else {
+            lowCardinalityKeys.put(Objects.requireNonNull(context.getName()), getLowCardinalityKeys(context));
+        }
+    }
+
+    private Set<String> getLowCardinalityKeys(Context context) {
+        return getKeys(context.getLowCardinalityKeyValues());
+    }
+
+    private Set<String> getKeys(KeyValues keyValues) {
+        return keyValues.stream().map(KeyValue::getKey).collect(Collectors.toSet());
+    }
+
     private static void throwInvalidObservationException(ValidationResult validationResult) {
-        History history = validationResult.getContext().getOrDefault(History.class, () -> new History());
+        History history = validationResult.getContext().getOrDefault(History.class, History::new);
         throw new InvalidObservationException(validationResult.getMessage(), validationResult.getContext(),
                 history.getHistoryElements());
     }

micrometer-observation-test/src/main/java/io/micrometer/observation/tck/TestObservationRegistry.java

@@ -21,12 +21,11 @@
 import org.assertj.core.api.AssertProvider;
 import org.jspecify.annotations.Nullable;
 
-import java.util.HashSet;
-import java.util.Objects;
-import java.util.Queue;
-import java.util.Set;
+import java.util.*;
 import java.util.concurrent.ConcurrentLinkedQueue;
 
+import static io.micrometer.observation.tck.TestObservationRegistry.Capability.OBSERVATIONS_WITH_THE_SAME_NAME_SHOULD_HAVE_THE_SAME_SET_OF_LOW_CARDINALITY_KEYS;
+
 /**
  * Implementation of {@link ObservationRegistry} used for testing.
  *
@@ -42,16 +41,24 @@ public final class TestObservationRegistry
 
     private final StoringObservationHandler handler = new StoringObservationHandler();
 
-    private TestObservationRegistry() {
-        observationConfig().observationHandler(this.handler).observationHandler(new ObservationValidator());
+    private TestObservationRegistry(Set<Capability> capabilities) {
+        observationConfig().observationHandler(this.handler).observationHandler(new ObservationValidator(capabilities));
     }
 
     /**
      * Crates a new instance of mock observation registry.
      * @return mock instance of observation registry
      */
     public static TestObservationRegistry create() {
-        return new TestObservationRegistry();
+        return builder().build();
+    }
+
+    /**
+     * @return builder to create {@link TestObservationRegistry}.
+     * @since 1.16.0
+     */
+    public static TestObservationRegistryBuilder builder() {
+        return new TestObservationRegistryBuilder();
     }
 
     @Override
@@ -130,6 +137,67 @@ public void onEvent(Observation.Event event, Observation.Context context) {
 
     }
 
+    /**
+     * Builder to create {@link TestObservationRegistry}.
+     *
+     * @since 1.16.0
+     */
+    public static class TestObservationRegistryBuilder {
+
+        private final Set<Capability> capabilities = new HashSet<>(Arrays.asList(Capability.values()));
+
+        /**
+         * Enables/disables validating that Observations with the same name should have
+         * the same set of low cardinality keys.
+         * <p>
+         * Example 1:
+         * <p>
+         * <pre>
+         * observation{name=test, lowCardinalityKeyValues=[color=red]}
+         * observation{name=test, lowCardinalityKeyValues=[color=green]}
+         * observation{name=test, lowCardinalityKeyValues=[color=blue]}
+         * </pre>
+         * <p>
+         * Example 1 is valid since all the observations with the same name ("test") has
+         * the same set of low cardinality keys ("color").
+         * <p>
+         * Example 2:
+         * <p>
+         * <pre>
+         * observation{name=test, lowCardinalityKeyValues=[color=red]}
+         * observation{name=test, lowCardinalityKeyValues=[]}
+         * observation{name=test, lowCardinalityKeyValues=[status=ok]}
+         * </pre>
+         * <p>
+         * Example 2 is invalid since the second observation is missing a key the first
+         * one has ("color") and the third observation is not only missing a key the first
+         * one has ("color") but it also has an extra key the first one does not have
+         * ("status").
+         */
+        public TestObservationRegistryBuilder validateObservationsWithTheSameNameHavingTheSameSetOfLowCardinalityKeys(
+                boolean flag) {
+            if (flag) {
+                this.capabilities.add(OBSERVATIONS_WITH_THE_SAME_NAME_SHOULD_HAVE_THE_SAME_SET_OF_LOW_CARDINALITY_KEYS);
+            }
+            else {
+                this.capabilities
+                    .remove(OBSERVATIONS_WITH_THE_SAME_NAME_SHOULD_HAVE_THE_SAME_SET_OF_LOW_CARDINALITY_KEYS);
+            }
+            return this;
+        }
+
+        public TestObservationRegistry build() {
+            return new TestObservationRegistry(capabilities);
+        }
+
+    }
+
+    enum Capability {
+
+        OBSERVATIONS_WITH_THE_SAME_NAME_SHOULD_HAVE_THE_SAME_SET_OF_LOW_CARDINALITY_KEYS;
+
+    }
+
     static class TestObservationContext {
 
         private final Observation.Context context;

micrometer-observation-test/src/test/java/io/micrometer/observation/tck/ObservationValidatorTests.java

@@ -234,4 +234,73 @@ void nullObservationShouldBeIgnored() {
         new NullObservation(registry).openScope();
     }
 
+    @Test
+    void capabilitiesCanBeDisabledUsingTheBuilder() {
+        TestObservationRegistry registry = TestObservationRegistry.builder()
+            .validateObservationsWithTheSameNameHavingTheSameSetOfLowCardinalityKeys(false)
+            .build();
+        Observation.createNotStarted("test", registry).lowCardinalityKeyValue("key1", "value1").start().stop();
+        Observation.createNotStarted("test", registry).start().stop();
+        Observation.createNotStarted("test", registry).lowCardinalityKeyValue("key2", "value2").start().stop();
+    }
+
+    @Test
+    void observationsWithTheSameNameShouldHaveTheSameSetOfLowCardinalityKeysByDefaultUsingCreate() {
+        TestObservationRegistry registry = TestObservationRegistry.create();
+        Observation.createNotStarted("test", registry).lowCardinalityKeyValue("key1", "value1").start().stop();
+        assertThatThrownBy(() -> {
+            Observation.createNotStarted("test", registry).start().stop();
+        }).isExactlyInstanceOf(InvalidObservationException.class)
+            .hasMessageContaining(
+                    "Metrics backends may require that all observations with the same name have the same set of low cardinality keys.");
+
+        assertThatThrownBy(() -> {
+            Observation.createNotStarted("test", registry).lowCardinalityKeyValue("key2", "value2").start().stop();
+        }).isExactlyInstanceOf(InvalidObservationException.class)
+            .hasMessageContaining(
+                    "Metrics backends may require that all observations with the same name have the same set of low cardinality keys.");
+
+        Observation.createNotStarted("test", registry).lowCardinalityKeyValue("key1", "value2").start().stop();
+    }
+
+    @Test
+    void observationsWithTheSameNameShouldHaveTheSameSetOfLowCardinalityKeysByDefaultUsingTheBuilder() {
+        TestObservationRegistry registry = TestObservationRegistry.builder().build();
+        Observation.createNotStarted("test", registry).lowCardinalityKeyValue("key1", "value1").start().stop();
+        assertThatThrownBy(() -> {
+            Observation.createNotStarted("test", registry).start().stop();
+        }).isExactlyInstanceOf(InvalidObservationException.class)
+            .hasMessageContaining(
+                    "Metrics backends may require that all observations with the same name have the same set of low cardinality keys.");
+
+        assertThatThrownBy(() -> {
+            Observation.createNotStarted("test", registry).lowCardinalityKeyValue("key2", "value2").start().stop();
+        }).isExactlyInstanceOf(InvalidObservationException.class)
+            .hasMessageContaining(
+                    "Metrics backends may require that all observations with the same name have the same set of low cardinality keys.");
+
+        Observation.createNotStarted("test", registry).lowCardinalityKeyValue("key1", "value2").start().stop();
+    }
+
+    @Test
+    void observationsWithTheSameNameShouldHaveTheSameSetOfLowCardinalityKeysWhenEnabledUsingTheBuilder() {
+        TestObservationRegistry registry = TestObservationRegistry.builder()
+            .validateObservationsWithTheSameNameHavingTheSameSetOfLowCardinalityKeys(true)
+            .build();
+        Observation.createNotStarted("test", registry).lowCardinalityKeyValue("key1", "value1").start().stop();
+        assertThatThrownBy(() -> {
+            Observation.createNotStarted("test", registry).start().stop();
+        }).isExactlyInstanceOf(InvalidObservationException.class)
+            .hasMessageContaining(
+                    "Metrics backends may require that all observations with the same name have the same set of low cardinality keys.");
+
+        assertThatThrownBy(() -> {
+            Observation.createNotStarted("test", registry).lowCardinalityKeyValue("key2", "value2").start().stop();
+        }).isExactlyInstanceOf(InvalidObservationException.class)
+            .hasMessageContaining(
+                    "Metrics backends may require that all observations with the same name have the same set of low cardinality keys.");
+
+        Observation.createNotStarted("test", registry).lowCardinalityKeyValue("key1", "value2").start().stop();
+    }
+
 }