Avoid ClassCastException on direct SDK access #931
Conversation
trask
requested review from
anuraaga,
iNikem,
jkwatson and
tylerbenson
as code owners
| if (!messageAlreadyLogged.getAndSet(true)) { | ||
| String message = | ||
| "direct usage of the OpenTelemetry SDK is not supported when running agent" | ||
| + " (run with debug logging to see stack trace)"; |
There was a problem hiding this comment.
I'd include the flag here if mentioning debug logging
| public Object unobfuscate() { | ||
| if (!messageAlreadyLogged.getAndSet(true)) { | ||
| String message = | ||
| "direct usage of the OpenTelemetry SDK is not supported when running agent" |
There was a problem hiding this comment.
Adding the concrete operation can provide good context
Direct usage of the OpenTelemetry SDK by calling OpenTelemetrySdk.getTracrProvider() is not...
There was a problem hiding this comment.
Also maybe a link to how to configure the agent (presumably what they were trying to do).
|
These shaded and unshaded terms are very confusing to me and don't seem natural. Can we replace them with some words that reflect the problem domain, not the solution space? I mean we have one set of classes that we bring as dependency, right? And another set which application can bring, right? So maybe agent/application or internal/external or something? |
|
Yeah I think we can address it outside this PR but I agree with @iNikem. Actually, it seems to be the worst of all worlds right now, where unprefixed is shaded, and prefixed is unshaded which is highly unusual. So use-case specific package names like @iNikem suggests would be nice, if there's a technical problem with that, I think swapping so unprefixed is unshaded would be easier to make presentations about :P |
Great idea, created #935 |
| public Object unobfuscate() { | ||
| if (!messageAlreadyLogged.getAndSet(true)) { | ||
| String message = | ||
| "direct usage of the OpenTelemetry SDK is not supported when running agent" |
There was a problem hiding this comment.
What does it mean "not supported"? We don't throw, we don't exist, we have test it place to verify nothing is broken. So what exactly is not supported?
There was a problem hiding this comment.
Not supported means that if you use OpenTelemetrySdk.getTraceProvider(), you won't get back the agent's TraceProvider bridge, and so any telemetry you emit won't get intercepted / correlated / exported by the agent the way we do if you use OpenTelemetry.getTraceProvider() from OpenTelemetry API
...ava/io/opentelemetry/auto/instrumentation/opentelemetryapi/trace/UnshadedTracerProvider.java
Show resolved
Hide resolved
|
A! I was too late! :( |
|
Oh my bad!! I meant to address @anuraaga's comments. Will submit another PR and we can continue discussing there. |
|
Can we discuss this during special meeting? |
|
👍 added special topic meeting to public calendar for Tue Aug 11, 10pm Pacific Time |
Closes #336