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

feat(tracing): propagate gcp trace header in tracing plugins #11254

Merged
merged 1 commit into from
Aug 22, 2023

Conversation

jonabc
Copy link
Contributor

@jonabc jonabc commented Jul 19, 2023

Summary

This change parses and propagates the X-Trace-Context header that Google provides for calls in Google Cloud Projects (GCP). Google recommends using the OpenTelemetry tracing standard when instrumenting applications, and this provides handling of the header at the API gateway layer 👍

Checklist

Full changelog

  • OpenTelemetry: Support GCP X-Cloud-Trace-Context header
    The field header_type now accepts the value gcp to propagate the
    Google Cloud trace header

Issue reference

I didn't see an open issue for this, but I'm happy to open an issue and link it here if wanted.

@CLAassistant
Copy link

CLAassistant commented Jul 19, 2023

CLA assistant check
All committers have signed the CLA.

@jonabc
Copy link
Contributor Author

jonabc commented Aug 14, 2023

@hanshuebner 👋 I saw you gave a review on a similar PR for propagating AWS' xray trace headers, would you mind taking a look here as well? Sorry for the ping, this has been sitting around a little while and I'd love to have this included in Kong if possible 🙏

@hanshuebner
Copy link
Contributor

@samugi Can you have a look? This seems to be right up your alley.

Copy link
Member

@samugi samugi left a comment

Choose a reason for hiding this comment

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

Thank you so much for your contribution!

I gave this a first pass, I'll be looking into the tests next but I have already left some comments that might suggest at least one missing test case.

kong/tracing/propagation.lua Outdated Show resolved Hide resolved
kong/tracing/propagation.lua Outdated Show resolved Hide resolved
kong/tracing/propagation.lua Outdated Show resolved Hide resolved
kong/tracing/propagation.lua Outdated Show resolved Hide resolved
kong/tracing/propagation.lua Outdated Show resolved Hide resolved
@jonabc
Copy link
Contributor Author

jonabc commented Aug 18, 2023

@samugi thanks for the feedback! I think I've addressed everything you brought up in 75344f1 (force-pushed to rebase on top of latest kong master). Would you mind taking another look?

kong/tracing/propagation.lua Outdated Show resolved Hide resolved
spec/01-unit/26-tracing/02-propagation_spec.lua Outdated Show resolved Hide resolved
@jonabc jonabc force-pushed the feat/gcp-otel branch 2 times, most recently from 75bc1d2 to 0a6a9f0 Compare August 18, 2023 23:44
Copy link
Member

@samugi samugi left a comment

Choose a reason for hiding this comment

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

Almost there, with these tiny changes it should be ready!

kong/tracing/propagation.lua Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
Co-Authored-By: Samuele <samuele8@gmail.com>
@jonabc
Copy link
Contributor Author

jonabc commented Aug 21, 2023

@samugi thanks again! Updated (+ rebase on latest Kong:master) in e1b04fb. I also moved the changelog entry to unreleased, it looks like it was still set in 3.4.0.

Do you have any estimates on when 3.5.0 will be released, assuming this will go out with the next feature release?

@samugi
Copy link
Member

samugi commented Aug 22, 2023

assuming this will go out with the next feature release

@jonabc That is correct. I don't have a date to share yet but I expect 3.5.0 to be out roughly in 2-3 months considering 3.4.0 was just released. Again thanks so much for your contribution!

@samugi samugi merged commit 0897b43 into Kong:master Aug 22, 2023
@jonabc jonabc deleted the feat/gcp-otel branch August 22, 2023 15:03
@outsinre
Copy link
Contributor

Is this in EE?

@samugi
Copy link
Member

samugi commented Aug 25, 2023

It is now @outsinre

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants