From f167c36eec5efb103f4277675acd0ffe2e92d2bc Mon Sep 17 00:00:00 2001 From: "Alex E." <36134278+chusitoo@users.noreply.github.com> Date: Mon, 2 Dec 2024 17:00:03 -0500 Subject: [PATCH] [SHIM] Add size to all string view mappings between OT and OTel (#3181) --- .../opentelemetry/opentracingshim/propagation.h | 16 +++++++++------- .../opentelemetry/opentracingshim/shim_utils.h | 7 +++++-- opentracing-shim/src/span_context_shim.cc | 2 +- opentracing-shim/src/span_shim.cc | 14 +++++++++----- opentracing-shim/src/tracer_shim.cc | 13 +++++++------ 5 files changed, 31 insertions(+), 21 deletions(-) diff --git a/opentracing-shim/include/opentelemetry/opentracingshim/propagation.h b/opentracing-shim/include/opentelemetry/opentracingshim/propagation.h index b2a9be5a80..4e44276a3d 100644 --- a/opentracing-shim/include/opentelemetry/opentracingshim/propagation.h +++ b/opentracing-shim/include/opentelemetry/opentracingshim/propagation.h @@ -29,7 +29,8 @@ class CarrierWriterShim : public opentelemetry::context::propagation::TextMapCar virtual void Set(nostd::string_view key, nostd::string_view value) noexcept override { - writer_.Set(key.data(), value.data()); + writer_.Set(opentracing::string_view{key.data(), key.size()}, + opentracing::string_view{value.data(), value.size()}); } private: @@ -46,17 +47,17 @@ class CarrierReaderShim : public opentelemetry::context::propagation::TextMapCar nostd::string_view value; // First try carrier.LookupKey since that can potentially be the fastest approach. - if (auto result = reader_.LookupKey(key.data())) + if (auto result = reader_.LookupKey(opentracing::string_view{key.data(), key.size()})) { - value = result.value().data(); + value = nostd::string_view{result.value().data(), result.value().size()}; } else // Fall back to iterating through all of the keys. { reader_.ForeachKey([key, &value](opentracing::string_view k, opentracing::string_view v) -> opentracing::expected { - if (k == key.data()) + if (key == nostd::string_view{k.data(), k.size()}) { - value = v.data(); + value = nostd::string_view{v.data(), v.size()}; // Found key, so bail out of the loop with a success error code. return opentracing::make_unexpected(std::error_code{}); } @@ -77,8 +78,9 @@ class CarrierReaderShim : public opentelemetry::context::propagation::TextMapCar return reader_ .ForeachKey([&callback](opentracing::string_view key, opentracing::string_view) -> opentracing::expected { - return callback(key.data()) ? opentracing::make_expected() - : opentracing::make_unexpected(std::error_code{}); + return callback(nostd::string_view{key.data(), key.size()}) + ? opentracing::make_expected() + : opentracing::make_unexpected(std::error_code{}); }) .has_value(); } diff --git a/opentracing-shim/include/opentelemetry/opentracingshim/shim_utils.h b/opentracing-shim/include/opentelemetry/opentracingshim/shim_utils.h index a7feb5efc4..3436473870 100644 --- a/opentracing-shim/include/opentelemetry/opentracingshim/shim_utils.h +++ b/opentracing-shim/include/opentelemetry/opentracingshim/shim_utils.h @@ -29,7 +29,10 @@ static inline opentelemetry::common::AttributeValue attributeFromValue( AttributeValue operator()(int64_t v) { return v; } AttributeValue operator()(uint64_t v) { return v; } AttributeValue operator()(const std::string &v) { return nostd::string_view{v}; } - AttributeValue operator()(opentracing::string_view v) { return nostd::string_view{v.data()}; } + AttributeValue operator()(opentracing::string_view v) + { + return nostd::string_view{v.data(), v.size()}; + } AttributeValue operator()(std::nullptr_t) { return nostd::string_view{}; } AttributeValue operator()(const char *v) { return v; } AttributeValue operator()(opentracing::util::recursive_wrapper) @@ -54,7 +57,7 @@ static inline std::string stringFromValue(const opentracing::Value &value) std::string operator()(int64_t v) { return std::to_string(v); } std::string operator()(uint64_t v) { return std::to_string(v); } std::string operator()(const std::string &v) { return v; } - std::string operator()(opentracing::string_view v) { return std::string{v.data()}; } + std::string operator()(opentracing::string_view v) { return std::string{v.data(), v.size()}; } std::string operator()(std::nullptr_t) { return std::string{}; } std::string operator()(const char *v) { return std::string{v}; } std::string operator()(opentracing::util::recursive_wrapper) diff --git a/opentracing-shim/src/span_context_shim.cc b/opentracing-shim/src/span_context_shim.cc index 54af4da9a3..cd1069e9d9 100644 --- a/opentracing-shim/src/span_context_shim.cc +++ b/opentracing-shim/src/span_context_shim.cc @@ -23,7 +23,7 @@ bool SpanContextShim::BaggageItem(nostd::string_view key, std::string &value) co void SpanContextShim::ForeachBaggageItem(VisitBaggageItem f) const { baggage_->GetAllEntries([&f](nostd::string_view key, nostd::string_view value) { - return f(key.data(), value.data()); + return f(std::string{key.data(), key.size()}, std::string{value.data(), value.size()}); }); } diff --git a/opentracing-shim/src/span_shim.cc b/opentracing-shim/src/span_shim.cc index 1be909f1bd..afc974f057 100644 --- a/opentracing-shim/src/span_shim.cc +++ b/opentracing-shim/src/span_shim.cc @@ -45,7 +45,7 @@ void SpanShim::FinishWithOptions(const opentracing::FinishSpanOptions &finish_sp void SpanShim::SetOperationName(opentracing::string_view name) noexcept { - span_->UpdateName(name.data()); + span_->UpdateName(nostd::string_view{name.data(), name.size()}); } void SpanShim::SetTag(opentracing::string_view key, const opentracing::Value &value) noexcept @@ -57,7 +57,8 @@ void SpanShim::SetTag(opentracing::string_view key, const opentracing::Value &va } else { - span_->SetAttribute(key.data(), utils::attributeFromValue(value)); + auto key_view = nostd::string_view{key.data(), key.size()}; + span_->SetAttribute(key_view, utils::attributeFromValue(value)); } } @@ -68,9 +69,11 @@ void SpanShim::SetBaggageItem(opentracing::string_view restricted_key, // Baggage key/value pair, and sets it as the current instance for this Span Shim. if (restricted_key.empty() || value.empty()) return; + auto restricted_key_view = nostd::string_view{restricted_key.data(), restricted_key.size()}; + auto value_view = nostd::string_view{value.data(), value.size()}; // This operation MUST be safe to be called concurrently. const std::lock_guard guard(context_lock_); - context_ = context_.newWithKeyValue(restricted_key.data(), value.data()); + context_ = context_.newWithKeyValue(restricted_key_view, value_view); } std::string SpanShim::BaggageItem(opentracing::string_view restricted_key) const noexcept @@ -82,7 +85,8 @@ std::string SpanShim::BaggageItem(opentracing::string_view restricted_key) const // This operation MUST be safe to be called concurrently. const std::lock_guard guard(context_lock_); std::string value; - return context_.BaggageItem(restricted_key.data(), value) ? value : ""; + auto restricted_key_view = nostd::string_view{restricted_key.data(), restricted_key.size()}; + return context_.BaggageItem(restricted_key_view, value) ? value : ""; } void SpanShim::Log(std::initializer_list fields) noexcept @@ -128,7 +132,7 @@ void SpanShim::logImpl(nostd::span fields, for (const auto &entry : fields) { - nostd::string_view key = entry.first.data(); + nostd::string_view key{entry.first.data(), entry.first.size()}; // ... including mapping of the following key/value pairs: if (is_error) { diff --git a/opentracing-shim/src/tracer_shim.cc b/opentracing-shim/src/tracer_shim.cc index 7848cba478..e06961a88e 100644 --- a/opentracing-shim/src/tracer_shim.cc +++ b/opentracing-shim/src/tracer_shim.cc @@ -24,12 +24,13 @@ std::unique_ptr TracerShim::StartSpanWithOptions( if (is_closed_) return nullptr; - const auto &opts = utils::makeOptionsShim(options); - const auto &links = utils::makeIterableLinks(options); - const auto &attributes = utils::makeIterableTags(options); - const auto &baggage = utils::makeBaggage(options); - auto span = tracer_->StartSpan(operation_name.data(), attributes, links, opts); - auto span_shim = new (std::nothrow) SpanShim(*this, span, baggage); + const auto &opts = utils::makeOptionsShim(options); + const auto &links = utils::makeIterableLinks(options); + const auto &attributes = utils::makeIterableTags(options); + const auto &baggage = utils::makeBaggage(options); + auto operation_name_view = nostd::string_view{operation_name.data(), operation_name.size()}; + auto span = tracer_->StartSpan(operation_name_view, attributes, links, opts); + auto span_shim = new (std::nothrow) SpanShim(*this, span, baggage); // If an initial set of tags is specified and the OpenTracing error tag // is included after the OpenTelemetry Span was created.