feat: Implement baggage span tags feature #246

zacharycmontoya · 2025-09-01T03:01:53Z

Motivation

There are a set of W3C baggage items that the Datadog APM product, RUM product, etc. use to provide a comprehensive observability experience, which includes user.id, account.id, and session.id. To deliver on the APM experience, we must make sure that the values are copied over to our web server spans as span tags.

We also want to provide users the ability to customize this feature or disable if desired.

Changes

Adds new functionality to the Datadog NGINX module to automatically copy W3C baggage items as span tags to the nginx spans, following this RFC.

This is implemented using two new NGINX directives, copied directly from the doc/API.md markdown file since the reference documentation should stand on its own:

`datadog_baggage_tags_enabled`

syntax datadog_baggage_tags_enabled on|off
context: http, server, location

If on, enable the datadog_baggage_tags_keys feature in the current configuration context.
If off, disable the datadog_baggage_tags_keys feature in the current configuration context.

`datadog_baggage_tags_keys`

syntax datadog_baggage_tags_keys all | select <key> [<key> ...]
default select user.id account.id session.id
context: http, server, location

Set span tags for items in the current W3C Baggage header.

To set span tags for baggage items with a matching <key>, use datadog_baggage_tags_keys select <key> [<key> ...].

To set span tags for all baggage items, use datadog_baggage_tags_keys all.

… the entire value. This tells us if the configuration is UNSET (so we apply default values) or if the configuration is disabled with an empty string value

- Change the individual baggage span tags into a list (I think I want to change it back) - Correctly add a default value at the main conf level

…ual `datadog_baggage_span_tag` directives, which feels more NGINX-y

…abled test

zacharycmontoya · 2025-09-03T21:20:10Z

doc/API.md

 of the current request.  `<value>` is a string that may contain
 `$`-[variables][2] (including those provided by this module).

+### `datadog_baggage_span_tag`


Offline feedback: See if there is another name we can use for this directive that is more intuitive for users

I updated doc/API.md with better documentation in 50de5ec. If that looks ok, then I think we should keep the existing directive name so it's similar to other Datadog tracing libraries. WDYT @dmehala ?

src/datadog_conf.h

src/request_tracing.cpp

…ains all baggage items from the header and creates a corresponding span tag for each.

…e do not need to do a recursive lookup while handling each request.

…an_tags' directive which accepts one or more tags for the list of configured baggage keys to add as span tags, as opposed to multiple 'datadog_baggage_span_tag' directives that each add one baggage key to the list. The motivation for this approach is that it aligns with the DD_TRACE_BAGGAGE_TAG_KEYS environment variable adopted in other Datadog tracers that accepts one string of comma-separated keys (e.g. 'DD_TRACE_BAGGAGE_TAG_KEYS=user.id,account.id,session.id')

…tives: - 'datadog_baggage_span_tags' - 'baggage_span_tags_enabled'

codecov-commenter · 2025-09-03T23:06:12Z

Codecov Report

❌ Patch coverage is 76.81159% with 16 lines in your changes missing coverage. Please review.
✅ Project coverage is 68.69%. Comparing base (6b51d05) to head (b9eee16).
⚠️ Report is 9 commits behind head on master.

Files with missing lines	Patch %	Lines
src/tracing/directives.cpp	62.96%	8 Missing and 2 partials ⚠️
src/request_tracing.cpp	79.16%	0 Missing and 5 partials ⚠️
src/ngx_http_datadog_module.cpp	92.85%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #246      +/-   ##
==========================================
+ Coverage   68.61%   68.69%   +0.07%     
==========================================
  Files          57       57              
  Lines        7322     7391      +69     
  Branches     1035     1051      +16     
==========================================
+ Hits         5024     5077      +53     
- Misses       1796     1804       +8     
- Partials      502      510       +8

Files with missing lines	Coverage Δ
src/tracing_library.cpp	`81.20% <100.00%> (+0.51%)`	⬆️
src/ngx_http_datadog_module.cpp	`66.06% <92.85%> (+1.42%)`	⬆️
src/request_tracing.cpp	`76.92% <79.16%> (+0.27%)`	⬆️
src/tracing/directives.cpp	`57.75% <62.96%> (+0.87%)`	⬆️

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

dmehala

I’ve added a few comments, in particular, I believe the wildcard case needs some rework. Additionally, I noticed a couple of scenarios that are not yet covered by tests:

datadog_baggage_span_tags user.id; and datadog_baggage_span off; should not set span tags.
datadog_baggage_span_tags user.id * fancy.tag;, ensure that in this case all baggage is captured.

doc/API.md

example/tracing/services/nginx/nginx.conf

src/ngx_http_datadog_module.cpp

test/cases/baggage_span_tags/test_baggage_span_tags.py

dmehala · 2025-09-08T15:12:22Z

doc/API.md

+On the currently active span, if the current W3C baggage header has a baggage item
+whose key matches the specified `<key>`, automatically set a tag whose name is
+`baggage.<key>` and whose value is the value of the key-value pair.
+
+This overrides any `baggage_span_tags_enabled` directives at higher levels,
+and may be overriden by `baggage_span_tags_enabled` directives at lower levels.


The current wording makes the directive harder to understand. I recommend simplifying it to convey the purpose as clearly and directly as possible.

Suggested change

On the currently active span, if the current W3C baggage header has a baggage item

whose key matches the specified `<key>`, automatically set a tag whose name is

`baggage.<key>` and whose value is the value of the key-value pair.

This overrides any `baggage_span_tags_enabled` directives at higher levels,

and may be overriden by `baggage_span_tags_enabled` directives at lower levels.

Set span tags for all baggage items matching `<key>` from the current W3C Baggage header.

I also believe baggage_span_tags_enabled should always take precedence over this directive. WDYT?

Yes if the configuration is baggage_span_tags_enabled off that should disable this. I believe I had a copy/paste issue here, sorry about that!

In commit 341fdb7 I've reworded this highlighted portion to say the following, which is essentially what you suggested. WDYT?

Set span tags for all items from the current W3C Baggage header with a matching `<key>`.

doc/API.md

…ocumentation, and improved wildcard logic

…::variant<std::vector<std::string>, bool>'. This distinguishes between the first type containing baggage keys to set vs the second type indicating that the wildcard key '*' was specified so all baggage keys should be set.

…key>" and "datadog_baggage_span_tags_enabled off" are set, then no baggage span tags are set

zacharycmontoya · 2025-09-26T22:07:09Z

I’ve added a few comments, in particular, I believe the wildcard case needs some rework. Additionally, I noticed a couple of scenarios that are not yet covered by tests:

datadog_baggage_span_tags user.id; and datadog_baggage_span off; should not set span tags.

In b9eee16 I've added a test case to cover this scenario.

datadog_baggage_span_tags user.id * fancy.tag;, ensure that in this case all baggage is captured.

For this scenario, I'm getting clarification in the RFC but my reading is that this does not invoke the wildcard behavior. This appears to only be activated when the configuration is exactly datadog_baggage_span_tags *; (only one item with the value "*")

zacharycmontoya · 2025-09-29T16:42:44Z

datadog_baggage_span_tags user.id * fancy.tag;, ensure that in this case all baggage is captured.

For this scenario, I'm getting clarification in the RFC but my reading is that this does not invoke the wildcard behavior. This appears to only be activated when the configuration is exactly datadog_baggage_span_tags *; (only one item with the value "*")

I confirmed with the RFC author that only the configuration datadog_baggage_span_tags *; should be a wildcard. If there are multiple items such as datadog_baggage_span_tags tag1 tag2 *; then there's no wildcard behavior and the * is treated as a named header, since that character is allowed in HTTP headers. So my current implementation is up-to-date with that specification

dmehala · 2025-10-01T13:43:03Z

then there's no wildcard behavior and the * is treated as a named header, since that character is allowed in HTTP headers

That means, for the same directive,* has two meanings:

everything when its by itself.
the * character when there's multiple items.

That's error-prone, WDYT?

…gage_tags"

…gage_tags_keys"

…nt is 'all' or 'select'. 'all' invokes the wildcard behavior while 'select' accepts a list of keys

zacharycmontoya · 2025-10-03T17:22:13Z

then there's no wildcard behavior and the * is treated as a named header, since that character is allowed in HTTP headers

That means, for the same directive,* has two meanings:

everything when its by itself.

the * character when there's multiple items.

That's error-prone, WDYT?

As discussed offline, to eliminate the * character having two meanings, the keys directive must now be specified as datadog_baggage_tags_keys all to invoke the wildcard behavior or datadog_baggage_tags_keys select <key> [<key> ...] to select keys. So now * no longer has any special meanings.

zacharycmontoya added 6 commits August 29, 2025 17:40

TODO: Change the configuration from individual entries to one csv for…

de30bb6

… the entire value. This tells us if the configuration is UNSET (so we apply default values) or if the configuration is disabled with an empty string value

Two changes in one:

084e7f4

- Change the individual baggage span tags into a list (I think I want to change it back) - Correctly add a default value at the main conf level

Convert the combined datadog_baggage_span_tags directive to individ…

9c358a5

…ual `datadog_baggage_span_tag` directives, which feels more NGINX-y

Refactor and format test_baggage_span_tags.py. Also had a general dis…

3482602

…abled test

Small update to example conf

290cf4a

Small code comment update

4936a7e

zacharycmontoya commented Sep 3, 2025

View reviewed changes

src/datadog_conf.h Outdated Show resolved Hide resolved

zacharycmontoya commented Sep 3, 2025

View reviewed changes

src/request_tracing.cpp Outdated Show resolved Hide resolved

zacharycmontoya added 7 commits September 3, 2025 14:50

Update baggage_span_tags field from an unordered_set to a vector

e07bfa5

Fix bug so that specifying one span tag with the wildcard key "*" obt…

577d0c3

…ains all baggage items from the header and creates a corresponding span tag for each.

Move calculation of baggage span tags to configuration-time so that w…

82e66e8

…e do not need to do a recursive lookup while handling each request.

Update the API docs with a more complete description of the two direc…

50de5ec

…tives: - 'datadog_baggage_span_tags' - 'baggage_span_tags_enabled'

Satisfy linter

ae00942

Satisfy clang-format-14 formatting

d02fdaa

zacharycmontoya marked this pull request as ready for review September 3, 2025 23:04

zacharycmontoya requested a review from a team as a code owner September 3, 2025 23:04

zacharycmontoya requested review from Anilm3 and removed request for a team September 3, 2025 23:04

dmehala reviewed Sep 8, 2025

View reviewed changes

zacharycmontoya added 5 commits September 9, 2025 17:16

PR Feedback: Update tests to use tuples instead of lists

172732c

PR Feedback: Implement changes except for directive name, directive d…

7bec512

…ocumentation, and improved wildcard logic

Update docs for the baggage span_tags feature

341fdb7

Update test cases to ensure that if both "datadog_baggage_span_tags <…

b9eee16

…key>" and "datadog_baggage_span_tags_enabled off" are set, then no baggage span tags are set

zacharycmontoya added 4 commits October 2, 2025 19:53

refactor: Large rename of "datadog_baggage_span_tags" to "datadog_bag…

6a589e6

…gage_tags"

refactor: Rename the "datadog_baggage_tags" directive to "datadog_bag…

4ae1f47

…gage_tags_keys"

Update the 'datadog_baggage_tags_keys' directive so it's first argume…

2672a55

…nt is 'all' or 'select'. 'all' invokes the wildcard behavior while 'select' accepts a list of keys

Run formatter

c72e391

feat: Implement baggage span tags feature #246

Are you sure you want to change the base?

feat: Implement baggage span tags feature #246

Uh oh!

Conversation

zacharycmontoya commented Sep 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation

Changes

datadog_baggage_tags_enabled

datadog_baggage_tags_keys

Uh oh!

zacharycmontoya Sep 3, 2025

Choose a reason for hiding this comment

Uh oh!

zacharycmontoya Sep 3, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

codecov-commenter commented Sep 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

dmehala left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dmehala Sep 8, 2025

Choose a reason for hiding this comment

Uh oh!

zacharycmontoya Sep 26, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

zacharycmontoya commented Sep 26, 2025

Uh oh!

zacharycmontoya commented Sep 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dmehala commented Oct 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

zacharycmontoya commented Oct 3, 2025

Uh oh!

Uh oh!

zacharycmontoya commented Sep 1, 2025 •

edited

Loading

`datadog_baggage_tags_enabled`

`datadog_baggage_tags_keys`

codecov-commenter commented Sep 3, 2025 •

edited

Loading

zacharycmontoya commented Sep 29, 2025 •

edited

Loading

dmehala commented Oct 1, 2025 •

edited

Loading