Skip to content

Fix Activity.Baggage items Order#42659

Merged
tarekgh merged 3 commits intodotnet:masterfrom
tarekgh:FixActivityBaggageOrder
Sep 24, 2020
Merged

Fix Activity.Baggage items Order#42659
tarekgh merged 3 commits intodotnet:masterfrom
tarekgh:FixActivityBaggageOrder

Conversation

@tarekgh
Copy link
Member

@tarekgh tarekgh commented Sep 23, 2020

Fixes #42596

In pre 5.0 .NET versions, when calling Activity.AddBaggage, will add a new Baggage item to internal linked list. The list was LIFO, which means when calling Activity.Baggage it will return the list in the reverse order of the added items. Also, Activity.AddBaggage allow adding 2 items with same key and when calling Activity.GetBaggageItem to get a Baggage item value for a key which has duplication in the list, the API will return the last added item with that key.

In 5.0, we used the internal LinkedList for other new scenarios (e.g. AddEvent) which required to have the list maintained on the order of the input. This had a side effect on Baggage order. The Baggage order became FIFO. Also, affected Activity.GetBaggageItem which made it to return the first added item with the specified input key instead of last added items.

This is important to fix because Baggage is intended to be serialized over the wire across the process/machine. Looks users assumed specific order of the list even we didn't document the behavior. The change here is to ensure the old behavior to avoid breaking anyone and allow services using different version of the System.Diagnostics.DiagnosticSource to communicate reliably through serializing/deserializing the Baggage list.

@ghost
Copy link

ghost commented Sep 23, 2020

Tagging subscribers to this area: @tarekgh, @tommcdon, @pjanotti
See info in area-owners.md if you want to be subscribed.

Copy link
Member

@noahfalk noahfalk left a comment

Choose a reason for hiding this comment

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

Thanks @tarekgh! I suggested a small change

@tarekgh
Copy link
Member Author

tarekgh commented Sep 24, 2020

@noahfalk I have addressed your feedback. please let me know if you have any more feedback.

Copy link
Member

@noahfalk noahfalk left a comment

Choose a reason for hiding this comment

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

LGTM

@tarekgh tarekgh merged commit e2465c2 into dotnet:master Sep 24, 2020
@tarekgh tarekgh deleted the FixActivityBaggageOrder branch September 24, 2020 23:35
@tarekgh
Copy link
Member Author

tarekgh commented Sep 24, 2020

/backport to release/5.0

@github-actions
Copy link
Contributor

Started backporting to release/5.0: https://github.com/dotnet/runtime/actions/runs/271504744

@ghost ghost locked as resolved and limited conversation to collaborators Dec 7, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

No way to update or remove item in Activity.Baggage

4 participants