Build more tags using method context

[hrothwell]

Aiming to solve #752 and potentially also make #612 possible

Any beans implementing TagsBasedOnMethodInvocationContext are injected and additional tags are computed using buildTags(MethodInvocationContext context)

[CLAassistant]

All committers have signed the CLA.

[sdelamo]

This is a breaking change. You would need to add a new constructor (the new one should be annotated with @Inject while keeping and deprecating the old one (protected TimedInterceptor(MeterRegistry meterRegistry, ConversionService conversionService)) removing the @Inject annotation from it

[sdelamo]

You need to add also documentation for this feature.

[hrothwell]

Just now realizing this is probably also helpful / can support @Counted annotations as well, so will move a few more things around to support that

[hrothwell]

bringing attention to @graemerocher 's comment here, this seems like something that could be done here as well? Perhaps not all @Timed or @Counted annotations really need/want all tags provided by beans in the context. My initial idea was your implementation of the tagger would be able to decide if it wants to tag it or not, but explicitly saying "add additional tags using this impl" in the annotation metadata seems more convenient to me. Thoughts?

The only thing I am unsure of is if you wanted your tagger to have some other bean injected how that would work. I feel there is a way to do it, I am just not familiar enough with that

[graemerocher]

you can inject it into the constructor of the tagger, for example if you need the MeterRegistry you just add a constructor argument for it

[hrothwell]

you can inject it into the constructor of the tagger, for example if you need the MeterRegistry you just add a constructor argument for it

Even if doing something like @Timed(strategies = [MyTagger class]) as you alluded to in that other PR comment? I like that idea of being able to explicitly call out what taggers to use in the annotations, I'm just not sure how to put that together 🤔

[graemerocher]

would be a matter of adding a filter to the stream based on the types that are resolved from the annotation

[hrothwell]

would be a matter of adding a filter to the stream based on the types that are resolved from the annotation

that makes sense, would that then mean we need to create our own annotations to replace @Timed and @Counted though? Or an annotation that works side by side with them? Definitely something I am interested in doing just not sure on the how, probably also isn't needed as part of this PR and could come later

[graemerocher]

yeah I guess we would have our own version kind of like how we have both Jakarta @Transactional and our own one

[hrothwell]

yeah I guess we would have our own version kind of like how we have both Jakarta @Transactional and our own one

another thought that came to mind: could you create a new annotation that is just used alongside the existing ones? Wouldn't introduce a breaking change, but maybe something like:

@Timed(value = "requests")
@AddTags(taggers = [MyTagger.class])
fun timedRequest() = println("time me")

Doesn't require updating the @Timed metric for users, can be retrieved from the AnnotationMetadata that is already pulled, etc

[graemerocher]

I would probably call it @MetricOptions or something in case it could be used for something else

[hrothwell]

I would probably call it @MetricOptions or something in case it could be used for something else

is this something that would be wanted for/in this PR or just something that could come later down the line?

[graemerocher]

No can be done in a subsequent PR

[hrothwell]

Am I expected to merge this PR? I don't seem to have the right authorization to do so with approvals present, and not sure if @sdelamo change requests need to be cleared prior