Add java guidance for native instrumentation and manual instrumentation#8618
Conversation
|
Please take a look @open-telemetry/java-approvers, @open-telemetry/java-instrumentation-approvers. |
| Native instrumentation has an interesting interaction with the OpenTelemetry | ||
| Java agent: On startup, the Java agent initializes an | ||
| [OpenTelemetry](../api/#opentelemetry)) instance and installs | ||
| [zero-code](#zero-code-java-agent) instrumentation. Libraries adding native | ||
| instrumentation should allow users to customize the `OpenTelemetry` instance | ||
| used, but ideally should automatically use the instance initialized by the Java | ||
| agent (if present). See [GlobalOpenTelemetry](../api/#globalopentelemetry) for | ||
| guidance on how to accomplish this. | ||
|
|
There was a problem hiding this comment.
🎉
@JonasKunz @SylvainJuge would be great to get this into the Elasticsearch native instrumentation!
theletterf
left a comment
theletterf
left a comment
There was a problem hiding this comment.
Thanks for this one! An initial review.
|
|
||
| The following code snippet explores `GlobalOpenTelemetry` API context | ||
| propagation: | ||
| `GlobalOpenTelemetry` is designed in a very particular way to avoid |
There was a problem hiding this comment.
What does "particular" imply here? Can we be more specific? Or just say "is designed to avoid..."
There was a problem hiding this comment.
Particular implies something like "surprising" or "make sure you read it twice". We're in a big of a bind with GlobalOpenTelemetry with there being no design that isn't surprising in some cases to some people.
The next sentence starting with "Specifically, " elaborates on what is particular about the design.
| Native instrumentation has an interesting interaction with the OpenTelemetry | ||
| Java agent: On startup, the Java agent initializes an | ||
| [OpenTelemetry](../api/#opentelemetry)) instance and installs |
There was a problem hiding this comment.
| Native instrumentation has an interesting interaction with the OpenTelemetry | |
| Java agent: On startup, the Java agent initializes an | |
| [OpenTelemetry](../api/#opentelemetry)) instance and installs | |
| Native instrumentation interacts with the OpenTelemetry Java agent | |
| in the following way: On startup, the Java agent initializes an | |
| [OpenTelemetry](../api/#opentelemetry)) instance and installs |
There was a problem hiding this comment.
Rephrased but went in a slightly different direction. Let me know what you think.
| written by application authors, and typically specific to the application | ||
| domain. | ||
|
|
||
| Manual instrumentation has an interesting interaction with the OpenTelemetry |
…y.io into native-instrumentation-advice
|
Ping @open-telemetry/docs-maintainers. Anything still blocking merging this? |
|
|
||
| ### GlobalOpenTelemetry | ||
|
|
||
| {{% alert title="Java agent" %}} The Java agent is a special case where |
There was a problem hiding this comment.
Can we add a replacement tl;dr for the impacient?
There was a problem hiding this comment.
WDYT of something like:
Use GlobalOpenTelemetry.getOrNoop() if your native instrumentation trying to integrate with the otel java agent. Use GlobalOpenTelemetry.isSet() ? GlobalOpenTelemetry.get() : initializeSdk() if your an application owner writing manual instrumentation and trying to integrate with the otel java agent. Otherwise, don't use GlobalOpenTelemetry. See below for more details.
There was a problem hiding this comment.
I would first mention to use *get" when you want to add manual instrumentation, because I believe this is what most users are trying to do.
There was a problem hiding this comment.
But I think its wrong to do so. Or at least not recommended. If there's manual instrumentation in an app that never uses the javaagent, just don't use the global and pass around OpenTelemetry using constructors or dependency injection. If there's manual instrumentation in an app that uses the java agent, use the GlobalOpenTelemetry.isSet() ? GlobalOpenTelemetry.get() : initializeSdk() pattern so you don't inadvertently cause side affects with GlobalOpenTelemetry.get().
There was a problem hiding this comment.
A very common case is to have the agent always - packaged in the docker image.
Those users should not have to think about sdk questions at all.
I should have been more specific about that.
There was a problem hiding this comment.
Gotcha. I think my guidance to these users would still to be use GlobalOpenTelemetry.getOrNoop().
There was a problem hiding this comment.
That's a good idea - then we can discourage get() altogether.
I created #8690
cartermp
left a comment
cartermp
left a comment
There was a problem hiding this comment.
Approving for merge despite some continuing discussion. We can potentially tweak guidance further in another PR if folks feel it's needed.
6 participants
Resolves open-telemetry/opentelemetry-java#6604
Depends on open-telemetry/opentelemetry-java-examples#956