fix: HTTPX tracing parallel requests

[ollym]

When performing parallel requests eg.

HTTPX.with_origin('https://api.ventrata.com').get('/', '/', '/‘)

The parallel requests have their context nested so appear like:

- HTTP GET
  - HTTP GET
    - HTTP GET

This change injects the headers only with that request's context so doesn't pollute the global state and the results are what we expect:

- HTTP GET
- HTTP GET
- HTTP GET

@HoneyryderChuck would appreciate your review as HTTPX is your project. Thanks!

[linux-foundation-easycla]

The committers listed above are authorized under a signed CLA.

• ✅ login: ollym / name: Oliver Morgan (5731e75, 173831c, 780ce06, ca06278, aa14567, 02f478b, 339613a, 6108efd, 7c02d50, aaf989b, d8a3a3c, cfb9d2f)
• ✅ login: kaylareopelle / name: Kayla Reopelle (5ad82b2)

[HoneyryderChuck]

👋 not so much into the internals of the otel lib to understand what's happening. FWIW this was based on excon's integration.

What I don't get is what's the problem of the current API, considering that the enhanced tracing context only injects the headers in the (single) request there. Did this work locally?

I'd also compare this integration to the datadog plugin implementation. The code was very similar back then, and I believe there were some bugs fixed in the datadog plugin which may not have been ported back to otel.

[ollym]

@HoneyryderChuck i've checked your datadog plugin and i think you have a bug in it. propagate_trace_http is called from initialize_span which is called from finish(response), but by which point it's too late to be adding request headers as the response is already finished. If you fix your datadog plugin (or clarify where I'm wrong) then i'll update the PR to match your behaviour.

[ollym]

Regardless, i've followed your datadog plugin minus the reset and moved the start span up to the call method from the finish call. Let me know if this aligns more closely.

[HoneyryderChuck]

i've checked your datadog plugin and i think you have a bug in it.

You are absolutely right! In fact, I have been investigating a bug report in the last weeks related to distributed headers, and had a working branch which the reporter hadn't picked up yet for testing. Your comment actually made me realize what was happening, thank you for that.

I did some additional changes to the structure. I'll be merging it today still, so feel free to use it as a blueprint.

[ollym]

Thanks @HoneyryderChuck i've updated the PR to match your datadog plugin as a blueprint. I'd still appreciate you to do a quick review of the change, just in case i'm missing anything. But I believe otherwise this should be good for review.

[ollym]

@HoneyryderChuck i just updated the PR with your proposed changes, can you take one final look please?

[ollym]

@arielvalentin what's the process for getting this merged?

[arielvalentin]

The gem owner and at least one open-telemetry/ruby-contrib-approver must review and approve the PR.

I'll review the PR later today and merge unless I find anything unusual.

Gems are typically released every Tuesday.

[ollym]

@HoneyryderChuck I followed your new datadog plugin nearly exactly but the tests now failing even worse than they were before. When you have time can you help take a look for me please? As I'm a little out of my depth as I'm neither an export of opentelemetry-ruby or httpx.

[ollym]

@HoneyryderChuck i spent some more time on this, and believe the issue with the tests is because
https://github.com/HoneyryderChuck/httpx/blob/master/lib/httpx/adapters/webmock.rb#L118
Your webmock adapter is not emitting :headers which this new blueprint relies on. Let me know how best to proceed.

[ollym]

@HoneyryderChuck I've proposed a change I that i think fixes this, let me know
HoneyryderChuck/httpx#73

[ollym]

@HoneyryderChuck better, but still getting failures:

# Running:

...F.F.FF.

Finished in 0.025210s, 396.6680 runs/s, 1705.6724 assertions/s.

  1) Failure:
OpenTelemetry::Instrumentation::HTTPX::Plugin::tracing#test_0003_after request with failure code [test/instrumentation/plugin_test.rb:63]:
The request GET http://example.com/failure with headers {'Traceparent'=>'00-ca385f94b8f0fbf874e7afa2ebb4bb21-d67c7958bb9ce91e-01'} was expected to execute 1 time but it executed 0 times

The following requests were made:

GET http://example.com/failure with headers {'Accept'=>'*/*', 'Accept-Encoding'=>'gzip, deflate', 'User-Agent'=>'httpx.rb/1.4.0'} was made 1 time

============================================================

  2) Failure:
OpenTelemetry::Instrumentation::HTTPX::Plugin::tracing#test_0004_after request timeout [test/instrumentation/plugin_test.rb:84]:
--- expected
+++ actual
@@ -1 +1 @@
-"Unhandled exception of type: HTTPX::TimeoutError"
+"Timed out"


  3) Failure:
OpenTelemetry::Instrumentation::HTTPX::Plugin::tracing#test_0005_merges HTTP client context [test/instrumentation/plugin_test.rb:111]:
The request GET http://example.com/success with headers {'Traceparent'=>'00-37f4e2c12c2d31675a6ec4bffc544ac7-c5dbb4cfd6b0dadb-01'} was expected to execute 1 time but it executed 0 times

The following requests were made:

GET http://example.com/success with headers {'Accept'=>'*/*', 'Accept-Encoding'=>'gzip, deflate', 'User-Agent'=>'httpx.rb/1.4.0'} was made 1 time

============================================================

  4) Failure:
OpenTelemetry::Instrumentation::HTTPX::Plugin::tracing#test_0002_after request with success code [test/instrumentation/plugin_test.rb:46]:
The request GET http://example.com/success with headers {'Traceparent'=>'00-1f7167c53ff4696d6a530f11bdd60b2d-ad1ec1e4642771f1-01'} was expected to execute 1 time but it executed 0 times

The following requests were made:

GET http://example.com/success with headers {'Accept'=>'*/*', 'Accept-Encoding'=>'gzip, deflate', 'User-Agent'=>'httpx.rb/1.4.0'} was made 1 time

============================================================

Seems like assert_requested is failing, is there anything in your webmock adapter that would have affected that? I'll fix the timeout error