Skip to content

Commit 560d70e

Browse files
dbarkerdbarker
authored
[API] Optimize trace propagators (#4696)
1 parent 1ccaa35 commit 560d70e

11 files changed

Lines changed: 443 additions & 78 deletions

File tree

‎api/include/opentelemetry/baggage/baggage.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -257,6 +257,7 @@ class OPENTELEMETRY_EXPORT Baggage
257257
};
258258

259259
std::string ret;
260+
ret.reserve(str.size());
260261

261262
for (size_t i = 0; i < str.size(); i++)
262263
{

‎api/include/opentelemetry/baggage/propagation/baggage_propagator.h‎

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -27,20 +27,23 @@ class BaggagePropagator : public context::propagation::TextMapPropagator
2727
const context::Context &context) noexcept override
2828
{
2929
auto baggage = baggage::GetBaggage(context);
30-
auto header = baggage->ToHeader();
31-
if (header.size())
30+
if (HasEntries(baggage))
3231
{
33-
carrier.Set(kBaggageHeader, header);
32+
carrier.Set(kBaggageHeader, baggage->ToHeader());
3433
}
3534
}
3635

3736
context::Context Extract(const context::propagation::TextMapCarrier &carrier,
3837
context::Context &context) noexcept override
3938
{
4039
nostd::string_view baggage_str = carrier.Get(baggage::kBaggageHeader);
41-
auto baggage = baggage::Baggage::FromHeader(baggage_str);
40+
if (baggage_str.empty())
41+
{
42+
return context;
43+
}
44+
auto baggage = baggage::Baggage::FromHeader(baggage_str);
4245

43-
if (baggage->ToHeader().size())
46+
if (HasEntries(baggage))
4447
{
4548
return baggage::SetBaggage(context, baggage);
4649
}
@@ -54,6 +57,13 @@ class BaggagePropagator : public context::propagation::TextMapPropagator
5457
{
5558
return callback(kBaggageHeader);
5659
}
60+
61+
private:
62+
static bool HasEntries(const nostd::shared_ptr<Baggage> &baggage) noexcept
63+
{
64+
return baggage &&
65+
!baggage->GetAllEntries([](nostd::string_view, nostd::string_view) { return false; });
66+
}
5767
};
5868
} // namespace propagation
5969
} // namespace baggage

‎api/include/opentelemetry/trace/propagation/b3_propagator.h‎

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,13 @@
44
#pragma once
55

66
#include <array>
7+
#include <memory>
8+
#include <new>
9+
#include <utility>
710

811
#include "detail/hex.h"
912
#include "detail/string.h"
13+
#include "opentelemetry/common/macros.h"
1014
#include "opentelemetry/context/propagation/text_map_propagator.h"
1115
#include "opentelemetry/trace/context.h"
1216
#include "opentelemetry/trace/default_span.h"
@@ -50,10 +54,22 @@ class B3PropagatorExtractor : public context::propagation::TextMapPropagator
5054
context::Context &context) noexcept override
5155
{
5256
SpanContext span_context = ExtractImpl(carrier);
53-
nostd::shared_ptr<Span> sp{new DefaultSpan(span_context)};
5457
if (span_context.IsValid())
5558
{
56-
return trace::SetSpan(context, sp);
59+
nostd::shared_ptr<Span> span;
60+
#if OPENTELEMETRY_HAVE_EXCEPTIONS
61+
try
62+
{
63+
span = nostd::shared_ptr<Span>{std::make_shared<DefaultSpan>(std::move(span_context))};
64+
}
65+
catch (const std::bad_alloc &)
66+
{
67+
return context;
68+
}
69+
#else
70+
span = nostd::shared_ptr<Span>{std::make_shared<DefaultSpan>(std::move(span_context))};
71+
#endif
72+
return trace::SetSpan(context, span);
5773
}
5874
else
5975
{

‎api/include/opentelemetry/trace/propagation/http_trace_context.h‎

Lines changed: 23 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,13 @@
44
#pragma once
55

66
#include <array>
7+
#include <memory>
8+
#include <new>
9+
#include <utility>
710

811
#include "detail/hex.h"
912
#include "detail/string.h"
13+
#include "opentelemetry/common/macros.h"
1014
#include "opentelemetry/context/propagation/text_map_propagator.h"
1115
#include "opentelemetry/nostd/function_ref.h"
1216
#include "opentelemetry/nostd/shared_ptr.h"
@@ -53,10 +57,22 @@ class HttpTraceContext : public context::propagation::TextMapPropagator
5357
context::Context &context) noexcept override
5458
{
5559
SpanContext span_context = ExtractImpl(carrier);
56-
nostd::shared_ptr<Span> sp{new DefaultSpan(span_context)};
5760
if (span_context.IsValid())
5861
{
59-
return trace::SetSpan(context, sp);
62+
nostd::shared_ptr<Span> span;
63+
#if OPENTELEMETRY_HAVE_EXCEPTIONS
64+
try
65+
{
66+
span = nostd::shared_ptr<Span>{std::make_shared<DefaultSpan>(std::move(span_context))};
67+
}
68+
catch (const std::bad_alloc &)
69+
{
70+
return context;
71+
}
72+
#else
73+
span = nostd::shared_ptr<Span>{std::make_shared<DefaultSpan>(std::move(span_context))};
74+
#endif
75+
return trace::SetSpan(context, span);
6076
}
6177
else
6278
{
@@ -106,10 +122,10 @@ class HttpTraceContext : public context::propagation::TextMapPropagator
106122
nostd::span<char, 2>{&trace_parent[kTraceIdSize + kSpanIdSize + 5], 2});
107123

108124
carrier.Set(kTraceParent, nostd::string_view(trace_parent, sizeof(trace_parent)));
109-
const auto trace_state = span_context.trace_state()->ToHeader();
110-
if (!trace_state.empty())
125+
const auto &trace_state = span_context.trace_state();
126+
if (trace_state && !trace_state->Empty())
111127
{
112-
carrier.Set(kTraceState, trace_state);
128+
carrier.Set(kTraceState, trace_state->ToHeader());
113129
}
114130
}
115131

@@ -175,7 +191,8 @@ class HttpTraceContext : public context::propagation::TextMapPropagator
175191
}
176192

177193
return SpanContext(trace_id, span_id, TraceFlagsFromHex(trace_flags_hex), true,
178-
trace::TraceState::FromHeader(trace_state));
194+
trace_state.empty() ? trace::TraceState::GetDefault()
195+
: trace::TraceState::FromHeader(trace_state));
179196
}
180197

181198
static SpanContext ExtractImpl(const context::propagation::TextMapCarrier &carrier)

‎api/include/opentelemetry/trace/propagation/jaeger.h‎

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,13 @@
33

44
#pragma once
55

6+
#include <memory>
7+
#include <new>
8+
#include <utility>
9+
610
#include "detail/hex.h"
711
#include "detail/string.h"
12+
#include "opentelemetry/common/macros.h"
813
#include "opentelemetry/context/propagation/text_map_propagator.h"
914
#include "opentelemetry/trace/context.h"
1015
#include "opentelemetry/trace/default_span.h"
@@ -52,10 +57,22 @@ class JaegerPropagator : public context::propagation::TextMapPropagator
5257
context::Context &context) noexcept override
5358
{
5459
SpanContext span_context = ExtractImpl(carrier);
55-
nostd::shared_ptr<Span> sp{new DefaultSpan(span_context)};
5660
if (span_context.IsValid())
5761
{
58-
return trace::SetSpan(context, sp);
62+
nostd::shared_ptr<Span> span;
63+
#if OPENTELEMETRY_HAVE_EXCEPTIONS
64+
try
65+
{
66+
span = nostd::shared_ptr<Span>{std::make_shared<DefaultSpan>(std::move(span_context))};
67+
}
68+
catch (const std::bad_alloc &)
69+
{
70+
return context;
71+
}
72+
#else
73+
span = nostd::shared_ptr<Span>{std::make_shared<DefaultSpan>(std::move(span_context))};
74+
#endif
75+
return trace::SetSpan(context, span);
5976
}
6077
else
6178
{

‎api/include/opentelemetry/trace/trace_state.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -104,9 +104,9 @@ class OPENTELEMETRY_EXPORT TraceState
104104
{
105105
first = false;
106106
}
107-
header_s.append(std::string(key.data(), key.size()));
107+
header_s.append(key.data(), key.size());
108108
header_s.append(1, kKeyValueSeparator);
109-
header_s.append(std::string(value.data(), value.size()));
109+
header_s.append(value.data(), value.size());
110110
return true;
111111
});
112112
return header_s;

‎api/test/baggage/propagation/baggage_propagator_test.cc‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
#include "opentelemetry/context/context.h"
1616
#include "opentelemetry/context/propagation/text_map_propagator.h"
1717
#include "opentelemetry/nostd/function_ref.h"
18+
#include "opentelemetry/nostd/shared_ptr.h"
1819
#include "opentelemetry/nostd/string_view.h"
1920

2021
using namespace opentelemetry;
@@ -56,6 +57,33 @@ TEST(BaggagePropagatorTest, ExtractNoBaggageHeader)
5657
EXPECT_EQ(ctx2_baggage->ToHeader(), "");
5758
}
5859

60+
TEST(BaggagePropagatorTest, ExtractEmptyOrInvalidHeaderPreservesBaggage)
61+
{
62+
auto original_baggage = baggage::Baggage::FromHeader("key=value");
63+
context::Context base;
64+
auto ctx = baggage::SetBaggage(base, original_baggage);
65+
66+
for (const auto *header : {"", "invalid_header"})
67+
{
68+
BaggageCarrierTest carrier;
69+
carrier.Set(baggage::kBaggageHeader, header);
70+
auto extracted = format.Extract(carrier, ctx);
71+
EXPECT_EQ(baggage::GetBaggage(extracted), original_baggage);
72+
EXPECT_EQ(baggage::GetBaggage(ctx), original_baggage);
73+
}
74+
}
75+
76+
TEST(BaggagePropagatorTest, InjectNullBaggage)
77+
{
78+
BaggageCarrierTest carrier;
79+
context::Context base;
80+
auto ctx = baggage::SetBaggage(base, nostd::shared_ptr<baggage::Baggage>{});
81+
82+
format.Inject(carrier, ctx);
83+
84+
EXPECT_TRUE(carrier.headers_.empty());
85+
}
86+
5987
TEST(BaggagePropagatorTest, ExtractAndInjectBaggage)
6088
{
6189
// create header string for baggage larger than allowed size (kMaxKeyValueSize)

‎api/test/trace/propagation/b3_propagation_test.cc‎

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
#include <map>
77
#include <string>
88
#include <utility>
9+
#include <vector>
910
#include "util.h"
1011

1112
#include "opentelemetry/context/context.h"
@@ -64,6 +65,28 @@ TEST(B3PropagationTest, TraceFlagsBufferGeneration)
6465
EXPECT_EQ(MapB3Context::TraceFlagsFromHex("1"), trace::TraceFlags(trace::TraceFlags::kIsSampled));
6566
}
6667

68+
TEST(B3PropagationTest, PropagateEmptyContext)
69+
{
70+
TextMapCarrierTest carrier;
71+
context::Context ctx;
72+
format.Inject(carrier, ctx);
73+
EXPECT_TRUE(carrier.headers_.empty());
74+
formatMultiHeader.Inject(carrier, ctx);
75+
EXPECT_TRUE(carrier.headers_.empty());
76+
}
77+
78+
TEST(B3PropagationTest, ExtractMissingHeaders)
79+
{
80+
TextMapCarrierTest carrier;
81+
context::Context ctx;
82+
auto extracted = format.Extract(carrier, ctx);
83+
EXPECT_FALSE(nostd::holds_alternative<nostd::shared_ptr<trace::Span>>(
84+
extracted.GetValue(trace::kSpanKey)));
85+
extracted = formatMultiHeader.Extract(carrier, ctx);
86+
EXPECT_FALSE(nostd::holds_alternative<nostd::shared_ptr<trace::Span>>(
87+
extracted.GetValue(trace::kSpanKey)));
88+
}
89+
6790
TEST(B3PropagationTest, PropagateInvalidContext)
6891
{
6992
// Do not propagate invalid trace context.
@@ -73,6 +96,50 @@ TEST(B3PropagationTest, PropagateInvalidContext)
7396
nostd::shared_ptr<trace::Span>(new trace::DefaultSpan(trace::SpanContext::GetInvalid()))};
7497
format.Inject(carrier, ctx);
7598
EXPECT_TRUE(carrier.headers_.count("b3") == 0);
99+
formatMultiHeader.Inject(carrier, ctx);
100+
EXPECT_TRUE(carrier.headers_.empty());
101+
}
102+
103+
TEST(B3PropagationTest, ExtractInvalidMultiHeaderContext)
104+
{
105+
const std::vector<std::pair<std::string, std::string>> identities = {
106+
{"", "e457b5a2e4d86bd1"},
107+
{"80f198ee56343ba864fe8b2a57d3eff7", ""},
108+
{"00000000000000000000000000000000", "e457b5a2e4d86bd1"},
109+
{"80f198ee56343ba864fe8b2a57d3eff7", "0000000000000000"},
110+
{"0000000zzz0000000000000000000000", "e457b5a2e4d86bd1"},
111+
{"80f198ee56343ba864fe8b2a57d3eff7", "0000000zzz000000"},
112+
};
113+
trace::SpanContext span_context{MapB3Context::TraceIdFromHex("80f198ee56343ba864fe8b2a57d3eff7"),
114+
MapB3Context::SpanIdFromHex("e457b5a2e4d86bd1"),
115+
trace::TraceFlags{1}, false};
116+
nostd::shared_ptr<trace::Span> existing_span{new trace::DefaultSpan{span_context}};
117+
118+
for (const auto &identity : identities)
119+
{
120+
SCOPED_TRACE(identity.first + ":" + identity.second);
121+
TextMapCarrierTest carrier;
122+
carrier.headers_["X-B3-Sampled"] = "1";
123+
if (!identity.first.empty())
124+
{
125+
carrier.headers_["X-B3-TraceId"] = identity.first;
126+
}
127+
if (!identity.second.empty())
128+
{
129+
carrier.headers_["X-B3-SpanId"] = identity.second;
130+
}
131+
132+
context::Context empty_context;
133+
auto extracted = formatMultiHeader.Extract(carrier, empty_context);
134+
EXPECT_FALSE(nostd::holds_alternative<nostd::shared_ptr<trace::Span>>(
135+
extracted.GetValue(trace::kSpanKey)));
136+
137+
context::Context existing_context{trace::kSpanKey, existing_span};
138+
extracted = formatMultiHeader.Extract(carrier, existing_context);
139+
auto span_value = extracted.GetValue(trace::kSpanKey);
140+
ASSERT_TRUE(nostd::holds_alternative<nostd::shared_ptr<trace::Span>>(span_value));
141+
EXPECT_EQ(nostd::get<nostd::shared_ptr<trace::Span>>(span_value).get(), existing_span.get());
142+
}
76143
}
77144

78145
TEST(B3PropagationTest, ExtractInvalidContext)

‎api/test/trace/propagation/http_text_format_test.cc‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,23 @@ TEST(TextMapPropagatorTest, NoSendEmptyTraceState)
100100
EXPECT_FALSE(carrier2.headers_.count("tracestate") > 0);
101101
}
102102

103+
TEST(TextMapPropagatorTest, NoSendNullTraceState)
104+
{
105+
TextMapCarrierTest carrier;
106+
trace::SpanContext span_context{
107+
MapHttpTraceContext::TraceIdFromHex("4bf92f3577b34da6a3ce929d0e0e4736"),
108+
MapHttpTraceContext::SpanIdFromHex("0102030405060708"), trace::TraceFlags{true}, false,
109+
nostd::shared_ptr<trace::TraceState>{}};
110+
context::Context ctx{trace::kSpanKey,
111+
nostd::shared_ptr<trace::Span>{new trace::DefaultSpan(span_context)}};
112+
113+
format.Inject(carrier, ctx);
114+
115+
EXPECT_EQ(carrier.headers_["traceparent"],
116+
"00-4bf92f3577b34da6a3ce929d0e0e4736-0102030405060708-01");
117+
EXPECT_EQ(carrier.headers_.count("tracestate"), 0);
118+
}
119+
103120
TEST(TextMapPropagatorTest, PropogateTraceState)
104121
{
105122
TextMapCarrierTest carrier;
@@ -118,6 +135,23 @@ TEST(TextMapPropagatorTest, PropogateTraceState)
118135
EXPECT_EQ(carrier2.headers_["tracestate"], "congo=t61rcWkgMzE");
119136
}
120137

138+
TEST(TextMapPropagatorTest, PropagateEmptyContext)
139+
{
140+
TextMapCarrierTest carrier;
141+
context::Context ctx;
142+
format.Inject(carrier, ctx);
143+
EXPECT_TRUE(carrier.headers_.empty());
144+
}
145+
146+
TEST(TextMapPropagatorTest, ExtractMissingHeaders)
147+
{
148+
TextMapCarrierTest carrier;
149+
context::Context ctx;
150+
auto extracted = format.Extract(carrier, ctx);
151+
EXPECT_FALSE(nostd::holds_alternative<nostd::shared_ptr<trace::Span>>(
152+
extracted.GetValue(trace::kSpanKey)));
153+
}
154+
121155
TEST(TextMapPropagatorTest, PropagateInvalidContext)
122156
{
123157
// Do not propagate invalid trace context.
@@ -199,6 +233,8 @@ TEST(TextMapPropagatorTest, InvalidIdentitiesAreNotExtracted)
199233
"00-0af7651916cd43dd8448eb211c8031-b9c7c989f97918e1-01",
200234
"00-0af7651916cd43dd8448eb211c80319c-b9c7c989f97-01",
201235
"00-1-1-00",
236+
"00-00000000000000000000000000000000-b9c7c989f97918e1-01",
237+
"00-0af7651916cd43dd8448eb211c80319c-0000000000000000-01",
202238
"00--b9c7c989f97918e1-01",
203239
"00-0af7651916cd43dd8448eb211c80319c1--01",
204240
"",

0 commit comments

Comments
 (0)