Skip to content

Conversation

martin308
Copy link
Contributor

Adds a baggage span processor as per the other contrib repositories.

This processor takes items from the baggage and adds the items as attributes on spans.

Eg. open-telemetry/opentelemetry-go-contrib#5404

@martin308
Copy link
Contributor Author

The macOS tests look like they might be flaky but the linux test result is for my new tests will poke around and try to find the cause

Copy link
Member

@nachoBonafonte nachoBonafonte left a comment

Choose a reason for hiding this comment

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

Could you change it to a more expressible naming, it looks very generic like this
It also would need proper documentation in the source code for the future users, for the rest it LGTM

@martin308
Copy link
Contributor Author

From the sync, naming wise we will go BaggagePropagationProcessor

@martin308 martin308 force-pushed the martin308/baggage-span-processor-contrib branch from 2472eb4 to caef819 Compare February 19, 2025 01:39
@martin308
Copy link
Contributor Author

martin308 commented Feb 19, 2025

Updated with the following changes:

  • new name for the processor
  • moved to an individual library/target for the processor

I left the contrib tests as a single test target but we could break those up into individual test targets too if we think it is worth it?

@martin308
Copy link
Contributor Author

I think the only remaining question now is wether we want to have all the contrib tests in one test target or also have many test targets for each piece of contrib

Copy link
Member

@nachoBonafonte nachoBonafonte left a comment

Choose a reason for hiding this comment

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

Looks good to e, I don't mind having the contribution tests under the same test target or separated

@nachoBonafonte nachoBonafonte merged commit b132329 into open-telemetry:main Feb 20, 2025
9 checks passed
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.

4 participants