Skip to content

Fix the Okhttp Otel parent span feature when not using RUM#2100

Merged
mariusc83 merged 1 commit into
developfrom
mconstantin/fix-otel-okhttp-parent-span-extension
Jun 21, 2024
Merged

Fix the Okhttp Otel parent span feature when not using RUM#2100
mariusc83 merged 1 commit into
developfrom
mconstantin/fix-otel-okhttp-parent-span-extension

Conversation

@mariusc83

Copy link
Copy Markdown
Member

What does this PR do?

A brief description of the change being made with this pull request.

Motivation

What inspired you to submit this pull request?

Additional Notes

Anything else we should know when reviewing?

Review checklist (to be filled by reviewers)

  • Feature or bugfix MUST have appropriate tests (unit, integration, e2e)
  • Make sure you discussed the feature or bugfix with the maintaining team in an Issue
  • Make sure each commit and the PR mention the Issue number (cf the CONTRIBUTING doc)

@mariusc83 mariusc83 self-assigned this Jun 21, 2024
@mariusc83
mariusc83 requested review from a team as code owners June 21, 2024 08:49
fun Request.Builder.addParentSpan(span: Span): Request.Builder {
val context = span.spanContext
val prioritySampling = if (context.isSampled) PrioritySampling.USER_KEEP.toInt() else PrioritySampling.UNSET.toInt()
// because the DatadogTracer is using the old legacy DDTracer for handling the spans around request

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is the DatadogTracer? I cannot find such symbol in the code

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes because it should be DatadogInterceptor there ;)

@mariusc83
mariusc83 force-pushed the mconstantin/fix-otel-okhttp-parent-span-extension branch from 097c3df to f41f886 Compare June 21, 2024 08:54
val prioritySampling = if (context.isSampled) PrioritySampling.USER_KEEP.toInt() else PrioritySampling.UNSET.toInt()
// because the DatadogInterceptor is using the old legacy DDTracer for handling the spans around request
// we need to provide the legacy sampling codes here.
val prioritySampling = if (context.isSampled) PrioritySampling.USER_KEEP else PrioritySampling.UNSET

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so the difference is only in the UNSET value? 0x80 vs Integer.MIN_VALUE. Do we have a possibility of such mismatch in other parts of the code?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes that was the problem, we do not have any other places this is just because we are using Otel here and on the other side we are using OpenTracing. We really need to get rid of that ASAP and switch everything to CoreTracer

@mariusc83
mariusc83 merged commit df0670f into develop Jun 21, 2024
@mariusc83
mariusc83 deleted the mconstantin/fix-otel-okhttp-parent-span-extension branch June 21, 2024 10:58
@xgouchet xgouchet added this to the 2.12.x milestone Jul 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants