Skip to content

Datadog: restore "resource.name" tag#30503

Merged
mattklein123 merged 2 commits into
envoyproxy:mainfrom
DataDog:david.goffredo/restore-resource-name-tag
Oct 25, 2023
Merged

Datadog: restore "resource.name" tag#30503
mattklein123 merged 2 commits into
envoyproxy:mainfrom
DataDog:david.goffredo/restore-resource-name-tag

Conversation

@dgoffredo
Copy link
Copy Markdown
Contributor

@dgoffredo dgoffredo commented Oct 25, 2023

These changes restore the Dataodog tracer's ability to set the Datadog notion of span "resource name" by setting a special tag named "resource.name".

This was previously possible with the OpenTracing-based implementation of Datadog tracing, but I removed the capability when I moved Envoy's Datadog extension away from OpenTracing.

It turns out that customers were using this feature. This pull request restores it.

Please consider backporting this revision as a bug fix once it is merged.

Testing

I modified some existing unit tests.

Also, by applying the patch below, I used the Datadog "demo" with a local build of Envoy to verify that a request_header-based custom tag for the tag "resource.name" really does result in a change to the resource name:

diff --git a/source/extensions/tracers/datadog/demo/envoy.yaml b/source/extensions/tracers/datadog/demo/envoy.yaml
index 839ba18aac..305c76059f 100644
--- a/source/extensions/tracers/datadog/demo/envoy.yaml
+++ b/source/extensions/tracers/datadog/demo/envoy.yaml
@@ -18,6 +18,13 @@ static_resources:
                 "@type": type.googleapis.com/envoy.config.trace.v3.DatadogConfig
                 collector_cluster: datadog_agent
                 service_name: envoy-demo
+            custom_tags:
+            - tag: "header.thing"
+              request_header:
+                name: "x-thing"
+            - tag: "resource.name"
+              request_header:
+                name: "x-datadog-resource"
           access_log:
           - name: envoy.access_loggers.file
             typed_config:

Commit Message: restore ability to set resource name by tag in Datadog tracer
Additional Description:
Risk Level: low
Testing: See modified unit tests. Also performed manual integration testing with the Datadog "demo."
Docs Changes: N/A
Release Notes: N/A
Platform Specific Features: N/A
[Optional Runtime guard:]
[Optional Fixes #Issue]
[Optional Fixes commit #PR or SHA]
[Optional Deprecated:]
[Optional API Considerations:]

Signed-off-by: David Goffredo <david.goffredo@datadoghq.com>
- support rootless docker for the Datadog Agent in docker-compose.yaml
- find envoy-static in bazel-bin/ instead of using
  `bazelisk info bazel-genfiles`

Signed-off-by: David Goffredo <david.goffredo@datadoghq.com>
@dgoffredo dgoffredo force-pushed the david.goffredo/restore-resource-name-tag branch from a9f4168 to 211117c Compare October 25, 2023 15:02
@mattklein123 mattklein123 merged commit 1203508 into envoyproxy:main Oct 25, 2023
@dgoffredo
Copy link
Copy Markdown
Contributor Author

Thanks @mattklein123!

Please consider backporting this revision as a bug fix once it is merged.

Can we get this tagged for review?

@hiddewie
Copy link
Copy Markdown
Contributor

hiddewie commented Nov 2, 2023

Hi @mattklein123 and @dgoffredo. Thanks for picking this PR up. We observed that it was not part of the recent 1.28 release. Is there a plan to backport this fix into previous Envoy versions?

@dgoffredo
Copy link
Copy Markdown
Contributor Author

I would like for this PR to be reviewed for backporting as a bug fix onto v1.27 and v1.28. @mattklein123, how do I start that process?

Smeb pushed a commit to Smeb/envoy that referenced this pull request Nov 15, 2023
Signed-off-by: David Goffredo <david.goffredo@datadoghq.com>
Smeb pushed a commit to Smeb/envoy that referenced this pull request Nov 15, 2023
Signed-off-by: David Goffredo <david.goffredo@datadoghq.com>
htuch pushed a commit that referenced this pull request Nov 16, 2023
This is the backport of #30503

Risk Level: low
Testing: local unit tests
Docs Changes: n/a
Release Notes: n/a
Platform Specific Features: n/a
Fixes: #30235

Signed-off-by: David Goffredo <david.goffredo@datadoghq.com>
htuch pushed a commit that referenced this pull request Nov 16, 2023
This is the backport of #30503

Risk Level: low
Testing: local unit tests
Docs Changes: n/a
Release Notes: n/a
Platform Specific Features: n/a
Fixes: #30235

Signed-off-by: David Goffredo <david.goffredo@datadoghq.com>
Signed-off-by: Ben Ryves <smeb@gmail.com>
Co-authored-by: David Goffredo <david.goffredo@datadoghq.com>
Co-authored-by: Ben Ryves <smeb@gmail.com>
SeanKilleen pushed a commit to SeanKilleen/envoy that referenced this pull request Apr 3, 2024
…voyproxy#30893)

This is the backport of envoyproxy#30503

Risk Level: low
Testing: local unit tests
Docs Changes: n/a
Release Notes: n/a
Platform Specific Features: n/a
Fixes: envoyproxy#30235

Signed-off-by: David Goffredo <david.goffredo@datadoghq.com>
Signed-off-by: Ben Ryves <smeb@gmail.com>
Co-authored-by: David Goffredo <david.goffredo@datadoghq.com>
Co-authored-by: Ben Ryves <smeb@gmail.com>
Signed-off-by: Sean Killeen <SeanKilleen@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants