-
Notifications
You must be signed in to change notification settings - Fork 896
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Consider reverting/amending #1534 (removes fallback tracer name) #1619
Comments
The requirement in question is a SHOULD-requirement. So if Java has good reasons to deviate (slightly), I think it is fine to do so with the current spec. |
It looks like this issue is caused by an unnecessary side effect in the PR. The original issue was to define a canonical tracer name when an invalid name is given. Returning the original invalid name of the tracer seems like incorrect behavior - we should return the default name, and log the error. |
cc @bogdandrutu |
Reading the spec (
|
After discussing it in today's SIG call, we decided to go with an empty string instead of |
#1534 changed the spec to require null tracer names to be kept intact. Unfortunately, this ends up introducing a pretty significant behavioral breaking change to the Java SDK interfaces for exporters. It would require our
InstrumentationLibraryInfo
to return anullable
name attribute, which means that all exporters (both ours and any 3rd party) now have to guard against a null attribute, or risk throwing NullPointerExceptions in the exporter.See this Java PR for the impact within the java repository itself: open-telemetry/opentelemetry-java#3153
I think that we should amend this part of the specification to note that nulls should be converted into empty strings, or we should revert that change and specify an actual fallback value.
The text was updated successfully, but these errors were encountered: