feat: complete relocation from io.opentelemetry in output JAR #427
Conversation
| @@ -40,27 +40,30 @@ tasks { | |||
|
|
|||
| mergeServiceFiles() | |||
|
|
|||
| relocate("com.blogspot.mydailyjava.weaklockfree", "io.opentelemetry.instrumentation.api.internal.shaded.weaklockfree") | |||
| relocate("com.blogspot.mydailyjava.weaklockfree", "ai.traceable.io.opentelemetry.instrumentation.api.internal.shaded.weaklockfree") | |||
There was a problem hiding this comment.
feels weird that we add traceable in hypertrace repos? Maybe org.hypertrace instead of ai.traceable?
There was a problem hiding this comment.
I will shift this entire repo to traceable javaagent
There was a problem hiding this comment.
Okay, then lets do that first and do this there? Addng traceable package names in this repo which is part of the hypertrace opensource code feels weird
There was a problem hiding this comment.
- There is a waiting customer requiring this fix
- Also instead of how we did for goagent, I would like a complete relocation of instrumentations for traceable. As that helps with debuggers. So, it will take time
- We already do use traceable's name in this repo
Like https://github.com/hypertrace/javaagent/blob/main/javaagent/src/main/java/org/hypertrace/agent/instrument/HypertraceAgent.java#L80
There was a problem hiding this comment.
- agreed
- agreed. When you migrate it, maybe worth it to switch these to hypertrace at that point, because we'll be archiving this repo at that point and its bad to leave the repo in a mangled state. Actually, just for my understanding, whats the issue when you add
hypertracein the name instead of traceable? I'd guess the shaded package can take any name as long as it doesnt conflict and not specifically requiretraceable. - this is specifically checking for traceable agent, not adding traceable as part of its own code.
There was a problem hiding this comment.
yeah, just moving to org.hypertrace shouldn't be problem though, I am just saying merging with traceable repo will take time
There was a problem hiding this comment.
ohh then, coming back to my first point, maybe worth it to use org.hypertrace here instead of ai.traceable here, point being since we have separate repos we try to keep traceable out of this open source repo? But this is really a nit, if you feel we dont need that, lmk and I can approve
There was a problem hiding this comment.
But one thing I don't like is that, when we use traceable agent, due to current structure instrumentation scope of spans will be shown as
InstrumentationScope org.hypertrace.io.opentelemetry.netty-4.1 1.33.0-alpha in the case if we relocate to org.hypertrace instead.
which is a bit weird according to me as all customers would use traceable javaagent
There was a problem hiding this comment.
which will anyways be fixed after you migrate this repo to the traceable one right?
There was a problem hiding this comment.
yup, should be. But till then
I am ok to change though, it is a nit call anyway
No description provided.