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

[Baggage span processor] Key predicate #5437

Closed
MikeGoldsmith opened this issue Apr 29, 2024 · 5 comments · Fixed by #5619
Closed

[Baggage span processor] Key predicate #5437

MikeGoldsmith opened this issue Apr 29, 2024 · 5 comments · Fixed by #5619
Milestone

Comments

@MikeGoldsmith
Copy link
Member

This issue is to track adding a method of selecting what baggage key entries should be copied.

Feedback in the JS contrib PR was to allow a user-provided predicate function. This puts the responsibility on the user to ensure sensitive baggage keys are not copied while also not prescribing how that is determined.

We had a similar feedback in the .NET contrib project but thought it was more complicated than just using a set of prefixes so created an issue to continue the discussion. The plain processor that copies all baggage entries (like using * in your example) is likely to be accepted first.

@Cirilla-zmh
Copy link
Contributor

Would anyone happen to be currently working on this feature? If not, I’d be more than willing to assist and contribute to the Go implementation. Please feel free to assign it to me.

@dmathieu
Copy link
Member

If this is to be implemented in all languages, shouldn't it have a specification update first?

@MikeGoldsmith
Copy link
Member Author

@dmathieu This is a OTel contrib component, there is no spec for it. I don't know of any specifications for contrib components, can you point me towards any? I'm happy to lobby for one if it should have one. I've done my best to make the initial contribution as consistent as possible and ideally it the key predicate enhancement also be consistent in each language, but I don't think it's strictly necessary.

@Cirilla-zmh The component has been accepted into Go, .NET, JS, Python & Ruby and waiting for feedback in Java. Each implementation copies all available baggage entries and still needs the key predict enhancement.

We also want the Java version to be accessible by the Java Agent so a user-defined function won't work for that. Instead, it would need to be configured using environment variables and as we know. However, getting consensus for environment variables may be tricky / slow.

I think a user-provided function is the most flexible and we can start with that. We can do prefix matching later using an env var.

@Cirilla-zmh
Copy link
Contributor

@MikeGoldsmith
Thank you for your response. However, I didn't notice an implementation for key predication of baggage in this project. I'm quite interested in this feature. Could you please point out where I might find it?

@MikeGoldsmith
Copy link
Member Author

@Cirilla-zmh it's currently not available right now. This issue is tracking adding. You're welcome to start on a PR that would allow a user configuring the processor to provide a user-defined function for key predicates.

MadVikingGod pushed a commit that referenced this issue Jun 6, 2024
## Which problem is this PR solving?
- Closes #5437 

## Short description of the changes
- Add `BaggageKeyPredicate` type and `AllowAllBaggageKeys`
implementation that allows all keys
- Add baggage key predicate as constructor parameter to processor
- Add unit tests to verify behaviour for prefix and regex
- Update README with examples of setting a filter

---------

Co-authored-by: Alex Boten <223565+codeboten@users.noreply.github.com>
@MrAlias MrAlias added this to the v1.28.0 milestone Jun 20, 2024
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 a pull request may close this issue.

4 participants