Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a DDTrace\generate_distributed_tracing_headers() function #1905

Merged
merged 1 commit into from
Feb 3, 2023

Conversation

bwoebi
Copy link
Collaborator

@bwoebi bwoebi commented Feb 3, 2023

Description

It's a direct opposite function to consume_distributed_tracing_headers, thereby trivially enabling users to use it for custom propoagation and later consumption.

Also needed for easy testing of parametric tests.

Readiness checklist

  • (only for Members) Changelog has been added to the release document.
  • Tests added for this feature/bug.

Reviewer checklist

  • Appropriate labels assigned.
  • Milestone is set.
  • Changelog has been added to the release document. For community contributors the reviewer is in charge of this task.

@bwoebi bwoebi added this to the 0.85.0 milestone Feb 3, 2023
@bwoebi bwoebi requested a review from a team as a code owner February 3, 2023 13:42
@bwoebi bwoebi force-pushed the bob/generate_distributed_tracing_headers branch from 4d368d7 to d0bb07f Compare February 3, 2023 14:27
It's a direct opposite function to consume_distributed_tracing_headers, thereby trivially enabling users to use it for custom propoagation and later consumption.

Also needed for easy testing of parametric tests.

And force phpunit < 10 to fix tests.
@bwoebi bwoebi force-pushed the bob/generate_distributed_tracing_headers branch from d0bb07f to 8c8c475 Compare February 3, 2023 15:13
Copy link
Collaborator

@morrisonlevi morrisonlevi left a comment

Choose a reason for hiding this comment

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

Approved, but have a question.

Comment on lines +2014 to +2015
if (get_DD_TRACE_ENABLED()) {
ddtrace_inject_distributed_headers(Z_ARR_P(return_value), true);
Copy link
Collaborator

Choose a reason for hiding this comment

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

If a given request had distributed headers passed to it, do we need to pass on those headers in outgoing requests even if the tracer is disabled? My memory isn't clear here, but IIRC there's something like that we should be doing.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

If the tracer is disabled, we don't even read the distributed headers. So, here, no, we should not do now.

Maybe, if it's something we should do, we need to refactor a couple things, but that should be separate work.

@bwoebi bwoebi merged commit 6c90cf6 into master Feb 3, 2023
@bwoebi bwoebi deleted the bob/generate_distributed_tracing_headers branch February 3, 2023 16:29
@bwoebi bwoebi mentioned this pull request Mar 14, 2023
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.

2 participants