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

Sandbox cURL (PHP 5) #911

Merged
merged 6 commits into from Jun 9, 2020
Merged

Sandbox cURL (PHP 5) #911

merged 6 commits into from Jun 9, 2020

Conversation

SammyK
Copy link
Contributor

@SammyK SammyK commented Jun 2, 2020

Description

This is a follow up PR to #814. This sandboxes the cURL integration on PHP 5.6 (PHP 5.4 is still limited to the legacy API so it does not apply.)

As in #814, this PR also provides custom internal function handlers for curl_exec, curl_copy_handle, curl_close, curl_setopt, and curl_setopt_array in order to inject distributed tracing headers. This PR differs slightly from #814 by injecting the distributed tracing headers with a single userland call to DDTrace\Bridge\curl_inject_distributed_headers() right before the curl_exec() call. This approach can be used for PHP 7 to reduce the dependency on the userland API.

Readiness checklist

  • (only for Members) Changelog has been added to the appropriate release draft. Create one if necessary.
  • Tests added for this feature/bug.

Reviewer checklist

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

@SammyK SammyK added c-extension Apply this label to issues and prs related to the C-extension 📦 Sandbox API labels Jun 2, 2020
@SammyK SammyK added this to the 0.47.0 milestone Jun 2, 2020
@SammyK SammyK force-pushed the sammyk/build/sandbox-curl-php5 branch from 70136da to 6cb95c9 Compare June 4, 2020 23:01
@SammyK SammyK force-pushed the sammyk/build/sandbox-curl-php5 branch from 6cb95c9 to 1da2232 Compare June 4, 2020 23:27
@morrisonlevi morrisonlevi force-pushed the sammyk/build/sandbox-curl-php5 branch from 6f38016 to abc2853 Compare June 5, 2020 16:51
@morrisonlevi morrisonlevi force-pushed the sammyk/build/sandbox-curl-php5 branch from abc2853 to 0066135 Compare June 5, 2020 17:32
morrisonlevi and others added 2 commits June 5, 2020 12:08
This is needed in oder to support the configurable sampler which allows custom sampling priorities per service or span name.
@SammyK SammyK force-pushed the sammyk/build/sandbox-curl-php5 branch from c28be00 to d2672f7 Compare June 9, 2020 17:34
@SammyK SammyK marked this pull request as ready for review June 9, 2020 17:37
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.

Testing if static analysis is required for merge.

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.

Looks good, Sammy. Thanks for wading through PHP 5!

@SammyK SammyK merged commit 874cb6d into master Jun 9, 2020
@SammyK SammyK deleted the sammyk/build/sandbox-curl-php5 branch June 9, 2020 21:19
@morrisonlevi morrisonlevi mentioned this pull request Jun 29, 2020
5 tasks
@morrisonlevi morrisonlevi mentioned this pull request Jul 7, 2020
@SammyK SammyK mentioned this pull request Jul 27, 2020
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c-extension Apply this label to issues and prs related to the C-extension 📦 Sandbox API
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants