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

[SHIM] Fix string_view mappings between OT and OTel #3181

Merged
merged 1 commit into from
Dec 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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<void> {
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{});
}
Expand All @@ -77,8 +78,9 @@ class CarrierReaderShim : public opentelemetry::context::propagation::TextMapCar
return reader_
.ForeachKey([&callback](opentracing::string_view key,
opentracing::string_view) -> opentracing::expected<void> {
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();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<opentracing::Values>)
Expand All @@ -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<opentracing::Values>)
Expand Down
2 changes: 1 addition & 1 deletion opentracing-shim/src/span_context_shim.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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()});
});
}

Expand Down
14 changes: 9 additions & 5 deletions opentracing-shim/src/span_shim.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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));
}
}

Expand All @@ -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<decltype(context_lock_)> 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
Expand All @@ -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<decltype(context_lock_)> 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<EventEntry> fields) noexcept
Expand Down Expand Up @@ -128,7 +132,7 @@ void SpanShim::logImpl(nostd::span<const EventEntry> 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)
{
Expand Down
13 changes: 7 additions & 6 deletions opentracing-shim/src/tracer_shim.cc
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,13 @@ std::unique_ptr<opentracing::Span> 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.
Expand Down
Loading