Draft: Add instrumentation for otel span annotations - #1348
Conversation
|
@Kleszczu98 hi. The title of this still says it's a draft PR, but it's not marked as such. Please remove the word Draft from the title if it's ready for review, or mark it truly as draft by clicking "Convert to draft". You'll will also need to sign the Contributor License Agreement (CLA). Thanks! |
There was a problem hiding this comment.
Are we aware of a use case that requires tracing a constructor? It sounds a bit of an edge case to me. If there's no foreseeable usage right now, I'd prefer to leave it for a future PR.
There was a problem hiding this comment.
Ok, i will remove the constructor implementation and focus on just the normal method case to simplify the PR.
| @Advice.OnMethodEnter(suppress = Throwable::class) | ||
| fun onEnter( | ||
| @Advice.AllArguments args: Array<Any?>, | ||
| @Advice.Origin method: Method |
There was a problem hiding this comment.
I'd like to follow best practices from the upstream instrumentation, such as avoiding using @Advice.Origin Method. In this case it seems like we can just get the method name by using @Advice.Origin("#m") String methodName.
| @JvmStatic | ||
| @Advice.OnMethodEnter(suppress = Throwable::class) | ||
| fun onEnter( | ||
| @Advice.AllArguments args: Array<Any?>, |
There was a problem hiding this comment.
I'm not faimilar with @WithSpan, though I'm curious why it might need all the method's arguments?
There was a problem hiding this comment.
I found the @AddingSpanAttributes annotation but it seems i might've misunderstood it's function.
It is not mentioned in the linked documentation so perhaps I should remove this as well and stick to @WithSpan and @SpanAttribute ? It covers all the most common use cases already.
There was a problem hiding this comment.
I see. I also don't see it in the docs so it should be fine to leave it out, at least for now. I'll cc @breedx-splk in case he's more familiar with it to make sure we're not missing something important.
- Removed constructor implementation for now
- Applied @Advice.Origin("#m") in AddingSpanAttributesMethodAdvice.kt
- Added basic test for a start
- Generated apiDump
- Ran spotlessApply
- Ran check
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1348 +/- ##
==========================================
- Coverage 64.17% 62.33% -1.84%
==========================================
Files 153 164 +11
Lines 3059 3181 +122
Branches 315 328 +13
==========================================
+ Hits 1963 1983 +20
- Misses 1007 1105 +98
- Partials 89 93 +4 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
fractalwrench
added
the
needs author feedback
|
This has been automatically marked as stale because it has been marked as needing author feedback and has not had any activity for 21 days. It will be closed automatically if there is no response from the author within 14 additional days from this comment. |
|
@Kleszczu98 , good day, |
5 participants
Example byteBuddy implementation of auto-instrumentation of the following annotations:
#412