chore: deprecate sampling delegation (#179)

Author: dmehala

doc/API.md

@@ -180,27 +180,6 @@ whose value is the result of evaluating the specified `<value>` in the context
 of the current request.  `<value>` is a string that may contain
 `$`-[variables][2] (including those provided by this module).
 
-### `datadog_delegate_sampling`
-- **syntax** `datadog_delegate_sampling [on|off]`
-- **default** `off`
-- **context** `http`, `server`, `location`
-
-If `on`, and if nginx is making the trace sampling decision (i.e. if nginx is
-the first service in the trace), then delegate the sampling decision to the
-upstream service.  nginx will make a provisional sampling decision, and convey
-it along with the intention to delegate to the upstream. The upstream service
-might then make its own sampling decision and convey that decision back in the
-response. If the upstream does so, then nginx will use the upstream's sampling
-decision instead of the provisional decision.
-
-Sampling delegation exists to allow nginx to act as a reverse proxy for multiple
-different services, where the trace sampling decision can be better made within
-the service than it can within nginx.
-
-Sampling delegation is `off` by default. The directive's argument can be a
-variable expression that evaluates to either of `on` or `off`. If the
-directive's argument is omitted, then it is as if it were `on`.
-
 ### `datadog_tracing`
 - **syntax** `datadog_tracing on|off`
 - **default** `on`
@@ -328,22 +307,6 @@ request.
 
 This option is `off` by default, so that only one span is produced per request.
 
-### `datadog_allow_sampling_delegation_in_subrequests`
-
-- **syntax** `datadog_allow_sampling_delegation_in_subrequests [on|off]`
-- **default**: `off`
-- **context**: `http`, `server`, `location`
-
-If `on`, then honor `datadog_delegate_sampling` directives in contexts where
-nginx is making a subrequest, e.g. with the `auth_request` directive.
-
-This option is `off` by default, so that sampling delegation is not performed
-for subrequests.
-
-Note that in addition to this directive, the nginx configuration must also
-contain the `log_subrequest on;` directive in order for tracing to be enabled
-for subrequests.
-
 ### `datadog_appsec_enabled` (AppSec builds)
 
 - **syntax** `datadog_appsec_enabled [on|off]`

src/datadog_conf.h

@@ -208,26 +208,6 @@ struct datadog_loc_conf_t {
   // The oldest ancestor (the `http` block) has `depth` zero. Each subsequent
   // generation has the `depth` of its parent plus one.
   int depth;
-  // If "on", then sampling decisions will be delegated to the upstream at this
-  // location. If "off", then not.
-  ngx_flag_t sampling_delegation_enabled = NGX_CONF_UNSET;
-  // `sampling_delegation_directive` is the source location of the
-  // `datadog_delegate_sampling` directive that applies this location, if any.
-  conf_directive_source_location_t sampling_delegation_directive;
-  // `allow_sampling_delegation_in_subrequests_script` evaluates to one of "on"
-  // or "off". If "on", then locations used as subrequests, such as those
-  // created by the `ngx_http_auth_request_module`, can delegate the trace
-  // sampling decision to their upstream if so configured (e.g. by a
-  // `datadog_delegate_sampling` directive at that location or in an enclosing
-  // configuration context). If "off", then sampling delegation will not be
-  // performed for subrequests in affected locations, even if those locations
-  // are configured to delegate sampling.
-  ngx_flag_t allow_sampling_delegation_in_subrequests = NGX_CONF_UNSET;
-  // `allow_sampling_delegation_in_subrequests_directive` is the source location
-  // of the `datadog_allow_sampling_delegation_in_subrequests` directive that
-  // applies this location, if any.
-  conf_directive_source_location_t
-      allow_sampling_delegation_in_subrequests_directive;
 
 #ifdef WITH_WAF
   // the thread pool used to run the WAF on

src/datadog_directive.cpp

@@ -363,45 +363,6 @@ char *set_datadog_agent_url(ngx_conf_t *cf, ngx_command_t *command,
   return NGX_OK;
 }
 
-char *hijack_auth_request(ngx_conf_t *cf, ngx_command_t *command,
-                          void *conf) noexcept try {
-  // Call the underlying directive handler, and then insert the following:
-  //
-  //     auth_request_set $datadog_dummy_variable $datadog_auth_request_hook;
-  //
-  // The handler for $datadog_auth_request_hook will expose the current span to
-  // any sampling-delegation-relevant response headers from the subrequest. It
-  // must be evaluated within `auth_request_set` so that the
-  // `$upstream_http_...` variables refer to the subrequest's response, as
-  // opposed to the main upstream request's response.
-  ngx_int_t rcode =
-      datadog_conf_handler({.conf = cf, .skip_this_module = true});
-  if (rcode != NGX_OK) {
-    return static_cast<char *>(NGX_CONF_ERROR);
-  }
-
-  ngx_str_t args[] = {ngx_string("auth_request_set"),
-                      ngx_string("$datadog_dummy_variable"),
-                      ngx_string("$datadog_auth_request_hook")};
-  ngx_array_t args_array;
-  args_array.elts = static_cast<void *>(&args);
-  args_array.nelts = sizeof args / sizeof args[0];
-
-  auto old_args = cf->args;
-  cf->args = &args_array;
-  const auto guard = defer([&]() { cf->args = old_args; });
-
-  rcode = datadog_conf_handler({.conf = cf, .skip_this_module = true});
-  if (rcode != NGX_OK) {
-    return static_cast<char *>(NGX_CONF_ERROR);
-  }
-  return static_cast<char *>(NGX_CONF_OK);
-} catch (const std::exception &e) {
-  ngx_log_error(NGX_LOG_ERR, cf->log, 0, "hijack_auth_request failed: %s",
-                e.what());
-  return static_cast<char *>(NGX_CONF_ERROR);
-}
-
 char *warn_deprecated_command_1_2_0(ngx_conf_t *cf, ngx_command_t * /*command*/,
                                     void * /*conf*/) noexcept {
   const auto elements = static_cast<ngx_str_t *>(cf->args->elts);

src/datadog_directive.h

@@ -37,9 +37,6 @@ char *set_datadog_propagation_styles(ngx_conf_t *cf, ngx_command_t *command,
 
 char *set_datadog_agent_url(ngx_conf_t *, ngx_command_t *, void *conf) noexcept;
 
-char *hijack_auth_request(ngx_conf_t *cf, ngx_command_t *command,
-                          void *conf) noexcept;
-
 char *warn_deprecated_command_1_2_0(ngx_conf_t *cf, ngx_command_t * /*command*/,
                                     void * /*conf*/) noexcept;
 

src/ngx_http_datadog_module.cpp

@@ -272,28 +272,6 @@ static ngx_command_t datadog_commands[] = {
       0,
       nullptr},
 
-    { ngx_string("datadog_delegate_sampling"),
-      NGX_HTTP_MAIN_CONF | NGX_HTTP_SRV_CONF | NGX_HTTP_LOC_CONF | NGX_CONF_TAKE1 | NGX_CONF_NOARGS,
-      ngx_conf_set_flag_slot,
-      NGX_HTTP_LOC_CONF_OFFSET,
-      offsetof(datadog_loc_conf_t, sampling_delegation_enabled),
-      nullptr},
-
-    { ngx_string("datadog_allow_sampling_delegation_in_subrequests"),
-      NGX_HTTP_MAIN_CONF | NGX_HTTP_SRV_CONF | NGX_HTTP_LOC_CONF | NGX_CONF_TAKE1 | NGX_CONF_NOARGS,
-      ngx_conf_set_flag_slot,
-      NGX_HTTP_LOC_CONF_OFFSET,
-    offsetof(datadog_loc_conf_t, allow_sampling_delegation_in_subrequests),
-      nullptr},
-
-    // based on ngx_http_auth_request_module.c
-    { ngx_string("auth_request"),
-      NGX_HTTP_MAIN_CONF|NGX_HTTP_SRV_CONF|NGX_HTTP_LOC_CONF|NGX_CONF_TAKE1,
-      hijack_auth_request,
-      NGX_HTTP_LOC_CONF_OFFSET,
-      0,
-      NULL },
-
 #ifdef WITH_WAF
     {
       ngx_string("datadog_waf_thread_pool_name"),
@@ -825,14 +803,6 @@ static char *merge_datadog_loc_conf(ngx_conf_t *cf, void *parent,
     }
   }
 
-  ngx_conf_merge_value(conf->sampling_delegation_enabled,
-                       prev->sampling_delegation_enabled, 0);
-  ngx_conf_merge_value(conf->allow_sampling_delegation_in_subrequests,
-                       prev->allow_sampling_delegation_in_subrequests, 0);
-  conf->sampling_delegation_directive = prev->sampling_delegation_directive;
-  conf->allow_sampling_delegation_in_subrequests_directive =
-      prev->allow_sampling_delegation_in_subrequests_directive;
-
 #ifdef WITH_WAF
   if (conf->waf_pool == nullptr) {
     conf->waf_pool = prev->waf_pool;

src/request_tracing.cpp

@@ -27,18 +27,6 @@ namespace datadog {
 namespace nginx {
 namespace {
 
-bool should_delegate(ngx_http_request_t *request,
-                     datadog_loc_conf_t *loc_conf) {
-  if (request->parent != nullptr) {
-    return loc_conf->sampling_delegation_enabled;
-  }
-
-  // `request` is a subrequest, sampling delegated must be allowed for
-  // subrequest.
-  return loc_conf->sampling_delegation_enabled &&
-         loc_conf->allow_sampling_delegation_in_subrequests;
-}
-
 std::optional<std::string> eval_complex_value(ngx_http_complex_value_t *conf,
                                               ngx_http_request_t *request_) {
   if (conf == nullptr) return std::nullopt;
@@ -268,14 +256,10 @@ RequestTracing::RequestTracing(ngx_http_request_t *request,
   set_sample_rate_tag(request_, loc_conf_, *request_span_);
 
   // Inject the active span
-  dd::InjectionOptions injection_opts;
-  injection_opts.delegate_sampling_decision =
-      should_delegate(request_, loc_conf_);
-
   NgxHeaderWriter writer(request_);
   auto &span = active_span();
   span.set_tag("span.kind", "client");
-  span.inject(writer, injection_opts);
+  span.inject(writer);
 }
 
 void RequestTracing::on_change_block(ngx_http_core_loc_conf_t *core_loc_conf,
@@ -304,14 +288,10 @@ void RequestTracing::on_change_block(ngx_http_core_loc_conf_t *core_loc_conf,
   set_sample_rate_tag(request_, loc_conf_, *request_span_);
 
   // Inject the active span
-  dd::InjectionOptions injection_opts;
-  injection_opts.delegate_sampling_decision =
-      should_delegate(request_, loc_conf);
-
   NgxHeaderWriter writer(request_);
   auto &span = active_span();
   span.set_tag("span.kind", "client");
-  span.inject(writer, injection_opts);
+  span.inject(writer);
 }
 
 dd::Span &RequestTracing::active_span() {
@@ -366,12 +346,6 @@ void RequestTracing::on_log_request() {
 
   request_span_->set_end_time(finish_timestamp);
 
-  if (should_delegate(request_, loc_conf_)) {
-    NgxHeaderReader reader(&request_->headers_out.headers);
-    auto delegated = request_span_->read_sampling_delegation_response(reader);
-    if (!delegated.if_error()) return;
-  }
-
   // We care about sampling rules for the request span only, because it's the
   // only span that could be the root span.
   set_sample_rate_tag(request_, loc_conf_, *request_span_);