Skip to content

trigger-http: Fix otel context propagation #3248

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
Closed
wants to merge 1 commit into from
Closed

trigger-http: Fix otel context propagation #3248

wants to merge 1 commit into from

Conversation

@lann
Copy link
Collaborator

@lann lann commented Aug 27, 2025 •
edited
Loading

The debug_span! was disabled by default tracing config, preventing otel context from being propagated through it.

@lann lann requested a review from calebschoepp August 27, 2025 20:50
@lann lann force-pushed the fix-otel-span-propagation branch from 131c297 to 5074e3c Compare August 27, 2025 20:52
@lann lann requested review from itowlson and removed request for calebschoepp August 27, 2025 20:52
@lann lann closed this Aug 27, 2025
@lann lann removed the request for review from itowlson August 27, 2025 20:53
@lann
Copy link
Collaborator Author

lann commented Aug 27, 2025

I made a hot mess of two parallel branches...

@lann lann reopened this Aug 27, 2025
@lann
trigger-http: Fix otel context propagation
3359e09 
The debug_span! was disabled by default tracing config, preventing otel
context from being propagated through it.

Signed-off-by: Lann Martin <lann.martin@fermyon.com>
@lann lann force-pushed the fix-otel-span-propagation branch from 5074e3c to 3359e09 Compare August 27, 2025 20:54
@lann lann changed the title outbound-http: Improve test assertion logging trigger-http: Fix otel context propagation Aug 27, 2025
@lann lann requested a review from calebschoepp August 27, 2025 20:54
calebschoepp
calebschoepp reviewed Aug 27, 2025
View reviewed changes
Copy link
Collaborator

@calebschoepp calebschoepp left a comment

Choose a reason for hiding this comment

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

image

This span was debug intentionally so that it wouldn't pollute the traces developers see. execute_wasi is redundant with execute_wasm_component foo.

Is there another way we could solve this problem?

@lann
Copy link
Collaborator Author

lann commented Aug 27, 2025

We could try removing it, just do in_current_span 🤷

@calebschoepp
Copy link
Collaborator

calebschoepp commented Aug 29, 2025

@lann were you going to try that or were you wanting me to try that?

@lann
Copy link
Collaborator Author

lann commented Aug 29, 2025

Either is fine with me I just might not get to it immediately...

@calebschoepp
Copy link
Collaborator

calebschoepp commented Aug 29, 2025

Tested in_current_span(). We correctly get the only two spans we want. I still need to test if this fixes the issue Andrew was experiencing.

image

@calebschoepp
Copy link
Collaborator

calebschoepp commented Aug 29, 2025 •
edited
Loading

Did some testing and it seems like neither the info span or in_current_span fix actually do anything to help with the issue Andrew was observing of the otel context not being propagated.

Actually I was just testing it wrong. Both ways work so I propose we go forward in_current_span.

@calebschoepp calebschoepp mentioned this pull request Aug 29, 2025
Merged
@lann
Copy link
Collaborator Author

lann commented Aug 29, 2025

#3260

@lann lann closed this Aug 29, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@calebschoepp calebschoepp calebschoepp left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@lann @calebschoepp