diff --git a/doc/API.md b/doc/API.md index 2a64b692..7eb6874a 100644 --- a/doc/API.md +++ b/doc/API.md @@ -179,6 +179,24 @@ whose value is the result of evaluating the specified `` in the context of the current request. `` is a string that may contain `$`-[variables][2] (including those provided by this module). +### `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 [ ...]` +- **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 ``, use `datadog_baggage_tags_keys select [ ...]`. + +To set span tags for all baggage items, use `datadog_baggage_tags_keys all`. + ### `datadog_tracing` - **syntax** `datadog_tracing on|off` - **default** `on` diff --git a/example/tracing/services/nginx/nginx.conf b/example/tracing/services/nginx/nginx.conf index 16d2fb2e..6acf630c 100644 --- a/example/tracing/services/nginx/nginx.conf +++ b/example/tracing/services/nginx/nginx.conf @@ -24,6 +24,9 @@ http { location /http { # Add a custom tag. Tag values can contain nginx variables. datadog_tag special.tag "The URI is $uri"; + # Add baggage span tags. If a baggage item has a matching key, the key-value is set a span tag. + datadog_baggage_span_tag user.id; + proxy_pass http://http:8080; } @@ -33,6 +36,10 @@ http { datadog_trace_locations on; # This tag will be on the location-specific span. datadog_tag special.tag "The URI is $uri"; + # Do not configure additional baggage span tags. When unspecified, this is the same as specifiying the following: + # datadog_baggage_span_tag user.id session.id account.id; + # To disable baggage span tags on this location, set 'datadog_baggage_tags_enabled off' + # The resource name is customizable for both the request span and # the location span. datadog_resource_name "request URI $uri"; diff --git a/src/datadog_conf.h b/src/datadog_conf.h index 0ee39f43..68e6e869 100644 --- a/src/datadog_conf.h +++ b/src/datadog_conf.h @@ -18,6 +18,7 @@ extern "C" { #include #include +#include #include #define DD_NGX_CONF_COMPLEX_UNSET \ @@ -215,6 +216,8 @@ struct datadog_loc_conf_t { // `service_version` is set by the `datadog_version` directive. ngx_http_complex_value_t *service_version = DD_NGX_CONF_COMPLEX_UNSET; std::unordered_map tags; + ngx_flag_t baggage_span_tags_enabled = NGX_CONF_UNSET; + std::variant, bool> baggage_span_tags; // `parent` is the parent context (e.g. the `server` to this `location`), or // `nullptr` if this context has no parent. datadog_loc_conf_t *parent; diff --git a/src/ngx_http_datadog_module.cpp b/src/ngx_http_datadog_module.cpp index 110277e0..730470ad 100644 --- a/src/ngx_http_datadog_module.cpp +++ b/src/ngx_http_datadog_module.cpp @@ -466,6 +466,9 @@ static char *merge_datadog_loc_conf(ngx_conf_t *cf, void *parent, TracingLibrary::tracing_on_by_default()); ngx_conf_merge_value(conf->enable_locations, prev->enable_locations, TracingLibrary::trace_locations_by_default()); + ngx_conf_merge_value(conf->baggage_span_tags_enabled, + prev->baggage_span_tags_enabled, + TracingLibrary::bagage_span_tags_by_default()); ngx_conf_merge_ptr_value(conf->service_name, prev->service_name, nullptr); ngx_conf_merge_ptr_value(conf->service_env, prev->service_env, nullptr); ngx_conf_merge_ptr_value(conf->service_version, prev->service_version, @@ -497,6 +500,25 @@ static char *merge_datadog_loc_conf(ngx_conf_t *cf, void *parent, conf->tags.merge(parent_tags); } + // Merge baggage span tags, but only if this conf has no specified baggage + // span tags. + + // Update only if the child configuration has no specified baggage span tags. + if (std::holds_alternative>( + conf->baggage_span_tags) && + std::get>(conf->baggage_span_tags).empty()) { + // If the parent configuration has no specified baggage span tags, use + // default values. Otherwise, use the parent configuration's baggage span + // tags. + if (std::holds_alternative>( + prev->baggage_span_tags) && + std::get>(prev->baggage_span_tags).empty()) { + conf->baggage_span_tags = TracingLibrary::default_baggage_span_tags(); + } else { + conf->baggage_span_tags = prev->baggage_span_tags; + } + } + #ifdef WITH_WAF if (conf->waf_pool == nullptr) { conf->waf_pool = prev->waf_pool; diff --git a/src/request_tracing.cpp b/src/request_tracing.cpp index 428acecf..13a41afd 100644 --- a/src/request_tracing.cpp +++ b/src/request_tracing.cpp @@ -74,6 +74,33 @@ static void add_status_tags(const ngx_http_request_t *request, dd::Span &span) { } } +// Iterate through the configured baggage_span_tags and create span tags for the +// configured baggage keys. The one special case is if baggage_span_tags=["*"]. +// In this case, create a corresponding span tag for all baggage items. +// +// Precondition: The local conf has the directive +// `datadog_baggage_tags_enabled` set. +void add_baggage_span_tags(datadog_loc_conf_t *conf, tracing::Baggage &baggage, + dd::Span &span) { + if (baggage.empty()) return; + + if (std::holds_alternative>( + conf->baggage_span_tags)) { + for (const auto &tag_name : + std::get>(conf->baggage_span_tags)) { + if (baggage.contains(tag_name)) { + span.set_tag(std::string("baggage.") + tag_name, + baggage.get(tag_name).value()); + } + } + } else if (std::holds_alternative(conf->baggage_span_tags) && + std::get(conf->baggage_span_tags)) { + baggage.visit([&span](std::string_view key, std::string_view value) { + span.set_tag(std::string("baggage.") + std::string(key), value); + }); + } +} + static void add_upstream_name(const ngx_http_request_t *request, dd::Span &span) { if (!request->upstream || !request->upstream->upstream || @@ -212,6 +239,13 @@ RequestTracing::RequestTracing(ngx_http_request_t *request, } else { request_span_.emplace(std::move(*maybe_span)); } + + if (loc_conf_->baggage_span_tags_enabled) { + auto maybe_baggage = tracer->extract_baggage(reader); + if (maybe_baggage && request_span_) { + add_baggage_span_tags(loc_conf, *maybe_baggage, *request_span_); + } + } } if (!request_span_) { diff --git a/src/tracing/directives.cpp b/src/tracing/directives.cpp index 6c4e4f5c..96934e27 100644 --- a/src/tracing/directives.cpp +++ b/src/tracing/directives.cpp @@ -63,6 +63,71 @@ char *set_datadog_tag(ngx_conf_t *cf, ngx_command_t *command, return NGX_CONF_OK; } +char *set_datadog_baggage_tags(ngx_conf_t *cf, ngx_command_t *command, + void *conf) noexcept { + auto loc_conf = static_cast(conf); + const auto values = static_cast(cf->args->elts); + assert(cf->args->nelts >= 1); + + const auto args = values + 1; + const auto nargs = cf->args->nelts - 1; + + if (str(args[0]) == "all") { + if (nargs == 1) { + loc_conf->baggage_span_tags = true; + return NGX_CONF_OK; + } else { + ngx_conf_log_error(NGX_LOG_ERR, cf, 0, + "Invalid arguments to %V directive. \"all\" may not" + "be followed with any other arguments.", + &command->name); + return static_cast(NGX_CONF_ERROR); + } + } else if (str(args[0]) == "select") { + if (nargs == 1) { + ngx_conf_log_error(NGX_LOG_ERR, cf, 0, + "Invalid arguments to %V directive. \"select\" must" + "be followed with at least one argument.", + &command->name); + return static_cast(NGX_CONF_ERROR); + } + } else { + ngx_conf_log_error(NGX_LOG_ERR, cf, 0, + "Invalid arguments to %V directive. The first argument " + "must be \"all\" " + "or \"select\".", + &command->name); + return static_cast(NGX_CONF_ERROR); + } + + // If a previous directive usage configured this to use the `all` wildcard, + // reset the variant to a vector to hold the new directive's arguments. + if (std::holds_alternative(loc_conf->baggage_span_tags)) { + ngx_conf_log_error(NGX_LOG_ERR, cf, 0, + "Invalid argument \"select\" to %V directive. This " + "directive was already configured with " + "the setting \"all\".", + &command->name); + return static_cast(NGX_CONF_ERROR); + } + + for (const ngx_str_t *arg = args + 1; arg != args + nargs; ++arg) { + const auto baggage_key = to_string_view(*arg); + if (baggage_key.empty()) { + ngx_conf_log_error(NGX_LOG_ERR, cf, 0, + "Invalid argument \"%V\" to %V directive. Expected a " + "non-empty string.", + arg, &command->name); + return static_cast(NGX_CONF_ERROR); + } + + std::get>(loc_conf->baggage_span_tags) + .emplace_back(std::string(baggage_key)); + } + + return NGX_CONF_OK; +} + char *set_datadog_sample_rate(ngx_conf_t *cf, ngx_command_t *command, void *conf) noexcept { const auto loc_conf = static_cast(conf); diff --git a/src/tracing/directives.h b/src/tracing/directives.h index 83dd5549..ba07f302 100644 --- a/src/tracing/directives.h +++ b/src/tracing/directives.h @@ -36,6 +36,9 @@ constexpr ngx_uint_t anywhere = char *set_datadog_tag(ngx_conf_t *cf, ngx_command_t *command, void *conf) noexcept; +char *set_datadog_baggage_tags(ngx_conf_t *cf, ngx_command_t *command, + void *conf) noexcept; + char *set_datadog_sample_rate(ngx_conf_t *cf, ngx_command_t *command, void *conf) noexcept; @@ -144,6 +147,24 @@ constexpr datadog::nginx::directive tracing_directives[] = { nullptr, }, + { + "datadog_baggage_tags_enabled", + anywhere | NGX_CONF_TAKE1, + ngx_conf_set_flag_slot, + NGX_HTTP_LOC_CONF_OFFSET, + offsetof(datadog_loc_conf_t, baggage_span_tags_enabled), + nullptr, + }, + + { + "datadog_baggage_tags_keys", + anywhere | NGX_CONF_1MORE, + set_datadog_baggage_tags, + NGX_HTTP_LOC_CONF_OFFSET, + 0, + nullptr, + }, + // aliases opentracing (legacy) ALIAS_COMMAND("datadog_tracing", "opentracing", anywhere | NGX_CONF_TAKE1), ALIAS_COMMAND("datadog_operation_name", "opentracing_operation_name", diff --git a/src/tracing_library.cpp b/src/tracing_library.cpp index b1a6cc79..71b0a866 100644 --- a/src/tracing_library.cpp +++ b/src/tracing_library.cpp @@ -232,6 +232,10 @@ TracingLibrary::default_tags() { return tags; } +std::vector TracingLibrary::default_baggage_span_tags() { + return {"user.id", "session.id", "account.id"}; +} + std::string_view TracingLibrary::default_resource_name_pattern() { return "$request_method $uri"; } @@ -240,5 +244,7 @@ bool TracingLibrary::tracing_on_by_default() { return true; } bool TracingLibrary::trace_locations_by_default() { return false; } +bool TracingLibrary::bagage_span_tags_by_default() { return true; } + } // namespace nginx } // namespace datadog diff --git a/src/tracing_library.h b/src/tracing_library.h index d0c22f40..5982d69d 100644 --- a/src/tracing_library.h +++ b/src/tracing_library.h @@ -113,6 +113,20 @@ struct TracingLibrary { // that they will refer to string literals). static std::unordered_map default_tags(); + // Return the default baggage span tags. These tags will be defined + // automatically during configuration as if they appeared in the nginx + // configuration file's http section, e.g. + // + // http { + // datadog_baggage_tags_keys user.id session.id account.id; + // ... + // } + // + // Note that the storage to which each returned `std::string_view` refers + // must outlive any usage of the return value (realistically this means + // that they will refer to string literals). + static std::vector default_baggage_span_tags(); + // Return the default setting for whether tracing is enabled in nginx. static bool tracing_on_by_default(); @@ -120,6 +134,10 @@ struct TracingLibrary { // An HTTP location is an endpoint as configured using a "location" block // in the nginx configuration. static bool trace_locations_by_default(); + + // Return the default setting for whether baggage span tags will be added + // to the current span. + static bool bagage_span_tags_by_default(); }; } // namespace nginx diff --git a/test/cases/baggage_span_tags/README.md b/test/cases/baggage_span_tags/README.md new file mode 100644 index 00000000..875e7334 --- /dev/null +++ b/test/cases/baggage_span_tags/README.md @@ -0,0 +1,6 @@ +This test verifies that the expected baggage span tags are added to spans produced by +the nginx module. + +Some tags are defined by default (see `TracingLibrary::default_baggage_span_tags` in the +module source), while others can be defined by the user via the `datadog_baggage_span_tag` +configuration directive. diff --git a/test/cases/baggage_span_tags/__init__.py b/test/cases/baggage_span_tags/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/test/cases/baggage_span_tags/conf/builtins.conf b/test/cases/baggage_span_tags/conf/builtins.conf new file mode 100644 index 00000000..992a7b85 --- /dev/null +++ b/test/cases/baggage_span_tags/conf/builtins.conf @@ -0,0 +1,21 @@ +# "/datadog-tests" is a directory created by the docker build +# of the nginx test image. It contains the module, the +# nginx config, and "index.html". +load_module /datadog-tests/ngx_http_datadog_module.so; + +events { + worker_connections 1024; +} + +http { + datadog_agent_url http://agent:8126; + + server { + listen 80; + server_name localhost; + + location /http { + proxy_pass http://http:8080; + } + } +} diff --git a/test/cases/baggage_span_tags/conf/custom_in_http.conf b/test/cases/baggage_span_tags/conf/custom_in_http.conf new file mode 100644 index 00000000..68c49cd2 --- /dev/null +++ b/test/cases/baggage_span_tags/conf/custom_in_http.conf @@ -0,0 +1,22 @@ +# "/datadog-tests" is a directory created by the docker build +# of the nginx test image. It contains the module, the +# nginx config, and "index.html". +load_module /datadog-tests/ngx_http_datadog_module.so; + +events { + worker_connections 1024; +} + +http { + datadog_agent_url http://agent:8126; + datadog_baggage_tags_keys select snazzy.tag fancy.tag; + + server { + listen 80; + server_name localhost; + + location /http { + proxy_pass http://http:8080; + } + } +} diff --git a/test/cases/baggage_span_tags/conf/custom_in_location.conf b/test/cases/baggage_span_tags/conf/custom_in_location.conf new file mode 100644 index 00000000..f6a6f001 --- /dev/null +++ b/test/cases/baggage_span_tags/conf/custom_in_location.conf @@ -0,0 +1,22 @@ +# "/datadog-tests" is a directory created by the docker build +# of the nginx test image. It contains the module, the +# nginx config, and "index.html". +load_module /datadog-tests/ngx_http_datadog_module.so; + +events { + worker_connections 1024; +} + +http { + datadog_agent_url http://agent:8126; + + server { + listen 80; + server_name localhost; + + location /http { + datadog_baggage_tags_keys select snazzy.tag fancy.tag; + proxy_pass http://http:8080; + } + } +} diff --git a/test/cases/baggage_span_tags/conf/custom_in_server.conf b/test/cases/baggage_span_tags/conf/custom_in_server.conf new file mode 100644 index 00000000..6dd01a97 --- /dev/null +++ b/test/cases/baggage_span_tags/conf/custom_in_server.conf @@ -0,0 +1,23 @@ +# "/datadog-tests" is a directory created by the docker build +# of the nginx test image. It contains the module, the +# nginx config, and "index.html". +load_module /datadog-tests/ngx_http_datadog_module.so; + +events { + worker_connections 1024; +} + +http { + datadog_agent_url http://agent:8126; + + server { + listen 80; + server_name localhost; + + datadog_baggage_tags_keys select snazzy.tag fancy.tag; + + location /http { + proxy_pass http://http:8080; + } + } +} diff --git a/test/cases/baggage_span_tags/conf/disabled.conf b/test/cases/baggage_span_tags/conf/disabled.conf new file mode 100644 index 00000000..485f3725 --- /dev/null +++ b/test/cases/baggage_span_tags/conf/disabled.conf @@ -0,0 +1,22 @@ +# "/datadog-tests" is a directory created by the docker build +# of the nginx test image. It contains the module, the +# nginx config, and "index.html". +load_module /datadog-tests/ngx_http_datadog_module.so; + +events { + worker_connections 1024; +} + +http { + datadog_agent_url http://agent:8126; + datadog_baggage_tags_enabled off; + + server { + listen 80; + server_name localhost; + + location /http { + proxy_pass http://http:8080; + } + } +} diff --git a/test/cases/baggage_span_tags/conf/overwrite_defaults.conf b/test/cases/baggage_span_tags/conf/overwrite_defaults.conf new file mode 100644 index 00000000..32dc9e6b --- /dev/null +++ b/test/cases/baggage_span_tags/conf/overwrite_defaults.conf @@ -0,0 +1,38 @@ +# "/datadog-tests" is a directory created by the docker build +# of the nginx test image. It contains the module, the +# nginx config, and "index.html". +load_module /datadog-tests/ngx_http_datadog_module.so; + +events { + worker_connections 1024; +} + +http { + datadog_agent_url http://agent:8126; + datadog_baggage_tags_keys all; + + server { + listen 80; + server_name localhost; + + location /http { + proxy_pass http://http:8080; + } + + location /disabled_tags { + datadog_baggage_tags_keys select snazzy.tag fancy.tag; + datadog_baggage_tags_enabled off; + proxy_pass http://http:8080; + } + + location /all_baggage_span_tags { + datadog_baggage_tags_keys all; + proxy_pass http://http:8080; + } + + location /snazzy_tag { + datadog_baggage_tags_keys select snazzy.tag; + proxy_pass http://http:8080; + } + } +} diff --git a/test/cases/baggage_span_tags/test_baggage_span_tags.py b/test/cases/baggage_span_tags/test_baggage_span_tags.py new file mode 100644 index 00000000..c0292432 --- /dev/null +++ b/test/cases/baggage_span_tags/test_baggage_span_tags.py @@ -0,0 +1,141 @@ +from .. import case +from .. import formats + +from pathlib import Path +import pprint + + +class TestBaggageSpanTags(case.TestCase): + + def run_custom_tags_test(self, conf_relative_path, http_path, + configured_baggage_span_tags): + """Verify that spans produced by an nginx configured using the + specified nginx `conf_text` (from a file having the specified + `file_name`) contain expected values for the baggage span tags. + """ + conf_path = Path(__file__).parent / conf_relative_path + conf_text = conf_path.read_text() + # To test this, we make any old request to nginx, and then in order + # to ensure that nginx flushes its trace to the agent, reload nginx. + # Then we send a "sync" request to the agent in order to establish a + # log line that's strictly after the trace was flushed, and finally we + # examine the interim log lines from the agent to find the tags sent to + # it by nginx's tracer. + self.orch.nginx_replace_config(conf_text, conf_path.name) + + # Consume any previous logging from the agent. + self.orch.sync_service("agent") + + headers = { + "baggage": + "user.id=doggo,session.id=123,account.id=456,snazzy.tag=hard-coded,fancy.tag=GET" + } + + status, _, _ = self.orch.send_nginx_http_request(http_path, + headers=headers) + self.assertEqual(status, 200, conf_relative_path) + + self.orch.reload_nginx() + log_lines = self.orch.sync_service("agent") + + for line in log_lines: + segments = formats.parse_trace(line) + if segments is None: + # some other kind of logging; ignore + continue + for segment in segments: + for span in segment: + if span["service"] != "nginx": + continue + # We found an nginx span. Make sure that it has the + # "baggage.snazzy.tag" and "baggage.fancy.tag" tags, with the expected values. + # The two tags are assumed to be configured in `conf_text`. + tags = span["meta"] + + if (configured_baggage_span_tags + and "user.id" in configured_baggage_span_tags): + self.assertIn("baggage.user.id", tags, + conf_relative_path) + self.assertEqual(tags["baggage.user.id"], "doggo", + conf_relative_path) + else: + self.assertNotIn("baggage.user.id", tags, + conf_relative_path) + + if (configured_baggage_span_tags + and "session.id" in configured_baggage_span_tags): + self.assertIn("baggage.session.id", tags, + conf_relative_path) + self.assertEqual(tags["baggage.session.id"], "123", + conf_relative_path) + else: + self.assertNotIn("baggage.session.id", tags, + conf_relative_path) + + if (configured_baggage_span_tags + and "account.id" in configured_baggage_span_tags): + self.assertIn("baggage.account.id", tags, + conf_relative_path) + self.assertEqual(tags["baggage.account.id"], "456", + conf_relative_path) + else: + self.assertNotIn("baggage.account.id", tags, + conf_relative_path) + + if (configured_baggage_span_tags + and "snazzy.tag" in configured_baggage_span_tags): + self.assertIn("baggage.snazzy.tag", tags, + conf_relative_path) + self.assertEqual(tags["baggage.snazzy.tag"], + "hard-coded", conf_relative_path) + else: + self.assertNotIn("baggage.snazzy.tag", tags, + conf_relative_path) + + if (configured_baggage_span_tags + and "fancy.tag" in configured_baggage_span_tags): + self.assertIn("baggage.fancy.tag", tags, + conf_relative_path) + self.assertEqual(tags["baggage.fancy.tag"], "GET", + conf_relative_path) + else: + self.assertNotIn("baggage.fancy.tag", tags, + conf_relative_path) + + def test_custom_in_location(self): + return self.run_custom_tags_test("./conf/custom_in_location.conf", + "/http", ("snazzy.tag", "fancy.tag")) + + def test_custom_in_server(self): + return self.run_custom_tags_test("./conf/custom_in_server.conf", + "/http", ("snazzy.tag", "fancy.tag")) + + def test_custom_in_http(self): + return self.run_custom_tags_test("./conf/custom_in_http.conf", "/http", + ("snazzy.tag", "fancy.tag")) + + def test_default_tags(self): + return self.run_custom_tags_test( + "./conf/builtins.conf", "/http", + ("user.id", "session.id", "account.id")) + + def test_main_disabled(self): + return self.run_custom_tags_test("./conf/disabled.conf", "/http", None) + + def test_overwite_default_tags_empty_inherits_previous_conf(self): + return self.run_custom_tags_test( + "./conf/builtins.conf", "/http", + ("user.id", "session.id", "account.id")) + + def test_overwite_default_tags_disabled_at_location(self): + return self.run_custom_tags_test("./conf/overwrite_defaults.conf", + "/disabled_tags", None) + + def test_overwite_all_baggage_span_tags_using_wildcard(self): + return self.run_custom_tags_test( + "./conf/overwrite_defaults.conf", "/all_baggage_span_tags", + ("user.id", "session.id", "account.id", "snazzy.tag", "fancy.tag")) + + def test_overwite_default_tags_custom_ignores_previous_conf(self): + return self.run_custom_tags_test("./conf/overwrite_defaults.conf", + "/snazzy_tag", ("snazzy.tag"))