Skip to content

[release/5.0] Fix Activity.Baggage items Order#42712

Merged
tarekgh merged 3 commits into
release/5.0from
backport/pr-42659-to-release/5.0
Sep 28, 2020
Merged

[release/5.0] Fix Activity.Baggage items Order#42712
tarekgh merged 3 commits into
release/5.0from
backport/pr-42659-to-release/5.0

Conversation

@github-actions

@github-actions github-actions Bot commented Sep 24, 2020

Copy link
Copy Markdown
Contributor

Backport of #42659 to release/5.0

/cc @tarekgh

Customer Impact

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.

Testing

I have manually tested the affected scenario, run all regression tests, and added new tests for more scenarios was not covered before.

Risk

The change is scoped to the Baggage only and shouldn't affect any other scenarios which make the risk smaller.

@ghost

ghost commented Sep 24, 2020

Copy link
Copy Markdown

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

@tarekgh
tarekgh requested a review from noahfalk September 25, 2020 00:04
@tarekgh
tarekgh merged commit 387d033 into release/5.0 Sep 28, 2020
@tarekgh
tarekgh deleted the backport/pr-42659-to-release/5.0 branch September 28, 2020 16:39
dougbu added a commit to dotnet/aspnetcore that referenced this pull request Oct 28, 2020
@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.

3 participants