Skip to content

[API] reduce allocation in no span case #4254

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

Merged
marcalff merged 9 commits into from
Jul 17, 2026
Merged

[API] reduce allocation in no span case #4254

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
3336902
perf: reduce allocation
proost Jul 16, 2026
ead2aa8
doc: update change log
proost Jul 16, 2026
5bb2e06
Merge branch 'main' of github.com:open-telemetry/opentelemetry-cpp in…
proost Jul 16, 2026
cec5e30
Merge branch 'main' into perf-get-span-context
marcalff Jul 16, 2026
e4cfad2
Merge branch 'main' into perf-get-span-context
marcalff Jul 17, 2026
7d7b79a
fix: null shared ptr
proost Jul 17, 2026
ab3d8b0
Merge branch 'main' of github.com:open-telemetry/opentelemetry-cpp in…
proost Jul 17, 2026
8b3642d
Merge branch 'perf-get-span-context' of github.com:proost/opentelemet…
proost Jul 17, 2026
479e552
Merge branch 'main' into perf-get-span-context
marcalff Jul 17, 2026
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
5 changes: 5 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,11 @@ Increment the:
* [RELEASE] Bump main branch to 1.29.0-dev
[#4259](https://github.com/open-telemetry/opentelemetry-cpp/pull/4259)

* [API] Add `trace::GetSpanContext()` to read the active span's `SpanContext`
from a `Context` without the `DefaultSpan` allocation that
`GetSpan(context)->GetContext()` incurs, and use it at existing call sites.
[#4254](https://github.com/open-telemetry/opentelemetry-cpp/pull/4254)

## [1.28.0] 2026-07-16

* [RELEASE] Bump main branch to 1.28.0-dev
Expand Down
27 changes: 27 additions & 0 deletions api/include/opentelemetry/trace/context.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,33 @@ inline nostd::shared_ptr<Span> GetSpan(const context::Context &context) noexcept
return nostd::shared_ptr<Span>(new DefaultSpan(SpanContext::GetInvalid()));
}

// Get the SpanContext of the active span from explicit context. Falls back to a SpanContext
// stored directly in the context, and returns an invalid SpanContext if neither is present.
inline SpanContext GetSpanContext(const context::Context &context) noexcept
{
if (!context.HasKey(kSpanKey))
{
return SpanContext::GetInvalid();
}

const context::ContextValue context_value = context.GetValue(kSpanKey);

// Get the span metadata from the active span in the context.
if (const nostd::shared_ptr<Span> *maybe_span =
nostd::get_if<nostd::shared_ptr<Span>>(&context_value))

@ThomsonTan ThomsonTan Jul 16, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Could the active variant alternative contain an empty nostd::shared_ptr<Span> here? nostd::get_if would still return a non-null pointer to the stored shared_ptr, so this condition would succeed even though *maybe_span is null, and the next line would dereference it.

The same issue applies to maybe_span_context below. Should both branches also check that the stored shared_ptr is non-null and return SpanContext::GetInvalid() otherwise?

@proost proost Jul 17, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

You are right, Thank you!
7d7b79a

@marcalff marcalff Jul 17, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

On one hand, I am not sure if it is possible to add an empty nostd::shared_ptr<Span> into a context, so this extra check might not be needed.

On the other hand, now that this logic is in the API, being defensive and extra cautious is better ... especially because it needs to be robust to any version of the API/SDK.

So, this is a good change.

@dbarker dbarker Jul 17, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It is possible for the context to contain a span key and null span ptr value by passing a null span to SetSpan or when constructing a Scope object (neither check). Given the trace::GetSpan(context)->GetContext() pattern used before it seems it has not been a problem in practice but adding a check here shouldn't hurt.

{
return *maybe_span ? (*maybe_span)->GetContext() : SpanContext::GetInvalid();
}
// Get the span metadata directly from a SpanContext in the context.
// TODO: This path is unused and may be removed in the future.
if (const nostd::shared_ptr<SpanContext> *maybe_span_context =
nostd::get_if<nostd::shared_ptr<SpanContext>>(&context_value))
{
return *maybe_span_context ? **maybe_span_context : SpanContext::GetInvalid();
}
return SpanContext::GetInvalid();
}

// Check if the context is from a root span
inline bool IsRootSpan(const context::Context &context) noexcept
{
Expand Down
4 changes: 2 additions & 2 deletions api/include/opentelemetry/trace/propagation/b3_propagator.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ class B3Propagator : public B3PropagatorExtractor
void Inject(context::propagation::TextMapCarrier &carrier,
const context::Context &context) noexcept override
{
SpanContext span_context = trace::GetSpan(context)->GetContext();
SpanContext span_context = trace::GetSpanContext(context);
if (!span_context.IsValid())
{
return;
Expand Down Expand Up @@ -171,7 +171,7 @@ class B3PropagatorMultiHeader : public B3PropagatorExtractor
void Inject(context::propagation::TextMapCarrier &carrier,
const context::Context &context) noexcept override
{
SpanContext span_context = GetSpan(context)->GetContext();
SpanContext span_context = GetSpanContext(context);
if (!span_context.IsValid())
{
return;
Expand Down
2 changes: 1 addition & 1 deletion api/include/opentelemetry/trace/propagation/http_trace_context.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ class HttpTraceContext : public context::propagation::TextMapPropagator
void Inject(context::propagation::TextMapCarrier &carrier,
const context::Context &context) noexcept override
{
SpanContext span_context = trace::GetSpan(context)->GetContext();
SpanContext span_context = trace::GetSpanContext(context);

@dbarker dbarker Jul 16, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Not an ABI break. No observable behavior change.

if (!span_context.IsValid())
{
return;
Expand Down
2 changes: 1 addition & 1 deletion api/include/opentelemetry/trace/propagation/jaeger.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ class JaegerPropagator : public context::propagation::TextMapPropagator
void Inject(context::propagation::TextMapCarrier &carrier,
const context::Context &context) noexcept override
{
SpanContext span_context = trace::GetSpan(context)->GetContext();
SpanContext span_context = trace::GetSpanContext(context);
if (!span_context.IsValid())
{
return;
Expand Down
42 changes: 42 additions & 0 deletions api/test/trace/context_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,48 @@ TEST(TraceContextTest, GetSpan)
}
}

TEST(TraceContextTest, GetSpanContext)
{
{
context_api::Context context;
EXPECT_FALSE(trace_api::GetSpanContext(context).IsValid());
}

{
context_api::Context context;
auto input_span = MakeValidSpan();
auto context_with_span = trace_api::SetSpan(context, input_span);
auto span_context = trace_api::GetSpanContext(context_with_span);
EXPECT_TRUE(span_context.IsValid());
EXPECT_EQ(span_context, input_span->GetContext());
}

{
context_api::Context context;
auto context_with_null_span =
context.SetValue(trace_api::kSpanKey, nostd::shared_ptr<trace_api::Span>{});
EXPECT_FALSE(trace_api::GetSpanContext(context_with_null_span).IsValid());
}

{
context_api::Context context;
const auto input_span_context = MakeValidSpan()->GetContext();
auto context_with_span_context = context.SetValue(
trace_api::kSpanKey,
nostd::shared_ptr<trace_api::SpanContext>{new trace_api::SpanContext{input_span_context}});
auto span_context = trace_api::GetSpanContext(context_with_span_context);
EXPECT_TRUE(span_context.IsValid());
EXPECT_EQ(span_context, input_span_context);
}

{
context_api::Context context;
auto context_with_null_span_context =
context.SetValue(trace_api::kSpanKey, nostd::shared_ptr<trace_api::SpanContext>{});
EXPECT_FALSE(trace_api::GetSpanContext(context_with_null_span_context).IsValid());
}
}

TEST(TraceContextTest, SetSpan)
{
context_api::Context context;
Expand Down
2 changes: 1 addition & 1 deletion opentracing-shim/src/tracer_shim.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ opentracing::expected<std::unique_ptr<opentracing::SpanContext>> TracerShim::ext
CarrierReaderShim carrier{reader};
auto current_context = opentelemetry::context::RuntimeContext::GetCurrent();
auto context = propagator->Extract(carrier, current_context);
auto span_context = opentelemetry::trace::GetSpan(context)->GetContext();
auto span_context = opentelemetry::trace::GetSpanContext(context);
auto baggage = opentelemetry::baggage::GetBaggage(context);

// The operation MUST return a `SpanContext` Shim instance with the extracted values if any of
Expand Down
11 changes: 3 additions & 8 deletions sdk/include/opentelemetry/sdk/metrics/exemplar/reservoir_cell.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
# include "opentelemetry/sdk/metrics/data/metric_data.h"
# include "opentelemetry/sdk/metrics/exemplar/filter_type.h"
# include "opentelemetry/trace/context.h"
# include "opentelemetry/trace/span.h"
# include "opentelemetry/trace/span_context.h"
# include "opentelemetry/version.h"

Expand Down Expand Up @@ -135,14 +134,10 @@ class ReservoirCell
{
attributes_ = attributes;
record_time_ = opentelemetry::common::SystemTimestamp(std::chrono::system_clock::now());
auto span = opentelemetry::trace::GetSpan(context);
if (span)
const auto current_ctx = opentelemetry::trace::GetSpanContext(context);
if (current_ctx.IsValid())
{
auto current_ctx = span->GetContext();
if (current_ctx.IsValid())
{
context_.reset(new opentelemetry::trace::SpanContext{current_ctx});
}
context_.reset(new opentelemetry::trace::SpanContext{current_ctx});
}
}

Expand Down
34 changes: 2 additions & 32 deletions sdk/src/logs/logger.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,11 @@

#include "opentelemetry/common/timestamp.h"
#include "opentelemetry/context/context.h"
#include "opentelemetry/context/context_value.h"
#include "opentelemetry/context/runtime_context.h"
#include "opentelemetry/logs/event_id.h"
#include "opentelemetry/logs/log_record.h"
#include "opentelemetry/logs/noop.h"
#include "opentelemetry/logs/severity.h"
#include "opentelemetry/nostd/shared_ptr.h"
#include "opentelemetry/nostd/string_view.h"
#include "opentelemetry/nostd/unique_ptr.h"
#include "opentelemetry/nostd/variant.h"
Expand All @@ -26,10 +24,9 @@
#include "opentelemetry/sdk/logs/logger_context.h"
#include "opentelemetry/sdk/logs/processor.h"
#include "opentelemetry/sdk/logs/recordable.h"
#include "opentelemetry/trace/span.h"
#include "opentelemetry/trace/context.h"
#include "opentelemetry/trace/span_context.h"
#include "opentelemetry/trace/span_id.h"
#include "opentelemetry/trace/span_metadata.h"
#include "opentelemetry/trace/trace_flags.h"
#include "opentelemetry/version.h"

Expand All @@ -50,33 +47,6 @@ nostd::string_view GetEventName(const opentelemetry::logs::EventId &event_id) no
: nostd::string_view{};
}

trace_api::SpanContext ExtractSpanContextFromContext(const context::Context &context) noexcept
{
if (!context.HasKey(trace_api::kSpanKey))
{
return trace_api::SpanContext::GetInvalid();
}

const context::ContextValue context_value = context.GetValue(trace_api::kSpanKey);

// Get the span metadata from the active span in the context
if (const nostd::shared_ptr<trace_api::Span> *maybe_span =
nostd::get_if<nostd::shared_ptr<trace_api::Span>>(&context_value))
{
const nostd::shared_ptr<trace_api::Span> &span = *maybe_span;
return span->GetContext();
}
// Get the span metadata directly from a SpanContext in the context.
// TODO: This path is unused and may be removed in the future.
if (const nostd::shared_ptr<trace_api::SpanContext> *maybe_span_context =
nostd::get_if<nostd::shared_ptr<trace_api::SpanContext>>(&context_value))
{
const nostd::shared_ptr<trace_api::SpanContext> &span_context = *maybe_span_context;
return *span_context;
}
return trace_api::SpanContext::GetInvalid();
}

trace_api::SpanContext ExtractSpanContext(
const nostd::variant<trace_api::SpanContext, context::Context> &context_or_span) noexcept
{
Expand All @@ -86,7 +56,7 @@ trace_api::SpanContext ExtractSpanContext(
}
if (const context::Context *ctx = nostd::get_if<context::Context>(&context_or_span))
{
return ExtractSpanContextFromContext(*ctx);
return trace_api::GetSpanContext(*ctx);
}
return trace_api::SpanContext::GetInvalid();
}
Expand Down
2 changes: 1 addition & 1 deletion sdk/src/trace/tracer.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ nostd::shared_ptr<opentelemetry::trace::Span> Tracer::StartSpan(
else if (const context::Context *context = nostd::get_if<context::Context>(&options.parent))
{
// fetch span context from parent span stored in the context
auto parent_span_context = opentelemetry::trace::GetSpan(*context)->GetContext();
auto parent_span_context = opentelemetry::trace::GetSpanContext(*context);
if (parent_span_context.IsValid())
{
parent_context = parent_span_context;
Expand Down
Loading