RUMM-2620 Allow the usage of OTel headers in distributed tracing#1229
Conversation
…st is sampled out. Needed for tracecontext headers.
| requestId, | ||
| fakeMethod, | ||
| fakeUrl, | ||
| fakeUrl.lowercase(Locale.US), |
There was a problem hiding this comment.
Because the regex for generating the host name in test changed to allow for upper case letters, and because of RUMM-2900 we must currently lowercase the expected result.
| if (length > 32) { | ||
| traceId = BigInteger.ZERO; | ||
| continue; | ||
| } else if (length > 16) { |
There was a problem hiding this comment.
This confuses me a little bit --
There'a an edge case where a 16 character hex digit can be larger than the supported 63-bits. This may not be an issue if we're always generating the ID.
But if we're potentially extracting these IDs from user set headers (which I'm told we shouldn't be) then we likely need to handle the edge case (though it's possible validateUIng64BitsID does that?)
There was a problem hiding this comment.
This is original code from the Java tracing lib.
validateUInt64BitsID only accepts IDs between 0 and 2^64-1. It throws an exception otherwise.
In Android we do support extracting the IDs from the Request. It's been like this for quite some time.
0xnm
left a comment
0xnm
left a comment
There was a problem hiding this comment.
Great work! I went though the main code so far, will cover tests a bit later (big PR).
0xnm
left a comment
0xnm
left a comment
There was a problem hiding this comment.
Went through the tests, added some comments, but overall looks good.
| fun rum_resource_tracking_with_first_party_hosts_and_header_types() { | ||
| // In this case because the Span is generated by the backend as "android.request" Span | ||
| // there is no metric automatically generated for it so we had to create a special | ||
| // custom metric: "rum_resource_tracking_with_first_party_hosts" to assert this behaviour |
There was a problem hiding this comment.
this test will add more hits to the existing rum_resource_tracking_with_first_party_hosts metric, triggering this monitor "[RUM] [Android] Nightly - rum_resource_tracking_with_first_party_hosts: Number of spans generated from resource tracking should not be greater than 2/day as we only have one use case that should generate this Span".
I think we should either modify the value in this monitor, or create a separate metric for this test (and update the previous metric, so that it doesn't cover the new one as well).
| new ContextualScopeManager(Config.get().getScopeDepthLimit(), createScopeEventFactory()), | ||
| HttpCodec.createInjector(config), | ||
| HttpCodec.createExtractor(config, config.getHeaderTags()), | ||
| new ContextualScopeManager(config.get().getScopeDepthLimit(), createScopeEventFactory()), |
There was a problem hiding this comment.
SHouldn't this one be config and not config.get()?
There was a problem hiding this comment.
In theory yes, but for the sake of this PR it doesn't make a difference because we don't alter the properties that the ContextualScopeManager accesses.
I'll revert to avoid changing the original lib as much as possible.
Codecov Report
@@ Coverage Diff @@
## develop #1229 +/- ##
===========================================
- Coverage 82.82% 82.05% -0.78%
===========================================
Files 358 361 +3
Lines 12582 12827 +245
Branches 2093 2152 +59
===========================================
+ Hits 10421 10524 +103
- Misses 1523 1652 +129
- Partials 638 651 +13
|
mariusc83
left a comment
mariusc83
left a comment
There was a problem hiding this comment.
Apart from all the other comments LGTM
6 participants
What does this PR do?
This PR introduces the ability to select OTel headers (B3, B3Multi & Trace Context) to be injected in requests made to first party hosts.
Here we introduce a new configuration API
setFirstPartyHostsWithHeaderTypeallowing several types of tracing header to be configured for each host.Additional Notes
I've removed the optimization of not creating a new Span in
TracingInterceptoras Trace Context header specification requires spanId and traceId to be propagated, even if the request is sampled out.Review checklist (to be filled by reviewers)