Skip to content

[WIP] Adding Baggage Support #735

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

[WIP] Adding Baggage Support #735

wants to merge 3 commits into from

Conversation

PrinsonF77
Copy link

Description

The dapr runtime has added support for baggage propagation.
This PR allows for the users of the Dapr Go SDK to add baggage to the GRPC client, without having to import OpenTelemetry themselves.

Issue reference

Please reference the issue this PR will close: Issue 718

Checklist

Please make sure you've completed the relevant tasks for this PR, out of the following list:

  • [ ✅ ] Code compiles correctly
  • [ ✅ ] Created/updated tests
  • Extended the documentation

@PrinsonF77 PrinsonF77 requested review from a team as code owners June 10, 2025 07:08
Copy link
Contributor

@cicoyle cicoyle left a comment

Choose a reason for hiding this comment

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

Hey! Thanks for the PR here 🎉

Can you confirm that we don't need any additional logic to additionally support passing baggage in as headers? Dapr supports both ctx propagation and header propagation for consistency with tracing.

It might also be worth adding a tracing example to the repo showing how a user may use this, but at the very least we will want to add docs to the daprdocs directory

Copy link

codecov bot commented Jun 24, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 57.16%. Comparing base (6c59092) to head (8af1183).
Report is 25 commits behind head on main.

❗ There is a different number of reports uploaded between BASE (6c59092) and HEAD (8af1183). Click for more details.

HEAD has 3 uploads less than BASE
Flag BASE (6c59092) HEAD (8af1183)
4 1
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #735      +/-   ##
==========================================
- Coverage   62.52%   57.16%   -5.37%     
==========================================
  Files          56       58       +2     
  Lines        4139     4354     +215     
==========================================
- Hits         2588     2489      -99     
- Misses       1425     1736     +311     
- Partials      126      129       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@yaron2
Copy link
Member

yaron2 commented Jul 2, 2025

@mikeee Can you please look at this? 🙏

Copy link
Member

@mikeee mikeee left a comment

Choose a reason for hiding this comment

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

First pass, also agree with Cassie's comment. Feel free to ping if you need some help getting this over the finish line 😁

metadata, ok := metadata.FromOutgoingContext(ctx)
require.True(t, ok)
baggageString := metadata.Get(baggageHeader)
require.Equal(t, []string{"key1=value1,key2=value2"}, baggageString)
Copy link
Member

Choose a reason for hiding this comment

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

Do you think it would be pertinent to make this test accommodating of the non deterministic nature of the keys?

This test will be flaky otherwise

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

Successfully merging this pull request may close these issues.

Add Baggage Support
4 participants