Skip to content
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

Define the default value for null/empty instrumentation-name #1472

Closed
carlosalberto opened this issue Feb 26, 2021 · 4 comments · Fixed by #1534
Closed

Define the default value for null/empty instrumentation-name #1472

carlosalberto opened this issue Feb 26, 2021 · 4 comments · Fixed by #1534
Assignees
Labels
area:api Cross language API specification issue area:sdk Related to the SDK spec:trace Related to the specification/trace directory

Comments

@carlosalberto
Copy link
Contributor

Currently TracerProvider.GetTracer() is defined as:

In case an invalid name (null or empty string) is specified, a working default Tracer implementation as a fallback is returned rather than returning null or throwing an exception.

However, different SIGs have different values, i.e.

  • Java uses unknown
  • JS uses the empty string
  • Python uses ERROR:MISSING MODULE NAME

I'd suggest we define a SHOULD on this, so at least the default is (more or less) shared. Personally I'd go with unknown (although defaulting to "" probably would be helpful if we ever support a default Tracer, i.e. TracerProvider.getDefaultTracer())

@carlosalberto carlosalberto added area:api Cross language API specification issue spec:trace Related to the specification/trace directory labels Feb 26, 2021
@arminru
Copy link
Member

arminru commented Feb 26, 2021

Good catch! I think we should definitely go for a consistent fallback here.
Personally I like Python's fallback since it clearly points out that this an undesirable state and just used for gracefully handling a missing or invalid parameter rather than being a valid, intended default value.

What about <INVALID NAME PROVIDED> or something like that?
It should be clearly distinguishable from valid values provided by users (who would usually pass this.class.name and such).

As for a getDefaultTracer or getApplicationTracer that we might consider adding in future, I'd go for a different value there, probably even an empty string.

@arminru
Copy link
Member

arminru commented Feb 26, 2021

I also think this should only affect the SDK spec and not be mentioned in the API spec as well as the resulting interface definitions in the implementations. This is not something one should rely on. The API spec should only define what is valid as input for this method and if this is not complied with, it's undefined behavior from an API perspective.

The entire sentence

In case an invalid name (null or empty string) is specified, a working default Tracer implementation as a fallback is returned rather than returning null or throwing an exception.

should've been only mentioned in the SDK spec IMHO but we'll have to leave this one in now.

@Oberon00 Oberon00 added the area:sdk Related to the SDK label Feb 26, 2021
@Oberon00
Copy link
Member

I would vote for <INVALID INSTRUMENTATION NAME PROVIDED>. It's OK if its very long, you should only see it shortly until you fix it anyway 😃

By the way, as commented in #586 (comment), "working default tracer" could be interpreted as no-op tracer so we should maybe clear that up too.

@carlosalberto
Copy link
Contributor Author

Trying to re-awake this: I'd vow for going with unknown_instrumentation_name, trying to stay in line with what Resource does in absence of service.name.

In any case, assigning this to me to get ourselves moving ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:api Cross language API specification issue area:sdk Related to the SDK spec:trace Related to the specification/trace directory
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants