Add KeyValues to Observations using annotations #6667

isanghaessi · 2025-08-27T03:27:46Z

Summary

Add parameter-based tagging via @ObservedKeyValueTag(s) and handler support (#5826) with AOP.
Because there's inconvenience making span with tags like below.

public List<Recipes> findRecipes(
            String type,
            List<UUID> ownerIds,
            Boolean includeOwner,
            Boolean includeTested,
            String sort,
            Integer limit
) {
        KeyValues lowCardinalityKeyValues = KeyValues.of(
                "type", String.valueOf(type),
                "includeOwner", String.valueOf(includeOwner),
                "includeTested", String.valueOf(includeTested),
                "sort", String.valueOf(sort),
                "limit", String.valueOf(limit)
        );

        return Observation.createNotStarted("test", observationRegistry)
                .contextualName("findRecipes")
                .lowCardinalityKeyValues(lowCardinalityKeyValues)
                .highCardinalityKeyValue("ownerIds", String.valueOf(ownerIds))
                .observe(() -> recipeDao.findRecipes(
                        type,
                        ownerIds,
                        includeTested,
                        sort,
                        limit
                ));
    }

This PR introduces parameter-based tag (key-value) support for @observed by adding:

New annotations: @ObservedKeyValueTag and @ObservedKeyValueTags.
AOP support: ObservedKeyValueTagAnnotationHandler and related support classes.
ObservedAspect extension: setter to inject the tag annotation handler.
Common utilities: ExtendedImmutableKeyValue for KeyValue class.
Tests

And, changing some build.gradle.

Add preferring local module setting, because of conflicts in remote and local module in developing.
Remove unnecessary dependency in test module.

This brings parity with the developer experience of @MeterTag used in Counted/Timed.
It's just like cherrypick parameter-tag-binding feature of @MeterTag.

Changes

API
- Add @ObservedKeyValueTag and @ObservedKeyValueTags (container) annotations.
- Support static values, ValueResolver-based evaluation, and ValueExpressionResolver-based evaluation.
- Allow multiple annotations on the same parameter for multiple tags(no duplicate key).
AOP
- Add ObservedKeyValueTagAnnotationHandler to scan method parameters and attach key-values to Observation.Context
- Extend ObservedAspect with setObservedKeyValueTagAnnotationHandler(...) to enable the feature when configured.
Common
- Enhance key-value construction logic to streamline propagation and immutability where appropriate.
Tests
- Update tests to cover static, resolver, and expression-based tagging scenarios.

How to use(Same as @TImed/@MeterTag)

Wire the handler into ObservedAspect

Enable the feature by setting ObservedKeyValueTagAnnotationHandler on your ObservedAspect. Provide factories for ValueResolver and ValueExpressionResolver if you need dynamic values.

// Java
@Configuration
class ObservationConfig {

    @Bean
    ObservedAspect observedAspect(ObservationRegistry registry) {
        ObservedAspect aspect = new ObservedAspect(registry);
        aspect.setObservedKeyValueTagAnnotationHandler(
            new ObservedKeyValueTagAnnotationHandler(
                clazz -> new MyValueResolver(),          // ValueResolver factory
                clazz -> new MyExpressionResolver()      // ValueExpressionResolver factory
            )
        );
        return aspect;
    }
}

// Example resolvers
final class MyValueResolver implements io.micrometer.common.annotation.ValueResolver {
    @Override
    public String resolve(Object parameter) {
        return parameter == null ? "null" : "vr:" + parameter;
    }
}

final class MyExpressionResolver implements io.micrometer.common.annotation.ValueExpressionResolver {
    @Override
    public String resolve(String expression, Object parameter) {
        // Evaluate expression with your engine of choice; this is a simple demo
        return "expr:" + expression;
    }
}

Annotate method parameters

You can derive the tag value directly from the parameter, via a custom ValueResolver, or via a ValueExpressionResolver-based expression. Multiple tags can be attached to the same parameter.

// Java
class MyService {

    @Observed(name = "my.operation")
    public void process(
        // Key-only: derives value from parameter (toString / null-safe)
        @ObservedKeyValueTag(key = "customerId") String customerId,

        // Static value (if supported) or expression-based via ValueExpressionResolver
        @ObservedKeyValueTag(key = "kind", expression = "'type:' + dtoType") RequestDto dto,

        // Custom resolver-based
        @ObservedKeyValueTag(key = "status", resolver = MyStatusValueResolver.class) int statusCode
    ) {
        // ...
    }

    // Multiple tags from the same parameter (repeatable)
    @Observed(name = "my.batch")
    public void batch(
        @ObservedKeyValueTag(key = "itemId")
        @ObservedKeyValueTag(key = "itemType", expression = "'type:' + type")
        Item item
    ) {
        // ...
    }

    // Container annotation also supported
    @Observed(name = "my.container")
    public void container(
        @ObservedKeyValueTags({
            @ObservedKeyValueTag(key = "a"),
            @ObservedKeyValueTag(key = "b", expression = "'b:' + value")
        }) Data data
    ) {
        // ...
    }
}

Evaluation order and fallbacks(Same as @MeterTag)

If a resolver class is specified, it takes precedence and is used to compute the value.
Else if an expression is specified, the ValueExpressionResolver is used to compute the value.
Else the value falls back to the parameter’s toString(); if null, it becomes "null".
If resolver/expression evaluation throws, the implementation logs the error and falls back to the parameter-based value to keep Observation running.

Support for async/CompletionStage

The aspect handles methods returning CompletionStage by starting the Observation before proceeding and stopping it upon completion (or error). Parameter-based tags are attached at Observation start and remain unaffected by the async boundary.

Compatibility

Existing @Observed-based instrumentation continues to work without modification.

Review Points

API naming consistency (@ObservedKeyValueTag, @ObservedKeyValueTags) and attribute names.
Appropriateness of adding ImmutableExtendedKeyValue.
Test coverage depth and representative cases.

Reference

Closes #5826.

shakuzen · 2025-09-01T08:14:46Z

Thank you for the detailed pull request.

Appropriateness of adding ImmutableExtendedKeyValue.

I haven't had a chance to look closely at the changes and consider alternatives, but my initial reaction is I would like to avoid adding this if it is feasible to avoid it.

We'll take a closer look later and give a proper review, but I wanted to give that feedback in case you already have ideas of another good way to implement this.

isanghaessi · 2025-09-01T17:57:04Z

Thanks for opinion @shakuzen.
I changed the code as an alternative and removed ImmutableExtendedKeyValue.
And add some docs.
Please, take a look leter! I'm waiting.

micopiira · 2025-09-09T15:04:08Z

Do you think it would be possible to add support for deriving tags from the returned object using annotations?

isanghaessi · 2025-09-10T15:17:39Z

Yes, I think it's possible to add.
I made it with reference to the logic of @MeterTag, but it was not included in this issue, so I excluded it and applied it.
Do you think this is necessary logic?
If it is needed to add, I can add it.
@shakuzen, @jonatan-ivanov.

shakuzen

Thanks for the PR, it looks very complete. I left some comments. I see you changed from the ImmutableExtendedKeyValue approach - thanks for that. I think the current solution is better, but I'm still wondering if somehow there's a better way to handle this. I don't have a good idea right now, but I'll think on it some more.

build.gradle

...eter-observation/src/main/java/io/micrometer/observation/annotation/ObservedKeyValueTag.java

...eter-observation/src/main/java/io/micrometer/observation/aop/ObservedKeyValueTagSupport.java

shakuzen · 2025-09-11T07:28:43Z

Do you think it would be possible to add support for deriving tags from the returned object using annotations?

Sure, I think it's possible. From the perspective of making progress, it may be faster to get this PR merged without that feature and follow up with another pull request to add that functionality, though.

isanghaessi · 2025-09-11T13:44:26Z

Thanks for the PR, it looks very complete. I left some comments. I see you changed from the ImmutableExtendedKeyValue approach - thanks for that. I think the current solution is better, but I'm still wondering if somehow there's a better way to handle this. I don't have a good idea right now, but I'll think on it some more.

Very thanks for careful review and opinion @shakuzen!
I left a comments and pushed some changes.

remove 'tag' from '...observedkeyvaluetag....
remove unnecessary public access modifier.

docs/src/test/java/io/micrometer/docs/SpelValueExpressionResolver.java

micrometer-commons/src/main/java/io/micrometer/common/annotation/AnnotationHandler.java

docs/modules/ROOT/pages/observation/components.adoc

build.gradle

micrometer-observation/src/main/java/io/micrometer/observation/aop/ObservedAspect.java

samples/micrometer-samples-spring-framework6/build.gradle

jonatan-ivanov · 2025-09-16T21:34:04Z

micrometer-commons/src/main/java/io/micrometer/common/annotation/AnnotationHandler.java


    private final BiFunction<Annotation, Object, KeyValue> toKeyValue;

+    private final BiPredicate<Annotation, Object> validToAdd;


I think we should not do this, see my comment on ObservedKeyValueAnnotationHandler.

jonatan-ivanov · 2025-09-16T21:47:57Z

...servation/src/main/java/io/micrometer/observation/aop/ObservedKeyValueAnnotationHandler.java

+        this.highCardinalityAnnotationHandler = new AnnotationHandler<>(
+                (keyValue, context) -> context.addHighCardinalityKeyValue(keyValue), resolverProvider,
+                expressionResolverProvider, ObservedKeyValue.class, (annotation, object) -> {
+                    ObservedKeyValue observedKeyValue = (ObservedKeyValue) annotation;
+
+                    return KeyValue.of(ObservedKeyValueSupport.resolveTagKey(observedKeyValue), ObservedKeyValueSupport
+                        .resolveTagValue(observedKeyValue, object, resolverProvider, expressionResolverProvider));
+                }, (annotation, object) -> {
+                    if (annotation.annotationType() != ObservedKeyValue.class) {
+                        return false;
+                    }
+
+                    ObservedKeyValue observedKeyValue = (ObservedKeyValue) annotation;
+                    if (observedKeyValue.cardinality() != CardinalityType.HIGH) {
+                        return false;
+                    }
+
+                    return true;
+                });
+        this.lowCardinalityAnnotationHandler = new AnnotationHandler<>(
+                (keyValue, context) -> context.addLowCardinalityKeyValue(keyValue), resolverProvider,
+                expressionResolverProvider, ObservedKeyValue.class, (annotation, object) -> {
+                    ObservedKeyValue observedKeyValue = (ObservedKeyValue) annotation;


This is not very readable, I think refactoring these two out to separate private static inner classes would help.
But before we would try that I think it would make sense to check if this is the best solution for routing low and high cardinality keyvalues.

As far as I can tell the only difference between the two implementations here are

addLowCardinalityKeyValue/addHighCardinalityKeyValue in the keyValueConsumer (first line) and CardinalityType.LOW/CardinalityType.HIGH in the validToAdd BiPredicate. I'm wondering if it would make sense to skip the BiPredicate entirely and do this decision in the keyValueConsumer: pass the cardinality information and the context in an object that also contains the context.
Does this make sense? Please let me know if not/you want me to help/write some code to show what I'm thinking (at this point I haven't tried but it seems possible).

I understood your opinion.
I think that way seems to be risky.
That was my first approach to implement this issue.

There's problem with that way.
keyValueConsumer has this signature BiConsumer<KeyValue, T>.
And, KeyValue has only String key, String value.

public interface KeyValue extends Comparable<KeyValue> { /** * Use this if you want to indicate that the value is missing. */ String NONE_VALUE = "none"; String getKey(); String getValue(); ...

We have to do something(can make side effect) to pass CardinalityType information.

Change signature of keyValueConsumer in AnnotationHandler.
It's likely to cause problems at implementations of AnnotationHandler, so it doesn't seem like a good way.

Extend KeyValue and pass variable.
I tried this.
Make some ExtendedKeyValue like this.

class ExtendedKeyValue<T> extends KeyValue { T extended; ... }

But, this is weired.
Because we have to cast KeyValue to ExtendedKeyValue.
The structure in which an interface abstracting motion should be cast and used as a child class does not seem good.

So, I decided to separate two handlers and combine.

However, I accept that it's not easy to read.
I will try to make it better to read.
But, I don't get it how to refactor these two handers to static inner classes.
There's resolverProvider, expressionResolverProvider that makes these handlers stateful.
I think static inner classes are not stateful.

micrometer-observation/src/main/java/io/micrometer/observation/aop/ObservedKeyValueSupport.java

jonatan-ivanov · 2025-09-16T21:55:13Z

This is great, thank you very much!
I left a few comments, please let us know what you think. Also, please let us know if you need any clarification/help.

isanghaessi · 2025-09-17T20:27:44Z

Really thanks for very detail review @jonatan-ivanov!
I followed most of your comments.

Heres what I followed(left 👍 emojis).

And, heres what still in conversation.

isanghaessi · 2025-09-24T17:58:32Z

And, I'm watching why only jdk17 test fails.
I sitll don't know why. In my environment with jdk17(temurin, open jdk) works fine.
If there's any information about that, please let me know.

jonatan-ivanov · 2025-09-24T20:52:24Z

And, I'm watching why only jdk17 test fails.
I sitll don't know why. In my environment with jdk17(temurin, open jdk) works fine.
If there's any information about that, please let me know.

If you click on "view details" of the failure, CI says this:

Gradle build daemon disappeared unexpectedly (it may have been killed or may have crashed)

This means that probably there is nothing wrong with the PR, CI killed the build process for some reason (unfortunately it happens sometime, not sure why). I triggered a re-run, it should be fine soon.

micrometer-commons/src/main/java/io/micrometer/common/annotation/AnnotationHandler.java

...eter-observation/src/main/java/io/micrometer/observation/aop/ObservationKeyValueSupport.java

micrometer-observation/src/main/java/io/micrometer/observation/aop/ObservedAspect.java

jonatan-ivanov · 2025-10-09T01:35:28Z

Hi,

I pushed some changes to the PR, most of the commits are polishing things except one that changes the way low- and high cardinality information is passed.Connected to this discussion: #6667 (comment)

Please notice that this commit rolled back the changes in AnnotationHandler so we don't need to introduce any new concepts there (validToAdd) and the approach I took did not need surfacing components publicly, everything is a private implementation detail.

Please let us know what you think.

isanghaessi · 2025-10-09T03:26:41Z

Hi @jonatan-ivanov,

First, so thanks for polishing in detail my codes!
But, I think introduce new concept validToAdd seems better.

Because of down casting.
We have to do down casting to avoid introducing new concept 'validToAdd'.
I know that 'down casting' is pattern to avoid.

    private static void addKeyValue(KeyValue keyValue, Observation.Context context) {
        if (keyValue instanceof KeyValueWithCardinality) {
            if (((KeyValueWithCardinality) keyValue).cardinality == CardinalityType.LOW) {
                context.addLowCardinalityKeyValue(keyValue);
            }
            else {
                context.addHighCardinalityKeyValue(keyValue);
            }
        }
    }

Above the code, KeyValue is interface to represent getKey() and getValue() method. But, we use keyValue.cardinality. It seems miss-using interface that means not designed properly.
If there's only one way to implement, we have to do down casting. But, we can avoid this pattern to introduce new concept 'vlaidToAdd. AnnotatedHandleris designed forKeyValue. But, we need Cardinalityto processObservationKeyValue. Then, it seems right to add some logic to AnnotatedHandler. Changing the existing code that can create a side effect or make code more complicate is a problem, but adding a new code to avoid anti-pattern doesn't seem to have a problem(I think that's why you want to avoid introducing new concept validToAdd`).

Here is content that shows down casting is pattern to avoid.
https://www.reddit.com/r/cpp_questions/comments/az23wc/is_downcasting_a_sign_of_a_bad_code_design/

In summarize, it tells that downcasting is sign of bad code but there are some excpetions. Because of legacy code or third party dependencies.

jonatan-ivanov · 2025-10-09T19:33:30Z

Thank you for your feedback!
I agree with you that the KeyValue -> KeyValueWithCardinality cast is unfortunate and that it signals an improper design but let me give you some background about why I did things this way in this change-set:

When we came up with the KeyValue concept, we were considering adding cardinality to it but we decided not to, to keep the class simpler and smaller. This also has some performance benefits. Up until this point, this served us well. I think we cannot really introduce cardinality now without breaking changes but I'm not sure we would have the need to do that, see the next point.
AnnotationHandler lets you customize the type where you set the metadata (type T) but it does not let you customize the metadata type itself. It is hard-coded KeyValue instead of a type parameter which causes the issue of the cast above and also KeyValue is used at places where it is not needed (CountedMeterTagAnnotationHandler and MeterTagAnnotationHandler both need to use KeyValue while they need to set Tag). We cannot really fix it without a breaking change.
I agree that the cast shows an improper design, and I think this boils down to AnnotationHandler not letting us defining the metadata type. But this also falls into the "Because of legacy code or third party dependencies" category since this is a piece of code we cannot break to fix it in a minor release.
In order to work this around, one option is casting which is quite unfortunate but on the other hand, it's hidden from the users, it's a private implementation detail and as such, users should not be confused about it. Also, it does not grow our public API surface. I agree that this is bad but on the other hand, its scope is very narrow and hidden.
Another one is introducing the validToAdd concept which I like that it lets us avoiding the cast but I don't think it fixes the root problem in AnnotationHandler. On the other hand, it adds a new user-facing concept. It also forces ObservationKeyValueAnnotationHandler not being an AnnotationHandler (also user-facing) and I think it might be more brittle than the casting approach because of having two handlers in ObservationKeyValueAnnotationHandler and maintaining the "contract" between them and their lambdas. My biggest fear with this approach that if we need another AnnotationHandler that has some custom needs, we might end up introducing another user-facing concept just to patch-up the shortcomings of AnnotationHandler. So right now I think I would rather bite the bullet and do the cast in a non-user-facing way but also create an issue to fix AnnotationHandler in the next major release in Micrometer so that we can eliminate the cast too.

isanghaessi · 2025-10-10T01:29:18Z

Thank you so much for the detailed explanation, @jonatan-ivanov.
I agree with you 100 percent.

I can understand those.

There is problem to add function with cardinality without breaking changes to AnnotationHandler.
Introducing validToAdd may eliminate the original meaning of AnnotationHandler(new concepts may continue to be added to AnnotationHandler whenever features are added in the future).

jonatan-ivanov · 2025-10-11T00:26:57Z

Fyi: I slightly polished the tests.

@isanghaessi Thank you very much again for the PR, we really appreciate your help!

Signed-off-by: Seungyong Hong <[email protected]> Closes micrometer-metricsgh-4030 Co-authored-by: Jonatan Ivanov <[email protected]>

shakuzen · 2025-10-14T02:58:42Z

micrometer-observation/src/main/java/io/micrometer/observation/aop/Cardinality.java

+
+/**
+ * Represents the cardinality of a key-value. There are two types of cardinality and
+ * treated in different ways.


Probably worth clarifying this is for use with the annotation, specifically. There's no way to use it with KeyValue otherwise. We can polish this post-merge.

isanghaessi mentioned this pull request Aug 27, 2025

Add KeyValues to Observations using annotations #5826

Closed

shakuzen reviewed Sep 11, 2025

View reviewed changes

shakuzen requested a review from jonatan-ivanov September 11, 2025 07:27

isanghaessi changed the title ~~GH-5826: Add AOP feature that is parmater-based tagging to span~~ GH-5826: Add AOP feature that is parmater-based tagging to observation Sep 12, 2025

jonatan-ivanov reviewed Sep 16, 2025

View reviewed changes

jonatan-ivanov changed the title ~~GH-5826: Add AOP feature that is parmater-based tagging to observation~~ Add KeyValues to Observations using annotations Sep 16, 2025

jonatan-ivanov reviewed Sep 24, 2025

View reviewed changes

isanghaessi commented Sep 25, 2025

View reviewed changes

micrometer-observation/src/main/java/io/micrometer/observation/aop/ObservedAspect.java Show resolved Hide resolved

shakuzen linked an issue Oct 7, 2025 that may be closed by this pull request

Support KeyValues with annotations when using ObservedAspect / @Observed #4030

Closed

shakuzen mentioned this pull request Oct 7, 2025

Support KeyValues with annotations when using ObservedAspect / @Observed #4030

Closed

jonatan-ivanov mentioned this pull request Oct 9, 2025

Parameterize KeyValue in AnnotationHandler #6786

Open

jonatan-ivanov approved these changes Oct 11, 2025

View reviewed changes

jonatan-ivanov force-pushed the GH-5826 branch from 6b5c786 to 0ebb9d7 Compare October 13, 2025 23:45

Add KeyValues to Observations using annotations

449b8c1

Signed-off-by: Seungyong Hong <[email protected]> Closes micrometer-metricsgh-4030 Co-authored-by: Jonatan Ivanov <[email protected]>

jonatan-ivanov force-pushed the GH-5826 branch from 0ebb9d7 to 449b8c1 Compare October 13, 2025 23:48

shakuzen approved these changes Oct 14, 2025

View reviewed changes

shakuzen merged commit 5e58769 into micrometer-metrics:main Oct 14, 2025
11 checks passed


		private final BiFunction<Annotation, Object, KeyValue> toKeyValue;

		private final BiPredicate<Annotation, Object> validToAdd;

Add KeyValues to Observations using annotations #6667

Add KeyValues to Observations using annotations #6667

Uh oh!

Conversation

isanghaessi commented Aug 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Changes

Review Points

Reference

Uh oh!

shakuzen commented Sep 1, 2025

Uh oh!

isanghaessi commented Sep 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

micopiira commented Sep 9, 2025

Uh oh!

isanghaessi commented Sep 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

shakuzen left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

shakuzen commented Sep 11, 2025

Uh oh!

isanghaessi commented Sep 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jonatan-ivanov Sep 16, 2025

Choose a reason for hiding this comment

Uh oh!

jonatan-ivanov Sep 16, 2025

Choose a reason for hiding this comment

Uh oh!

isanghaessi Sep 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jonatan-ivanov commented Sep 16, 2025

Uh oh!

isanghaessi commented Sep 17, 2025

Uh oh!

isanghaessi commented Sep 24, 2025

Uh oh!

jonatan-ivanov commented Sep 24, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jonatan-ivanov commented Oct 9, 2025

Uh oh!

isanghaessi commented Oct 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jonatan-ivanov commented Oct 9, 2025

Uh oh!

isanghaessi commented Oct 10, 2025

Uh oh!

jonatan-ivanov commented Oct 11, 2025

Uh oh!

shakuzen Oct 14, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

isanghaessi commented Aug 27, 2025 •

edited

Loading

isanghaessi commented Sep 1, 2025 •

edited

Loading

isanghaessi commented Sep 10, 2025 •

edited

Loading

isanghaessi commented Sep 11, 2025 •

edited

Loading

isanghaessi Sep 17, 2025 •

edited

Loading

isanghaessi commented Oct 9, 2025 •

edited

Loading