Skip to content

Commit 02e6606

Browse files
authored
refactor: tracing injection mechanism (#79)
This commit introduces a refined header injection mechanism that modify the request structure with headers to inject, instead of mutating nginx's configuration with `proxy_set_header`, `fastcgi_param`, `grpc_set_header` or `uwsgi_param`. Resolves #19 Resolves #43 Changes: - Add a DictWriter implementation for nginx. - Add a test covering issue #19. - Deprecate `opentracing_*_propagate_context`. - Remove tests no longer necessary. - Update existing tests to align with the new implementation.
1 parent 73873c6 commit 02e6606

30 files changed

+434
-1230
lines changed

CMakeLists.txt

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,6 @@ target_sources(ngx_http_datadog_module
9494
src/ngx_http_datadog_module.cpp
9595
src/ngx_logger.cpp
9696
src/ngx_script.cpp
97-
src/propagation_header_querier.cpp
9897
src/request_tracing.cpp
9998
src/string_util.cpp
10099
src/tracing_library.cpp

doc/API.md

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -539,19 +539,6 @@ the environment, then the variable expands to a hyphen character (`-`) instead.
539539

540540
This family of variables is used in the tests for the Datadog nginx module.
541541

542-
### `datadog_propagation_header_*`
543-
`$datadog_propagation_header_<header>` expands to the value of the specified
544-
HTTP `<header>` as it would appear in a request to a service proxied using the
545-
`proxy_pass` directive.
546-
547-
`<header>` is transformed into the name of an HTTP request header by replacing
548-
underscores with hyphens, e.g. `$datadog_propagation_header_x_datadog_origin`
549-
expands to what would be the value of the `X-Datadog-Origin` header were trace
550-
context to be propagated to a service via `proxy_pass`.
551-
552-
This family of variables is used in the implementation of the Datadog nginx
553-
module.
554-
555542
### `datadog_auth_request_hook`
556543
This is an implementation detail of the module and should not be used.
557544

nginx-module.cmake

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,13 @@ separate_arguments(NGINX_CONF_ARGS NATIVE_COMMAND "${NGINX_CONF_ARGS}")
1010

1111
list(APPEND NGINX_CONF_ARGS
1212
"--add-dynamic-module=${CMAKE_SOURCE_DIR}/module/"
13-
"--with-threads"
1413
"--with-compat"
1514
)
15+
16+
if (NGINX_DATADOG_ASM_ENABLED)
17+
list(APPEND NGINX_CONF_ARGS "--with-threads")
18+
endif ()
19+
1620
if(CMAKE_BUILD_TYPE STREQUAL "Debug")
1721
list(APPEND NGINX_CONF_ARGS "--with-debug")
1822
endif()

src/datadog_conf.h

Lines changed: 0 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,6 @@ struct datadog_main_conf_t {
8686
// (e.g. before the first `server` block, before the first `access_log`
8787
// directive).
8888
bool are_log_formats_defined;
89-
std::vector<std::string_view> span_context_keys;
9089
// This module automates the forwarding of the environment variables in
9190
// `TracingLibrary::environment_variable_names()`. Rather than injecting
9291
// `env` directives into the configuration, or mucking around with the core
@@ -109,13 +108,6 @@ struct datadog_main_conf_t {
109108
std::optional<configured_value_t> environment;
110109
// `agent_url` is set by the `datadog_agent_url` directive.
111110
std::optional<configured_value_t> agent_url;
112-
// `is_sampling_delegation_response_header_added` is whether we have added an
113-
// `add_header` directive before the first `server` block within the
114-
// configuration. The directive is used to include the
115-
// X-Datadog-Trace-Sampling-Decision response header when the client
116-
// requested trace sampling delegation. This `bool` ensures that the
117-
// directive is not added more than once.
118-
bool is_sampling_delegation_response_header_added;
119111

120112
#ifdef WITH_WAF
121113
// DD_APPSEC_ENABLED
@@ -196,10 +188,6 @@ struct datadog_loc_conf_t {
196188
NgxScript loc_resource_name_script;
197189
ngx_flag_t trust_incoming_span = NGX_CONF_UNSET;
198190
ngx_array_t *tags;
199-
// `response_info_script` is a script containing variables that refer
200-
// to HTTP response headers. This is used by sampling delegation to
201-
// retrieve the sampling decision made by an upstream service.
202-
NgxScript response_info_script;
203191
// `proxy_directive` is the name of the configuration directive used to proxy
204192
// requests at this location, i.e. `proxy_pass`, `grpc_pass`, or
205193
// `fastcgi_pass`. If this location does not have such a directive directly
@@ -226,15 +214,6 @@ struct datadog_loc_conf_t {
226214
// `sampling_delegation_directive` is the source location of the
227215
// `datadog_delegate_sampling` directive that applies this location, if any.
228216
conf_directive_source_location_t sampling_delegation_directive;
229-
// `is_sampling_delegation_response_header_added` is whether we have added an
230-
// `add_header` directive before the first user-specified `add_header`
231-
// directive within this server/location block. The directive is used to
232-
// include the X-Datadog-Trace-Sampling-Decision response header when the
233-
// client requested trace sampling delegation. The inserted `add_header` is
234-
// necessary only if there are user-defined `add_header` directives at the
235-
// same level; if not, then the `add_header` is inherited from the `http`
236-
// block.
237-
bool is_sampling_delegation_response_header_added;
238217
// `block_type` is the name of the kind of configuration block we're in, e.g.
239218
// "http", "server", "location", or "if". `block_type` is used by some
240219
// configuration directives to alter their behavior based on the current

src/datadog_context.cpp

Lines changed: 0 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -86,17 +86,6 @@ void DatadogContext::on_log_request(ngx_http_request_t *request) {
8686
#endif
8787
}
8888

89-
ngx_str_t DatadogContext::lookup_propagation_header_variable_value(
90-
ngx_http_request_t *request, std::string_view key) {
91-
auto trace = find_trace(request);
92-
if (trace == nullptr) {
93-
throw std::runtime_error{
94-
"lookup_propagation_header_variable_value failed: could not find "
95-
"request trace"};
96-
}
97-
return trace->lookup_propagation_header_variable_value(key);
98-
}
99-
10089
ngx_str_t DatadogContext::lookup_span_variable_value(
10190
ngx_http_request_t *request, std::string_view key) {
10291
auto trace = find_trace(request);
@@ -107,17 +96,6 @@ ngx_str_t DatadogContext::lookup_span_variable_value(
10796
return trace->lookup_span_variable_value(key);
10897
}
10998

110-
ngx_str_t DatadogContext::lookup_sampling_delegation_response_variable_value(
111-
ngx_http_request_t *request) {
112-
auto trace = find_trace(request);
113-
if (trace == nullptr) {
114-
throw std::runtime_error{
115-
"lookup_sampling_delegation_response_variable_value failed: could not "
116-
"find request trace"};
117-
}
118-
return trace->lookup_sampling_delegation_response_variable_value();
119-
}
120-
12199
RequestTracing *DatadogContext::find_trace(ngx_http_request_t *request) {
122100
const auto found = std::find_if(
123101
traces_.begin(), traces_.end(),

src/datadog_context.h

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@
66
#include <vector>
77

88
#include "datadog_conf.h"
9-
#include "propagation_header_querier.h"
109
#include "request_tracing.h"
1110
#ifdef WITH_WAF
1211
#include "security/context.h"
@@ -41,15 +40,9 @@ class DatadogContext {
4140

4241
void on_log_request(ngx_http_request_t* request);
4342

44-
ngx_str_t lookup_propagation_header_variable_value(
45-
ngx_http_request_t* request, std::string_view key);
46-
4743
ngx_str_t lookup_span_variable_value(ngx_http_request_t* request,
4844
std::string_view key);
4945

50-
ngx_str_t lookup_sampling_delegation_response_variable_value(
51-
ngx_http_request_t* request);
52-
5346
RequestTracing& single_trace();
5447

5548
private:

0 commit comments

Comments
 (0)