From d6de427b9cee1a0331c27547c98dc9930edd98b5 Mon Sep 17 00:00:00 2001 From: Jonatan Ivanov Date: Wed, 16 Jul 2025 15:42:56 -0700 Subject: [PATCH] Validate low cardinality keys Observations with the same name should have the same set of low cardinality keys --- .../tck/InvalidObservationException.java | 14 +++- .../observation/tck/ObservationValidator.java | 57 ++++++++++--- .../tck/TestObservationRegistry.java | 82 +++++++++++++++++-- .../tck/ObservationValidatorTests.java | 69 ++++++++++++++++ 4 files changed, 200 insertions(+), 22 deletions(-) diff --git a/micrometer-observation-test/src/main/java/io/micrometer/observation/tck/InvalidObservationException.java b/micrometer-observation-test/src/main/java/io/micrometer/observation/tck/InvalidObservationException.java index d95f3c0dc4..b783862a57 100644 --- a/micrometer-observation-test/src/main/java/io/micrometer/observation/tck/InvalidObservationException.java +++ b/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 history; + InvalidObservationException(String message, Context context) { + this(message, context, Collections.emptyList()); + } + InvalidObservationException(String message, Context context, List history) { super(message); this.context = context; @@ -53,8 +58,13 @@ public List 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 { diff --git a/micrometer-observation-test/src/main/java/io/micrometer/observation/tck/ObservationValidator.java b/micrometer-observation-test/src/main/java/io/micrometer/observation/tck/ObservationValidator.java index b075d6a38c..7cd173c4ac 100644 --- a/micrometer-observation-test/src/main/java/io/micrometer/observation/tck/ObservationValidator.java +++ b/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 { private final Predicate supportsContextPredicate; - ObservationValidator() { - this(ObservationValidator::throwInvalidObservationException); - } + private final Map> lowCardinalityKeys; - ObservationValidator(Consumer consumer) { - this(consumer, context -> !(context instanceof NullContext)); - } + private final Set capabilities; - ObservationValidator(Consumer consumer, Predicate supportsContextPredicate) { - this.consumer = consumer; - this.supportsContextPredicate = supportsContextPredicate; + ObservationValidator(Set 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 existingKeys = lowCardinalityKeys.get(context.getName()); + Set 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 getLowCardinalityKeys(Context context) { + return getKeys(context.getLowCardinalityKeyValues()); + } + + private Set 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()); } diff --git a/micrometer-observation-test/src/main/java/io/micrometer/observation/tck/TestObservationRegistry.java b/micrometer-observation-test/src/main/java/io/micrometer/observation/tck/TestObservationRegistry.java index 5caf01cf7e..3aa75330f9 100644 --- a/micrometer-observation-test/src/main/java/io/micrometer/observation/tck/TestObservationRegistry.java +++ b/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,8 +41,8 @@ public final class TestObservationRegistry private final StoringObservationHandler handler = new StoringObservationHandler(); - private TestObservationRegistry() { - observationConfig().observationHandler(this.handler).observationHandler(new ObservationValidator()); + private TestObservationRegistry(Set capabilities) { + observationConfig().observationHandler(this.handler).observationHandler(new ObservationValidator(capabilities)); } /** @@ -51,7 +50,15 @@ private TestObservationRegistry() { * @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 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. + *

+ * Example 1: + *

+ * 

+         * observation{name=test, lowCardinalityKeyValues=[color=red]}
+         * observation{name=test, lowCardinalityKeyValues=[color=green]}
+         * observation{name=test, lowCardinalityKeyValues=[color=blue]}
+         *
+ *

+ * Example 1 is valid since all the observations with the same name ("test") has + * the same set of low cardinality keys ("color"). + *

+ * Example 2: + *

+ * 

+         * observation{name=test, lowCardinalityKeyValues=[color=red]}
+         * observation{name=test, lowCardinalityKeyValues=[]}
+         * observation{name=test, lowCardinalityKeyValues=[status=ok]}
+         *
+ *

+ * 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; diff --git a/micrometer-observation-test/src/test/java/io/micrometer/observation/tck/ObservationValidatorTests.java b/micrometer-observation-test/src/test/java/io/micrometer/observation/tck/ObservationValidatorTests.java index 30c401ef2a..12d26a9675 100644 --- a/micrometer-observation-test/src/test/java/io/micrometer/observation/tck/ObservationValidatorTests.java +++ b/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(); + } + }