Convert null to empty string for tracer name #712
Conversation
Codecov Report
@@ Coverage Diff @@
## main #712 +/- ##
==========================================
+ Coverage 94.72% 94.73% +0.01%
==========================================
Files 216 216
Lines 9895 9900 +5
==========================================
+ Hits 9373 9379 +6
+ Misses 522 521 -1
|
| @@ -23,6 +23,9 @@ nostd::shared_ptr<opentelemetry::trace::Tracer> TracerProvider::GetTracer( | |||
| nostd::string_view library_name, | |||
| nostd::string_view library_version) noexcept | |||
| { | |||
| if (library_name.data() == nullptr) { | |||
| library_name = ""; | |||
| } | |||
| // if (library_name == "") { | |||
There was a problem hiding this comment.
Do you still intend keeping the previous TODO, since now the code intentionally forces that condition?
( I assume you have to until we sort out some internal macro for logging internal SDK trace messages ? )
There was a problem hiding this comment.
The TODO is still needed I think, as the spec requires message reporting. Yes, I assume it is internal SDK logging.
message reporting that the specified value is invalid SHOULD be logged
sdk/src/trace/tracer_provider.cc
Outdated
| @@ -23,6 +23,9 @@ nostd::shared_ptr<opentelemetry::trace::Tracer> TracerProvider::GetTracer( | |||
| nostd::string_view library_name, | |||
| nostd::string_view library_version) noexcept | |||
| { | |||
| if (library_name.data() == nullptr) { | |||
There was a problem hiding this comment.
Passing a nullptr to GetTrace() is the application/intrumentation code issue, and the right behavior should be to let it crash so that the application developer can fix this. Probably I need to understand the spec change more clearly.
There was a problem hiding this comment.
The latest spec mentioned this in explicit that null name should not crash and should be treat like empty name.
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 be set to an empty string,
|
@ThomsonTan - Can you look into CI failures so that we can merge this ? |
Thanks for the reminder, fixed the format. |
Changes
Below spec change required tracer name to be converted to empty string if it is null. As we are using
nostd::string_viewto represent null, this change converts it to empty string inTracerProvider::GetTracer(name, version).open-telemetry/opentelemetry-specification#1654
For significant contributions please make sure you have completed the following items:
CHANGELOG.mdupdated for non-trivial changes