Skip to content

[CPLAT-4487] use custom header for tracing #2

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
bellatoris merged 1 commit into from
Nov 4, 2024
Merged

[CPLAT-4487] use custom header for tracing #2

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
6 changes: 3 additions & 3 deletions .circleci/config.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ workflows:
requires:
- build_arm64
- build_x64


executors:
arm64:
Expand Down Expand Up @@ -161,7 +161,7 @@ jobs:
- run:
name: Building envoy
command: |
./ci/run_envoy_docker.sh 'ci/do_ci.sh release.server_only'
./ci/run_envoy_docker.sh 'ci/do_ci.sh release test/extensions/tracers/opentelemetry:*'
mkdir release_archive
ARCH=$(uname -m)
if [[ ${ARCH} == "x86_64" ]]; then
Expand All @@ -172,4 +172,4 @@ jobs:

- persist_to_workspace:
root: .
paths: release_archive
paths: release_archive
4 changes: 2 additions & 2 deletions source/extensions/tracers/opentelemetry/span_context_extractor.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ namespace OpenTelemetry {

class OpenTelemetryConstantValues {
public:
const Tracing::TraceContextHandler TRACE_PARENT{"traceparent"};
Copy link

@dlunch dlunch Nov 4, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

It would be better to extract header name this into configuration, we can upstream it if we do

Copy link
Author

@bellatoris bellatoris Nov 4, 2024

Choose a reason for hiding this comment

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

Ok let me create a ticker for that.

const Tracing::TraceContextHandler TRACE_STATE{"tracestate"};
const Tracing::TraceContextHandler TRACE_PARENT{"x-sendbird-traceparent"};
const Tracing::TraceContextHandler TRACE_STATE{"x-sendbird-tracestate"};
};

using OpenTelemetryConstants = ConstSingleton<OpenTelemetryConstantValues>;
Expand Down
4 changes: 2 additions & 2 deletions source/extensions/tracers/opentelemetry/tracer.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,11 @@ using opentelemetry::proto::collector::trace::v1::ExportTraceServiceRequest;
namespace {

const Tracing::TraceContextHandler& traceParentHeader() {
CONSTRUCT_ON_FIRST_USE(Tracing::TraceContextHandler, "traceparent");
CONSTRUCT_ON_FIRST_USE(Tracing::TraceContextHandler, "x-sendbird-traceparent");
}

const Tracing::TraceContextHandler& traceStateHeader() {
CONSTRUCT_ON_FIRST_USE(Tracing::TraceContextHandler, "tracestate");
CONSTRUCT_ON_FIRST_USE(Tracing::TraceContextHandler, "x-sendbird-tracestate");
}

void callSampler(SamplerSharedPtr sampler, const absl::optional<SpanContext> span_context,
Expand Down
16 changes: 8 additions & 8 deletions ...extensions/tracers/opentelemetry/samplers/always_on/always_on_sampler_integration_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ INSTANTIATE_TEST_SUITE_P(IpVersions, AlwaysOnSamplerIntegrationTest,
TEST_P(AlwaysOnSamplerIntegrationTest, TestWithTraceparentAndTracestate) {
Http::TestRequestHeaderMapImpl request_headers{
{":method", "GET"}, {":path", "/test/long/url"}, {":scheme", "http"},
{":authority", "host"}, {"tracestate", "key=value"}, {"traceparent", TRACEPARENT_VALUE}};
{":authority", "host"}, {"x-sendbird-tracestate", "key=value"}, {"x-sendbird-traceparent", TRACEPARENT_VALUE}};

auto response = sendRequestAndWaitForResponse(request_headers, 0, default_response_headers_, 0);

Expand All @@ -71,14 +71,14 @@ TEST_P(AlwaysOnSamplerIntegrationTest, TestWithTraceparentAndTracestate) {

// traceparent should be set: traceid should be re-used, span id should be different
absl::string_view traceparent_value = upstream_request_->headers()
.get(Http::LowerCaseString("traceparent"))[0]
.get(Http::LowerCaseString("x-sendbird-traceparent"))[0]
->value()
.getStringView();
EXPECT_TRUE(absl::StartsWith(traceparent_value, TRACEPARENT_VALUE_START));
EXPECT_NE(TRACEPARENT_VALUE, traceparent_value);
// tracestate should be forwarded
absl::string_view tracestate_value = upstream_request_->headers()
.get(Http::LowerCaseString("tracestate"))[0]
.get(Http::LowerCaseString("x-sendbird-tracestate"))[0]
->value()
.getStringView();
EXPECT_EQ("key=value", tracestate_value);
Expand All @@ -90,7 +90,7 @@ TEST_P(AlwaysOnSamplerIntegrationTest, TestWithTraceparentOnly) {
{":path", "/test/long/url"},
{":scheme", "http"},
{":authority", "host"},
{"traceparent", TRACEPARENT_VALUE}};
{"x-sendbird-traceparent", TRACEPARENT_VALUE}};
auto response = sendRequestAndWaitForResponse(request_headers, 0, default_response_headers_, 0);

ASSERT_TRUE(response->waitForEndStream());
Expand All @@ -99,14 +99,14 @@ TEST_P(AlwaysOnSamplerIntegrationTest, TestWithTraceparentOnly) {

// traceparent should be set: traceid should be re-used, span id should be different
absl::string_view traceparent_value = upstream_request_->headers()
.get(Http::LowerCaseString("traceparent"))[0]
.get(Http::LowerCaseString("x-sendbird-traceparent"))[0]
->value()
.getStringView();
EXPECT_TRUE(absl::StartsWith(traceparent_value, TRACEPARENT_VALUE_START));
EXPECT_NE(TRACEPARENT_VALUE, traceparent_value);
// OTLP tracer adds an empty tracestate
absl::string_view tracestate_value = upstream_request_->headers()
.get(Http::LowerCaseString("tracestate"))[0]
.get(Http::LowerCaseString("x-sendbird-tracestate"))[0]
->value()
.getStringView();
EXPECT_EQ("", tracestate_value);
Expand All @@ -125,11 +125,11 @@ TEST_P(AlwaysOnSamplerIntegrationTest, TestWithoutTraceparentAndTracestate) {

// traceparent will be added, trace_id and span_id will be generated, so there is nothing we can
// assert
EXPECT_EQ(upstream_request_->headers().get(::Envoy::Http::LowerCaseString("traceparent")).size(),
EXPECT_EQ(upstream_request_->headers().get(::Envoy::Http::LowerCaseString("x-sendbird-traceparent")).size(),
1);
// OTLP tracer adds an empty tracestate
absl::string_view tracestate_value = upstream_request_->headers()
.get(Http::LowerCaseString("tracestate"))[0]
.get(Http::LowerCaseString("x-sendbird-tracestate"))[0]
->value()
.getStringView();
EXPECT_EQ("", tracestate_value);
Expand Down
16 changes: 8 additions & 8 deletions ...extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_sampler_integration_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ TEST_P(DynatraceSamplerIntegrationTest, TestWithTraceparentAndTracestate) {
// tracestate does not contain a Dynatrace tag
Http::TestRequestHeaderMapImpl request_headers{
{":method", "GET"}, {":path", "/test/long/url"}, {":scheme", "http"},
{":authority", "host"}, {"tracestate", "key=value"}, {"traceparent", TRACEPARENT_VALUE}};
{":authority", "host"}, {"x-sendbird-tracestate", "key=value"}, {"x-sendbird-traceparent", TRACEPARENT_VALUE}};

auto response = sendRequestAndWaitForResponse(request_headers, 0, default_response_headers_, 0);

Expand All @@ -73,14 +73,14 @@ TEST_P(DynatraceSamplerIntegrationTest, TestWithTraceparentAndTracestate) {

// traceparent should be set: traceid should be re-used, span id should be different
absl::string_view traceparent_value = upstream_request_->headers()
.get(Http::LowerCaseString("traceparent"))[0]
.get(Http::LowerCaseString("x-sendbird-traceparent"))[0]
->value()
.getStringView();
EXPECT_TRUE(absl::StartsWith(traceparent_value, TRACEPARENT_VALUE_START));
EXPECT_NE(TRACEPARENT_VALUE, traceparent_value);
// Dynatrace tracestate should be added to existing tracestate
absl::string_view tracestate_value = upstream_request_->headers()
.get(Http::LowerCaseString("tracestate"))[0]
.get(Http::LowerCaseString("x-sendbird-tracestate"))[0]
->value()
.getStringView();
// use StartsWith because path-info (last element in trace state) contains a random value
Expand All @@ -96,7 +96,7 @@ TEST_P(DynatraceSamplerIntegrationTest, TestWithTraceparentOnly) {
{":path", "/test/long/url"},
{":scheme", "http"},
{":authority", "host"},
{"traceparent", TRACEPARENT_VALUE}};
{"x-sendbird-traceparent", TRACEPARENT_VALUE}};
auto response = sendRequestAndWaitForResponse(request_headers, 0, default_response_headers_, 0);

ASSERT_TRUE(response->waitForEndStream());
Expand All @@ -105,14 +105,14 @@ TEST_P(DynatraceSamplerIntegrationTest, TestWithTraceparentOnly) {

// traceparent should be set: traceid should be re-used, span id should be different
absl::string_view traceparent_value = upstream_request_->headers()
.get(Http::LowerCaseString("traceparent"))[0]
.get(Http::LowerCaseString("x-sendbird-traceparent"))[0]
->value()
.getStringView();
EXPECT_TRUE(absl::StartsWith(traceparent_value, TRACEPARENT_VALUE_START));
EXPECT_NE(TRACEPARENT_VALUE, traceparent_value);
// Dynatrace tag should be added to tracestate
absl::string_view tracestate_value = upstream_request_->headers()
.get(Http::LowerCaseString("tracestate"))[0]
.get(Http::LowerCaseString("x-sendbird-tracestate"))[0]
->value()
.getStringView();
// use StartsWith because path-info (last element in trace state contains a random value)
Expand All @@ -133,11 +133,11 @@ TEST_P(DynatraceSamplerIntegrationTest, TestWithoutTraceparentAndTracestate) {

// traceparent will be added, trace_id and span_id will be generated, so there is nothing we can
// assert
EXPECT_EQ(upstream_request_->headers().get(::Envoy::Http::LowerCaseString("traceparent")).size(),
EXPECT_EQ(upstream_request_->headers().get(::Envoy::Http::LowerCaseString("x-sendbird-traceparent")).size(),
1);
// Dynatrace tag should be added to tracestate
absl::string_view tracestate_value = upstream_request_->headers()
.get(Http::LowerCaseString("tracestate"))[0]
.get(Http::LowerCaseString("x-sendbird-tracestate"))[0]
->value()
.getStringView();
EXPECT_TRUE(absl::StartsWith(tracestate_value, "5b3f9fed-980df25c@dt=fw4;0;0;0;0;0;0;"))
Expand Down
32 changes: 16 additions & 16 deletions test/extensions/tracers/opentelemetry/span_context_extractor_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ constexpr absl::string_view trace_flags = "01";

TEST(SpanContextExtractorTest, ExtractSpanContext) {
Tracing::TestTraceContextImpl request_headers{
{"traceparent", fmt::format("{}-{}-{}-{}", version, trace_id, parent_id, trace_flags)}};
{"x-sendbird-traceparent", fmt::format("{}-{}-{}-{}", version, trace_id, parent_id, trace_flags)}};

SpanContextExtractor span_context_extractor(request_headers);
absl::StatusOr<SpanContext> span_context = span_context_extractor.extractSpanContext();
Expand All @@ -38,7 +38,7 @@ TEST(SpanContextExtractorTest, ExtractSpanContext) {
TEST(SpanContextExtractorTest, ExtractSpanContextNotSampled) {
const std::string trace_flags_unsampled{"00"};
Tracing::TestTraceContextImpl request_headers{
{"traceparent",
{"x-sendbird-traceparent",
fmt::format("{}-{}-{}-{}", version, trace_id, parent_id, trace_flags_unsampled)}};
SpanContextExtractor span_context_extractor(request_headers);
absl::StatusOr<SpanContext> span_context = span_context_extractor.extractSpanContext();
Expand All @@ -62,7 +62,7 @@ TEST(SpanContextExtractorTest, ThrowsExceptionWithoutHeader) {

TEST(SpanContextExtractorTest, ThrowsExceptionWithTooLongHeader) {
Tracing::TestTraceContextImpl request_headers{
{"traceparent", fmt::format("000{}-{}-{}-{}", version, trace_id, parent_id, trace_flags)}};
{"x-sendbird-traceparent", fmt::format("000{}-{}-{}-{}", version, trace_id, parent_id, trace_flags)}};
SpanContextExtractor span_context_extractor(request_headers);

absl::StatusOr<SpanContext> span_context = span_context_extractor.extractSpanContext();
Expand All @@ -73,7 +73,7 @@ TEST(SpanContextExtractorTest, ThrowsExceptionWithTooLongHeader) {

TEST(SpanContextExtractorTest, ThrowsExceptionWithTooShortHeader) {
Tracing::TestTraceContextImpl request_headers{
{"traceparent", fmt::format("{}-{}-{}", trace_id, parent_id, trace_flags)}};
{"x-sendbird-traceparent", fmt::format("{}-{}-{}", trace_id, parent_id, trace_flags)}};
SpanContextExtractor span_context_extractor(request_headers);

absl::StatusOr<SpanContext> span_context = span_context_extractor.extractSpanContext();
Expand All @@ -84,7 +84,7 @@ TEST(SpanContextExtractorTest, ThrowsExceptionWithTooShortHeader) {

TEST(SpanContextExtractorTest, ThrowsExceptionWithInvalidHyphenation) {
Tracing::TestTraceContextImpl request_headers{
{"traceparent", fmt::format("{}{}-{}-{}", version, trace_id, parent_id, trace_flags)}};
{"x-sendbird-traceparent", fmt::format("{}{}-{}-{}", version, trace_id, parent_id, trace_flags)}};
SpanContextExtractor span_context_extractor(request_headers);

absl::StatusOr<SpanContext> span_context = span_context_extractor.extractSpanContext();
Expand All @@ -97,7 +97,7 @@ TEST(SpanContextExtractorTest, ThrowsExceptionWithInvalidSizes) {
const std::string invalid_version{"0"};
const std::string invalid_trace_flags{"001"};
Tracing::TestTraceContextImpl request_headers{
{"traceparent",
{"x-sendbird-traceparent",
fmt::format("{}-{}-{}-{}", invalid_version, trace_id, parent_id, invalid_trace_flags)}};
SpanContextExtractor span_context_extractor(request_headers);

Expand All @@ -110,7 +110,7 @@ TEST(SpanContextExtractorTest, ThrowsExceptionWithInvalidSizes) {
TEST(SpanContextExtractorTest, ThrowsExceptionWithInvalidHex) {
const std::string invalid_version{"ZZ"};
Tracing::TestTraceContextImpl request_headers{
{"traceparent",
{"x-sendbird-traceparent",
fmt::format("{}-{}-{}-{}", invalid_version, trace_id, parent_id, trace_flags)}};
SpanContextExtractor span_context_extractor(request_headers);

Expand All @@ -123,7 +123,7 @@ TEST(SpanContextExtractorTest, ThrowsExceptionWithInvalidHex) {
TEST(SpanContextExtractorTest, ThrowsExceptionWithAllZeroTraceId) {
const std::string invalid_trace_id{"00000000000000000000000000000000"};
Tracing::TestTraceContextImpl request_headers{
{"traceparent",
{"x-sendbird-traceparent",
fmt::format("{}-{}-{}-{}", version, invalid_trace_id, parent_id, trace_flags)}};
SpanContextExtractor span_context_extractor(request_headers);

Expand All @@ -136,7 +136,7 @@ TEST(SpanContextExtractorTest, ThrowsExceptionWithAllZeroTraceId) {
TEST(SpanContextExtractorTest, ThrowsExceptionWithAllZeroParentId) {
const std::string invalid_parent_id{"0000000000000000"};
Tracing::TestTraceContextImpl request_headers{
{"traceparent",
{"x-sendbird-traceparent",
fmt::format("{}-{}-{}-{}", version, trace_id, invalid_parent_id, trace_flags)}};
SpanContextExtractor span_context_extractor(request_headers);

Expand All @@ -148,7 +148,7 @@ TEST(SpanContextExtractorTest, ThrowsExceptionWithAllZeroParentId) {

TEST(SpanContextExtractorTest, ExtractSpanContextWithEmptyTracestate) {
Tracing::TestTraceContextImpl request_headers{
{"traceparent", fmt::format("{}-{}-{}-{}", version, trace_id, parent_id, trace_flags)}};
{"x-sendbird-traceparent", fmt::format("{}-{}-{}-{}", version, trace_id, parent_id, trace_flags)}};
SpanContextExtractor span_context_extractor(request_headers);
absl::StatusOr<SpanContext> span_context = span_context_extractor.extractSpanContext();

Expand All @@ -158,8 +158,8 @@ TEST(SpanContextExtractorTest, ExtractSpanContextWithEmptyTracestate) {

TEST(SpanContextExtractorTest, ExtractSpanContextWithTracestate) {
Tracing::TestTraceContextImpl request_headers{
{"traceparent", fmt::format("{}-{}-{}-{}", version, trace_id, parent_id, trace_flags)},
{"tracestate", "sample-tracestate"}};
{"x-sendbird-traceparent", fmt::format("{}-{}-{}-{}", version, trace_id, parent_id, trace_flags)},
{"x-sendbird-tracestate", "sample-tracestate"}};
SpanContextExtractor span_context_extractor(request_headers);
absl::StatusOr<SpanContext> span_context = span_context_extractor.extractSpanContext();

Expand All @@ -168,7 +168,7 @@ TEST(SpanContextExtractorTest, ExtractSpanContextWithTracestate) {
}

TEST(SpanContextExtractorTest, IgnoreTracestateWithoutTraceparent) {
Tracing::TestTraceContextImpl request_headers{{"tracestate", "sample-tracestate"}};
Tracing::TestTraceContextImpl request_headers{{"x-sendbird-tracestate", "sample-tracestate"}};
SpanContextExtractor span_context_extractor(request_headers);
absl::StatusOr<SpanContext> span_context = span_context_extractor.extractSpanContext();

Expand All @@ -178,9 +178,9 @@ TEST(SpanContextExtractorTest, IgnoreTracestateWithoutTraceparent) {

TEST(SpanContextExtractorTest, ExtractSpanContextWithMultipleTracestateEntries) {
Http::TestRequestHeaderMapImpl request_headers{
{"traceparent", fmt::format("{}-{}-{}-{}", version, trace_id, parent_id, trace_flags)},
{"tracestate", "sample-tracestate"},
{"tracestate", "sample-tracestate-2"}};
{"x-sendbird-traceparent", fmt::format("{}-{}-{}-{}", version, trace_id, parent_id, trace_flags)},
{"x-sendbird-tracestate", "sample-tracestate"},
{"x-sendbird-tracestate", "sample-tracestate-2"}};
Tracing::HttpTraceContext trace_context(request_headers);
SpanContextExtractor span_context_extractor(trace_context);
absl::StatusOr<SpanContext> span_context = span_context_extractor.extractSpanContext();
Expand Down