From 74c29368d667557d1cee8d1c2ec550456228f47f Mon Sep 17 00:00:00 2001 From: David Goffredo Date: Thu, 12 Oct 2023 13:28:55 -0400 Subject: [PATCH] [Backport 1.26] tracing: fix Datadog span name (#29932) Backport c3646f994e0296ed44ecac2869fa65e7f58bf986 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 --- changelogs/current.yaml | 3 +++ source/extensions/tracers/datadog/tracer.cc | 7 ++++++- test/extensions/tracers/datadog/tracer_test.cc | 10 +++++++--- 3 files changed, 16 insertions(+), 4 deletions(-) diff --git a/changelogs/current.yaml b/changelogs/current.yaml index 9ecf0d6e48ce..38c1f0f5611a 100644 --- a/changelogs/current.yaml +++ b/changelogs/current.yaml @@ -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 ` diff --git a/source/extensions/tracers/datadog/tracer.cc b/source/extensions/tracers/datadog/tracer.cc index d857df2e06d4..d9b48be8998c 100644 --- a/source/extensions/tracers/datadog/tracer.cc +++ b/source/extensions/tracers/datadog/tracer.cc @@ -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; diff --git a/test/extensions/tracers/datadog/tracer_test.cc b/test/extensions/tracers/datadog/tracer_test.cc index a87ad438b9da..d964946e7252 100644 --- a/test/extensions/tracers/datadog/tracer_test.cc +++ b/test/extensions/tracers/datadog/tracer_test.cc @@ -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),