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

Get Tracer operation should use an empty string if the specified name is null #1654

Merged
merged 4 commits into from
Apr 28, 2021

Conversation

carlosalberto
Copy link
Contributor

Fixes #1619 - as discussed previously in a previous SIG call.

The Get Tracer operation, upon receiving an invalid value, should set the name to an empty string, instead of allowing null values to pass through.

@carlosalberto carlosalberto requested review from a team April 27, 2021 16:39
@carlosalberto carlosalberto changed the title Get Tracer operation should use an empty string in case if invalid name. Get Tracer operation should use an empty string if the specified name is null Apr 27, 2021
CHANGELOG.md Outdated Show resolved Hide resolved
@@ -110,7 +110,7 @@ This API MUST accept the following parameters:
or application.
In case an invalid name (null or empty string) is specified, a working
Tracer implementation MUST be returned as a fallback rather than returning
null or throwing an exception, its `name` property SHOULD keep the original invalid value,
null or throwing an exception, its `name` property SHOULD be set to an **empty** string,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should TracerProvider::GetTracer(null) and TracerProvider::GetTracer("") return the same Tracer with "" as name?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Identity of the returned tracer is not relevant to the spec, as the only per-tracer state is the (immutable) Instrumentation Library name and version.

Copy link
Contributor

@ThomsonTan ThomsonTan Apr 28, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. I am still a little bit unclear about "its name property SHOULD be set to an empty string." Does it mean the name property of the tracer should be set to empty string even if null is passed to GetTracer, but keep the name stored in Instrumentation Library as null which is the original value passed to GetTracer?

Copy link
Contributor Author

@carlosalberto carlosalberto Apr 28, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably we need a further clarification, but the idea is that Get Tracer (null) essentially becomes a call to Get Tracer ("") (with name exactly being from this point "", which will be used also in the stored instrumentation).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Get Tracer(null) becomes a call to Get Tracer("") is very clear to me. I think further clarification will be helpful to guide implementations.

arminru and others added 2 commits April 28, 2021 14:08
Co-authored-by: John Watson <jkwatson@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Consider reverting/amending #1534 (removes fallback tracer name)
10 participants