micrometer-metrics / tracing

Provides tracing abstractions over tracers and tracing system reporters.
https://micrometer.io
Apache License 2.0
248 stars 45 forks source link

SpanAspect does not take into account proxy objects #544

Open lex-em opened 9 months ago

lex-em commented 9 months ago

Hello. I tried to use @NewSpan on spring data repositories, but it idin't work

NewSpan newSpan = method.getAnnotation(NewSpan.class);

method here is from proxy and annotation not found.

I fixed that by using org.apache.commons.lang3.reflect.MethodUtils.getAnnotation(method, NewSpan::class.java, true, true)

Please fix them in library

marcingrzejszczak commented 9 months ago

Can you create a simple reproducer for this issue or a test that shows that this doesn't work?

lex-em commented 9 months ago

@marcingrzejszczak reproducer with instructions in readme is here https://github.com/lex-em/micrometer-tracing-on-proxy CustomSpanAspect class contains additional comments

marcingrzejszczak commented 9 months ago

@shakuzen @jonatan-ivanov what is your opinion on adding a dependency to apache commons to have this functionality? I guess we would need to do the same in micrometer for those aspects

jonatan-ivanov commented 9 months ago

I'm thinking if:

  1. We can copy that single method: getAnnotation
  2. Document that methods like these should be either wrapped or .observed(methodReference) should be used
  3. Instrument Spring Data repositories
marcingrzejszczak commented 9 months ago

That's not copying of just a single method :grimacing: