Implement GCP X-Cloud-Trace-Context Propagator #1132
Conversation
|
Thanks for opening your first pull request! If you haven't yet signed our Contributor License Agreement (CLA), then please do so that we can accept your contribution. A link should appear shortly in this PR if you have not already signed one. |
ynikitin-etsy
force-pushed
the
ynikitin-etsy/implement-xcloudtrace-propagator
branch
from
e9cd79b to
2c3c170
Compare
|
Will reopen after further review. |
|
Hi @ynikitin-etsy this PR looked ok to me. If you want to discuss further, here is good or in our slack channel. |
|
@brettmc sorry about that. I had to go back for a CLA review, but this should be getting sorted out asap. I'm re-opening for review. Let me know if any changes are wanted. Thank you! |
There was a problem hiding this comment.
Can you please add this extension to the git hub actions (under .github), and also to .gitsplit.yml (I've created https://github.com/opentelemetry-php/extension-propagator-cloudtrace as a destination).
Also, it needs a basic README - you can copy the B3 one.
|
Looking good! I'd like to see the tests running, so don't forget to add to .github/workflows/php.yml and also .gitsplit.yml |
Fixes with tests/formating/README pushed. Added to .gitsplit.yml. Sorry, I'm not sure I'm understanding what I'm supposed to add to php.yml? Tests should run as is since they are just under the normal |
Sorry yes you are right, I'm thinking about our contrib repo which is set up differently... |
Sorry about, I tried running the makefile stuff before, but had issues with login to fetch the image and getting the right platform images. This should be all set? FYI to other people on ARM Mac's, if you put |
Codecov Report
@@ Coverage Diff @@
## main #1132 +/- ##
============================================
+ Coverage 84.27% 84.28% +0.01%
- Complexity 2160 2193 +33
============================================
Files 279 282 +3
Lines 6137 6224 +87
============================================
+ Hits 5172 5246 +74
- Misses 965 978 +13
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report in Codecov by Sentry.
|
Should be fixed. Hopefully I can sign it within next few days. Thanks a bunch for your time! |
|
Okey, this should be all set! I had to work through getting the CLA approved in my org. Thanks for waiting! |
|
@ynikitin-etsy I noticed in the readme that there's some differences in the naming of the repo vs the package name ( |
Sorry for the delay. I can update to cloudtrace so we don't have to change repos and etc. I picked xcloudtrace to make it stand out thats its has to do with header propagation, because cloud trace sounds a bit generic. Anyways, I can open another PR to update these. |
|
Opened this: #1142 |
This is an implementation for GCP/Google X-Cloud-Trace-Context header propagator. Propagator for this header is available in most languages except PHP and my team needs it, thus I'm presenting this PR for review.
The formatting/parsing logic is heavily borrowed from OpenCensus implementation with a few changes: https://github.com/census-instrumentation/opencensus-php/blob/master/src/Trace/Propagator/TraceContextFormatter.php
Tests are heavily borrowed from existing B3 Propagator tests in this library.
Please let me know how this needs to be adjusted. This has also been tested end to end with a Slim API and Honeycomb exporter.