@Observed #3221
@Observed #3221
Conversation
Observation TCK should be in the io.micrometer.observation.tck package instead of io.micrometer.core.tck. micrometer-test already has the io.micrometer.core.tck package so this change also resolves potential issues with the module system on Java 9 and above.
jonatan-ivanov
added
enhancement
![sonatype-lift[bot]](https://avatars.githubusercontent.com/in/9843?s=60&v=4){kind=small}
micrometer-observation/src/main/java/io/micrometer/observation/annotation/Observed.java
Show resolved
Hide resolved
micrometer-observation/src/main/java/io/micrometer/observation/aop/ObservedAspect.java
Show resolved
Hide resolved
micrometer-observation/src/main/java/io/micrometer/observation/aop/ObservedAspect.java
Show resolved
Hide resolved
micrometer-observation/src/main/java/io/micrometer/observation/Observation.java
Outdated
Show resolved
Hide resolved
...ervation-test/src/main/java/io/micrometer/observation/tck/TestObservationRegistryAssert.java
Outdated
Show resolved
Hide resolved
| * Low cardinality key values. | ||
| * @return an array of low cardinality key values. | ||
| */ | ||
| String[] lowCardinalityKeyValues() default {}; |
There was a problem hiding this comment.
Why aren't we allowing to set highCardinalityKeyValues ?
Also, shouldn't we give an option to provide a KeyValuesProvider class ? It would have to have a default constructor (when someone doesn't want to calculate the tags from the annotated method arguments) or (maybe) a constructor that takes in Object[] where the array would be mapped to methods arguments? Or even maybe also a Method argument? Does it make any sense? 🤔
There was a problem hiding this comment.
Why aren't we allowing to set highCardinalityKeyValues ?
Because attribute values of annotations must be constants, you can't have dynamic values since you can't even call a method there, only literals and constants are allowed.
Does it make any sense?
It makes sense to me. I did write a PoC and also proposed a PR for a pretty similar feature for @Timed years ago 😄, see #1586
Check out the issue (there is also a PR), the PoC tries to do two things:
- Add an
@ExtraTagannotation which is like@SpanTagin Sleuth and somewhat can cover theObject[]/Method/JointPontscenario that you mentioned above. - An
ExtraTagsPropagationclass which is similar to Sleuth 2'sExtraFieldPropagation.
I think if we want to do something like this (I think it would be a nice feature) we should keep @Timed and @Observed in feature parity to a degree.
Also, I think we should discuss about two things in terms of @Observed:
- Setting keyvalues based on the jointpoint/method/params (an extra annotation can cover that, see above)
- Setting keyvalues based on the context
Can we move this to a separate issue/PR?
micrometer-observation/src/main/java/io/micrometer/observation/aop/ObservedAspect.java
Outdated
Show resolved
Hide resolved
# Conflicts: # micrometer-observation/src/main/java/io/micrometer/observation/Observation.java
micrometer-observation/src/main/java/io/micrometer/observation/aop/ObservedAspect.java
Show resolved
Hide resolved
No description provided.