Skip to content

Commit

Permalink
[Backport 1.26] tracing: fix Datadog span name (envoyproxy#29932)
Browse files Browse the repository at this point in the history
Backport c3646f9 to 1.26

Additional testing:
Also, I ran the sources/extensions/tracers/datadog/demo both with and without these changes. Verified that the produced span's "operation name" before these changes is not as desired. Verified that the produced span's "operation name" after these changes is as desired.

Desired: "Operation name" is "envoy.proxy", and "resource name" is the operation_name passed to startSpan.

Risk Level: low
Testing: See the modified unit test.
Docs Changes: n/a
Release Notes: updated
Signed-off-by: David Goffredo <[email protected]>
  • Loading branch information
dgoffredo authored and florianmutter committed Oct 13, 2023
1 parent c77027f commit 74c2936
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 4 deletions.
3 changes: 3 additions & 0 deletions changelogs/current.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@ minor_behavior_changes:

bug_fixes:
# *Changes expected to improve the state of the world and are unlikely to have negative effects*
- area: tracing
change: |
Fixed a bug in the Datadog tracer where Datadog's "operation name" field would contain what should be in the "resource name" field.
removed_config_or_runtime:
# *Normally occurs at the end of the* :ref:`deprecation period <deprecated>`
Expand Down
7 changes: 6 additions & 1 deletion source/extensions/tracers/datadog/tracer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,12 @@ Tracing::SpanPtr Tracer::startSpan(const Tracing::Config&, Tracing::TraceContext
// The OpenTracing implementation ignored the `Tracing::Config` argument,
// so we will as well.
datadog::tracing::SpanConfig span_config;
span_config.name = operation_name;
// The `operation_name` parameter to this function more closely matches
// Datadog's concept of "resource name." Datadog's "span name," or "operation
// name," instead describes the category of operation being performed, which
// here we hard-code.
span_config.name = "envoy.proxy";
span_config.resource = operation_name;
span_config.start = estimateTime(start_time);

datadog::tracing::Tracer& tracer = *thread_local_tracer.tracer;
Expand Down
10 changes: 7 additions & 3 deletions test/extensions/tracers/datadog/tracer_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -109,9 +109,13 @@ TEST_F(DatadogTracerTest, SpanProperties) {
ASSERT_TRUE(maybe_dd_span);
const datadog::tracing::Span& dd_span = *maybe_dd_span;

// Verify that the span has the expected service name, operation name, start
// time, and sampling decision.
EXPECT_EQ("do.thing", dd_span.name());
// Verify that the span has the expected service name, operation name,
// resource name, start time, and sampling decision.
// Note that the `operation_name` we specified above becomes the
// `resource_name()` of the resulting Datadog span, while the Datadog span's
// `name()` (operation name) is hard-coded to "envoy.proxy."
EXPECT_EQ("envoy.proxy", dd_span.name());
EXPECT_EQ("do.thing", dd_span.resource_name());
EXPECT_EQ("envoy", dd_span.service_name());
ASSERT_TRUE(dd_span.trace_segment().sampling_decision());
EXPECT_EQ(int(datadog::tracing::SamplingPriority::USER_DROP),
Expand Down

0 comments on commit 74c2936

Please sign in to comment.