Skip to content

Fix: preserve new baggage values in context on key conflict #1061

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

Merged

Conversation

amitsinha11
Copy link
Contributor

Fixes: #1060

@shakuzen
Copy link
Member

Thanks for the pull request. It'd help to add a test (or tests) to demonstrate the cases where this was failing and is now fixed.

@amitsinha11
Copy link
Contributor Author

@shakuzen UTs are added to demonstrate the fix.

@amitsinha11 amitsinha11 force-pushed the fix/context-baggage-overwrite branch from fd233a8 to 5f388a9 Compare May 30, 2025 10:28
@jonatan-ivanov
Copy link
Member

@amitsinha11 Thank for the PR, could you please also sign-off your commits so the DCO check can pass? You can read more about it by clicking on the failing check.

amitsinha11 pushed a commit to amitsinha11/micrometer-tracing that referenced this pull request May 31, 2025
@amitsinha11 amitsinha11 force-pushed the fix/context-baggage-overwrite branch from 5f388a9 to 906f93d Compare May 31, 2025 05:40
@amitsinha11
Copy link
Contributor Author

@jonatan-ivanov done, DCO check is ok now.

@shakuzen shakuzen requested a review from chemicL June 2, 2025 02:33
@shakuzen
Copy link
Member

shakuzen commented Jun 3, 2025

@chemicL could you review when you have a chance, in case we're missing anything from a Reactor or context-propagation perspective?

Copy link
Contributor

@chemicL chemicL left a comment

Choose a reason for hiding this comment

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

@shakuzen the change looks great, I have no reservations.

@amitsinha11
Copy link
Contributor Author

@shakuzen, could you merge this if all is ok?

@shakuzen
Copy link
Member

shakuzen commented Jun 5, 2025

It's on my list of things to do. I want to check on a few things before merging.

@shakuzen shakuzen force-pushed the fix/context-baggage-overwrite branch from 906f93d to 9fb865b Compare June 5, 2025 07:54
@shakuzen shakuzen changed the base branch from main to 1.3.x June 5, 2025 07:54
@shakuzen shakuzen merged commit e72dc6b into micrometer-metrics:1.3.x Jun 5, 2025
5 checks passed
@shakuzen
Copy link
Member

shakuzen commented Jun 5, 2025

Thanks for the pull request and quick updates on feedback. This is now merged to 1.3.x and forward through main, so it will be in upcoming patch releases for actively maintained branches. Feel free to try out snapshots before the next release.

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.

ReactorBaggage's append method overwrites new baggage with existing entries on key conflict
4 participants