Add:HTTP header tagging with DD_TRACE_HEADER_TAGS for clients

[marcotc]

I recommend reviewing #2935 before this one, as most of the context is in there.

What does this PR do?

As a follow up to #2935, this PR adds the ability for clients to save the value of HTTP clients in their application's HTTP clients as span tags by configuring the DD_TRACE_HEADER_TAGS environment variable.

Motivation

This PR brings HTTP clients on par with HTTP servers, which are configurable through DD_TRACE_HEADER_TAGS since #2935.

Also, as soon as Remote Configure is available for Ruby Tracing, this configuration will be configurable remotely.

Additional Notes

How to test the change?

The test suite of each affected HTTP client has been modified to cover this new use case.
Running their test suites cover all changes in this PR.

[codecov-commenter]

Codecov Report

Merging #2946 (854fa13) into DD_TRACE_HEADER_TAGS (414d918) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@                  Coverage Diff                   @@
##           DD_TRACE_HEADER_TAGS    #2946    +/-   ##
======================================================
  Coverage                 98.09%   98.10%            
======================================================
  Files                      1305     1305            
  Lines                     72808    72938   +130     
  Branches                   3375     3379     +4     
======================================================
+ Hits                      71423    71554   +131     
+ Misses                     1385     1384     -1     

Impacted Files	Coverage Δ
lib/datadog/tracing/contrib/ethon/easy_patch.rb	100.00% <100.00%> (ø)
lib/datadog/tracing/contrib/excon/middleware.rb	94.94% <100.00%> (+0.32%)	⬆️
lib/datadog/tracing/contrib/faraday/middleware.rb	100.00% <100.00%> (ø)
...ib/datadog/tracing/contrib/http/instrumentation.rb	97.26% <100.00%> (+0.15%)	⬆️
...adog/tracing/contrib/httpclient/instrumentation.rb	95.52% <100.00%> (+0.28%)	⬆️
.../datadog/tracing/contrib/httprb/instrumentation.rb	92.95% <100.00%> (+0.42%)	⬆️
...tadog/tracing/contrib/rest_client/request_patch.rb	100.00% <100.00%> (ø)
...dog/tracing/contrib/ethon/integration_test_spec.rb	100.00% <100.00%> (ø)
...adog/tracing/contrib/excon/instrumentation_spec.rb	100.00% <100.00%> (ø)
...datadog/tracing/contrib/faraday/middleware_spec.rb	100.00% <100.00%> (ø)
... and 4 more

... and 2 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[GustavoCaso]

code LGTM. We need to make sure to fix system test failure on #2935 before merging both in master

[marcotc]

@GustavoCaso @lloeki, regrading the system-test failure in this PR, the issue is that we perform HTTP header tagging for sinatra.request spans, but that is not a service entry span in the Weblog test variant. Service entry spans get tagged by AppSec with extra HTTP headers,

dd-trace-rb/lib/datadog/appsec/event.rb

Line 64 in c0fc32e

service_entry_tags = build_service_entry_tags(event_group)

, but sinatra.request does not receive these tags.

And because Weblog variants have the environment variable DD_TRACE_HEADER_TAGS=user-agent, we are tagging http.request.headers.user-agent => Arachni/v1 rid/OAOALNJYOEAONLTFJKDKPHVCPORINRTNRCOO in the sinatra.request, as well as in the top-level rack.request span.

The issue happens because get_rid_from_span successfully returns the rid (request id) for this test for the sinatra.request. These means that sinatra.request is now selected (alongside the top-level rack.request span) as relevant for test assertion.

This causes the test assertion to fail because the remaining expected headers ("content-type", "content-length", "content-language"), at https://github.com/DataDog/system-tests/blob/072b94247b4179d589a7bddf058b25087c526c59/tests/appsec/test_traces.py#L354, are not present in sinatra.request because AppSec does not tag non-service entry spans.

I'm not too sure how this was working before, but I don't see where we could be setting all the 3 tags in all spans generated by this Weblog test run.

For reference, this is the stdout with the spans that are failing for this test (test name Test_CollectRespondHeaders):

 Name: sinatra.route
Span ID: 3547715443187073528
Parent ID: 991106531156891986
Trace ID: 556950475597441551
Type: web
Service: weblog
Resource: GET /headers
Error: 0
Start: 1689891643793740032
End: 1689891643793774080
Duration: 3.370000001723383e-05
Tags: [
   key1 => val1,
   key2  =>  val2,
   env => system-tests,
   version => 1.0.0,
   sinatra.app.name => Sinatra::Application,
   sinatra.route.path => /headers,
   component => sinatra,
   operation => route]
Metrics: [ _dd.measured => 1.0],
 
 Name: sinatra.request
Span ID: 991106531156891986
Parent ID: 2903024419441692001
Trace ID: 556950475597441551
Type: web
Service: weblog
Resource: GET /headers
Error: 0
Start: 1689891643792504832
End: 1689891643793895936
Duration: 0.0013905150000255162
Tags: [
   key1 => val1,
   key2  =>  val2,
   env => system-tests,
   version => 1.0.0,
   http.request.headers.user-agent => Arachni/v1 rid/OAOALNJYOEAONLTFJKDKPHVCPORINRTNRCOO,
   component => sinatra,
   operation => request,
   http.url => /headers,
   http.method => GET,
   sinatra.route.path => /headers,
   http.status_code => 200]
Metrics: [ _dd.measured => 1.0],
 
 Name: rack.request
Span ID: 2903024419441692001
Parent ID: 0
Trace ID: 556950475597441551
Type: web
Service: weblog
Resource: GET /headers
Error: 0
Start: 1689891643791098624
End: 1689891643794927616
Duration: 0.003829038999981549
Tags: [
   key1 => val1,
   key2  =>  val2,
   env => system-tests,
   version => 1.0.0,
   _dd.runtime_family => ruby,
   _dd.appsec.waf.version => 1.9.0,
   http.client_ip => 172.18.0.1,
   _dd.appsec.event_rules.version => 1.7.0,
   appsec.event => true,
   _dd.appsec.json => {"triggers":[{"rule":{"id":"ua0-600-12x","name":"Arachni","tags":{"type":"security_scanner","category":"attack_attempt"}},"rule_matches":[{"operator":"match_regex","operator_value":"^Arachni\\/v","parameters":[{"address":"server.request.headers.no_cookies","key_path":["user-agent"],"value":"Arachni/v1 rid/OAOALNJYOEAONLTFJKDKPHVCPORINRTNRCOO","highlight":["Arachni/v"]}]}]}]},
   http.request.headers.host => localhost:7777,
   http.request.headers.accept-encoding => identity,
   http.request.headers.user-agent => Arachni/v1 rid/OAOALNJYOEAONLTFJKDKPHVCPORINRTNRCOO,
   http.host => localhost,
   http.useragent => Arachni/v1 rid/OAOALNJYOEAONLTFJKDKPHVCPORINRTNRCOO,
   network.client.ip => 172.18.0.1,
   http.response.headers.content-type => text/plain,
   http.response.headers.content-language => en-US,
   http.response.headers.content-length => 15,
   _dd.origin => appsec,
   component => rack,
   operation => request,
   span.kind => server,
   http.method => GET,
   http.url => /headers,
   http.base_url => http://localhost:7777,
   http.status_code => 200]
Metrics: [
   _dd.appsec.enabled => 1.0,
   _dd.appsec.waf.timeouts => 0.0,
   _dd.appsec.waf.duration => 250.304,
   _dd.appsec.waf.duration_ext => 520.405,
   _dd.measured => 1.0,
   _dd.top_level => 1.0]]

A few solutions

1. Remove HTTP header tagging from sinatra.request spans, but Sinatra currently supports :headers configuration, and DD_TRACE_HEADER_TAGS is meant to control the :headers field (when an integration-specific :headers value is not provided).
2. Tweak the system test to skip sinatra.request checking.
3. Tweak weblog to make sinatra.request a service-entry span by changing the sinatra integration service_name.

None seem too clean, to be honest.

[GustavoCaso]

@marcotc Thanks so much for the detailed information on the current issue with the system test

I have a few questions:

Tweak weblog to make sinatra.request a service-entry span by changing the sinatra integration service_name.

How would that solve the issue? What is the underlying thing that happens underneath would make the test pass?

Tweak the system test to skip sinatra.request checking.

It might be risky, since this is affecting all languages, but it might be worth a try. Since we want the span with _dd.top_level and the right user-agent tag. What do you think?

Alternatively, if we see ourselves blocked by this, we could skip the test until we have a better solution.
@bug("sinatra" in context.weblog_variant, reason="wrong span selected for assertions")

[marcotc]

Opened a system-tests PR for this issue^: DataDog/system-tests#1430