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.

Sign up for GitHub

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

Jump to bottom

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

Merged
merged 4 commits into from
Apr 28, 2021
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
11f3994
Get Tracer operation should use an empty string in case if invalid name.
carlosalberto Apr 27, 2021
a51d0cc
Update CHANGELOG.
carlosalberto Apr 27, 2021
200b344
Merge branch 'main' into get_tracer_empty_str
arminru Apr 28, 2021
e4bea08
Update CHANGELOG.md
carlosalberto Apr 28, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ release.

### Traces

- `Get Tracer` should use an empty string if the specified `name` is null. ([#1654](https://github.com/open-telemetry/opentelemetry-specification/pull/1654))

### Metrics

### Logs
Expand Down
2 changes: 1 addition & 1 deletion specification/trace/api.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 •
edited
Loading

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 •
edited
Loading

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.

and a message reporting that the specified value is invalid SHOULD be logged.
A library, implementing the OpenTelemetry API *may* also ignore this name and
return a default instance for all calls, if it does not support "named"
Expand Down