From 59e421df87214b64ec15a13174f950daa5af9f3d Mon Sep 17 00:00:00 2001 From: Lalit Date: Tue, 3 May 2022 10:12:49 -0700 Subject: [PATCH 1/4] fix baggage propagation for empty/invalid baggage data --- .../baggage/propagation/baggage_propagator.h | 6 +++- .../propagation/baggage_propagator_test.cc | 29 +++++++++++++++++++ 2 files changed, 34 insertions(+), 1 deletion(-) diff --git a/api/include/opentelemetry/baggage/propagation/baggage_propagator.h b/api/include/opentelemetry/baggage/propagation/baggage_propagator.h index 11abd3107e..0d0214084d 100644 --- a/api/include/opentelemetry/baggage/propagation/baggage_propagator.h +++ b/api/include/opentelemetry/baggage/propagation/baggage_propagator.h @@ -21,7 +21,11 @@ class BaggagePropagator : public opentelemetry::context::propagation::TextMapPro const opentelemetry::context::Context &context) noexcept override { auto baggage = opentelemetry::baggage::GetBaggage(context); - carrier.Set(kBaggageHeader, baggage->ToHeader()); + auto header = baggage->ToHeader(); + if (header.size()) + { + carrier.Set(kBaggageHeader, header); + } } context::Context Extract(const opentelemetry::context::propagation::TextMapCarrier &carrier, diff --git a/api/test/baggage/propagation/baggage_propagator_test.cc b/api/test/baggage/propagation/baggage_propagator_test.cc index 0f1f8f338c..afe68581ad 100644 --- a/api/test/baggage/propagation/baggage_propagator_test.cc +++ b/api/test/baggage/propagation/baggage_propagator_test.cc @@ -82,3 +82,32 @@ TEST(BaggagePropagatorTest, ExtractAndInjectBaggage) EXPECT_EQ(fields[0], baggage::kBaggageHeader.data()); } } + +TEST(BaggagePropagatorTest, InjectEmptyHeader) +{ + // Test Missing baggage from context + BaggageCarrierTest carrier; + context::Context ctx = context::Context{}; + format.Inject(carrier, ctx); + EXPECT_EQ(carrier.headers_.find(baggage::kBaggageHeader), carrier.headers_.end()); + + { + // Test empty baggage in context + BaggageCarrierTest carrier1; + carrier1.headers_[baggage::kBaggageHeader.data()] = ""; + context::Context ctx1 = context::Context{}; + context::Context ctx2 = format.Extract(carrier1, ctx1); + format.Inject(carrier, ctx2); + EXPECT_EQ(carrier.headers_.find(baggage::kBaggageHeader), carrier.headers_.end()); + } + { + // Invali baggage in context + BaggageCarrierTest carrier1; + carrier1.headers_[baggage::kBaggageHeader.data()] = "InvalidBaggageData"; + context::Context ctx1 = context::Context{}; + context::Context ctx2 = format.Extract(carrier1, ctx1); + + format.Inject(carrier, ctx2); + EXPECT_EQ(carrier.headers_.find(baggage::kBaggageHeader), carrier.headers_.end()); + } +} From c18cd498ff84629289725c99484254dcc67e591a Mon Sep 17 00:00:00 2001 From: Lalit Kumar Bhasin Date: Tue, 3 May 2022 10:24:40 -0700 Subject: [PATCH 2/4] Update api/test/baggage/propagation/baggage_propagator_test.cc Co-authored-by: Tom Tan --- api/test/baggage/propagation/baggage_propagator_test.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/test/baggage/propagation/baggage_propagator_test.cc b/api/test/baggage/propagation/baggage_propagator_test.cc index afe68581ad..94fe8b005b 100644 --- a/api/test/baggage/propagation/baggage_propagator_test.cc +++ b/api/test/baggage/propagation/baggage_propagator_test.cc @@ -101,7 +101,7 @@ TEST(BaggagePropagatorTest, InjectEmptyHeader) EXPECT_EQ(carrier.headers_.find(baggage::kBaggageHeader), carrier.headers_.end()); } { - // Invali baggage in context + // Invalid baggage in context BaggageCarrierTest carrier1; carrier1.headers_[baggage::kBaggageHeader.data()] = "InvalidBaggageData"; context::Context ctx1 = context::Context{}; From 205633120ae6a361c6388641805c74c380eb580e Mon Sep 17 00:00:00 2001 From: Lalit Date: Tue, 3 May 2022 11:25:38 -0700 Subject: [PATCH 3/4] review comment --- .../baggage/propagation/baggage_propagator.h | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/api/include/opentelemetry/baggage/propagation/baggage_propagator.h b/api/include/opentelemetry/baggage/propagation/baggage_propagator.h index 0d0214084d..f85f322c7a 100644 --- a/api/include/opentelemetry/baggage/propagation/baggage_propagator.h +++ b/api/include/opentelemetry/baggage/propagation/baggage_propagator.h @@ -33,7 +33,15 @@ class BaggagePropagator : public opentelemetry::context::propagation::TextMapPro { nostd::string_view baggage_str = carrier.Get(opentelemetry::baggage::kBaggageHeader); auto baggage = opentelemetry::baggage::Baggage::FromHeader(baggage_str); - return opentelemetry::baggage::SetBaggage(context, baggage); + + if (baggage->ToHeader().size()) + { + return opentelemetry::baggage::SetBaggage(context, baggage); + } + else + { + return context::Context(context); + } } bool Fields(nostd::function_ref callback) const noexcept override From 03fa9c2146dfb39aa1330d8a84bc31123dd39d9b Mon Sep 17 00:00:00 2001 From: Lalit Kumar Bhasin Date: Tue, 3 May 2022 12:22:42 -0700 Subject: [PATCH 4/4] Update api/include/opentelemetry/baggage/propagation/baggage_propagator.h Co-authored-by: Ehsan Saei <71217171+esigo@users.noreply.github.com> --- .../opentelemetry/baggage/propagation/baggage_propagator.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/include/opentelemetry/baggage/propagation/baggage_propagator.h b/api/include/opentelemetry/baggage/propagation/baggage_propagator.h index f85f322c7a..3de60860b2 100644 --- a/api/include/opentelemetry/baggage/propagation/baggage_propagator.h +++ b/api/include/opentelemetry/baggage/propagation/baggage_propagator.h @@ -40,7 +40,7 @@ class BaggagePropagator : public opentelemetry::context::propagation::TextMapPro } else { - return context::Context(context); + return context; } }