Skip to content

otel/xxray: add custom X-Ray propagator that preserves X-Amzn-Trace-Id #3668

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.

Sign up for GitHub

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

Jump to bottom
Draft
AlexanderYastrebov wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

otel/xxray: add custom X-Ray propagator that preserves X-Amzn-Trace-Id #3668

AlexanderYastrebov wants to merge 1 commit into from

Conversation

@AlexanderYastrebov
Copy link
Contributor

@AlexanderYastrebov AlexanderYastrebov commented Oct 15, 2025

Updates #3663

@AlexanderYastrebov AlexanderYastrebov mentioned this pull request Oct 15, 2025
Open
@AlexanderYastrebov AlexanderYastrebov added major moderate risk, for example new API, small filter changes that have no risk like refactoring or logs tracing tracing or opentracing related issue labels Oct 15, 2025
AlexanderYastrebov
AlexanderYastrebov commented Oct 15, 2025
View reviewed changes
otel/otel.go Outdated
}

func init() {
autoprop.RegisterTextMapPropagator("xxray", xxray.NewPropagatorGenerator())
Copy link
Contributor Author

@AlexanderYastrebov AlexanderYastrebov Oct 15, 2025

Choose a reason for hiding this comment

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

We can also add a config field e.g .PropagatorXRayReuseTraceID instead of a new name and create custom implementation if it is set.

AlexanderYastrebov
AlexanderYastrebov commented Oct 15, 2025
View reviewed changes
otel/xxray/propagator.go
}
value := part[equalsIndex+1:]
switch {
case strings.HasPrefix(part, traceIDKey):
Copy link
Contributor Author

@AlexanderYastrebov AlexanderYastrebov Oct 15, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

This is ugly and also allows RootFooBar=123 key 🤷 . Parsing should be done via strings.Cut or similar to #3405

@universam1
Copy link
Contributor

universam1 commented Oct 16, 2025

maybe we can add these samples in the test: https://docs.aws.amazon.com/elasticloadbalancing/latest/application/load-balancer-request-tracing.html

szuecs
szuecs reviewed Oct 16, 2025
View reviewed changes
otel/xxray/propagator.go
@@ -0,0 +1,159 @@
package xxray
Copy link
Member

@szuecs szuecs Oct 16, 2025

Choose a reason for hiding this comment

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

Why not xray?

@universam1
Copy link
Contributor

universam1 commented Oct 27, 2025

Tested this in lab and it is promising, the ALB trace-id is forwarded!

example headers forwarded:

"traceparent": "00-68ff8fe86949c39c171cef723838f6ed-2b5fdb1944ad4911-00",
"x-amzn-trace-id": "Root=1-68ff8fe8-6949c39c171cef723838f6ed;Parent=2b5fdb1944ad4911;Sampled=0",

Just it looks like the flow-id log output is not covered here yet, would it not be correct to fill it at this place as well?
@szuecs @AlexanderYastrebov

skipper-ingress-678759b679-6kvxp {"audit":"","auth-user":"","duration":1,"flow-id":"",...

@szuecs
Copy link
Member

szuecs commented Oct 27, 2025

@universam1 can't you use https://opensource.zalando.com/skipper/reference/filters/#tracingtag ?
tracingTag("http.flow_id", "${request.header.X-Flow-Id}")

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

Reviewers

@szuecs szuecs szuecs left review comments

Assignees

No one assigned

Labels

major moderate risk, for example new API, small filter changes that have no risk like refactoring or logs tracing tracing or opentracing related issue

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@AlexanderYastrebov @universam1 @szuecs