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.

Sign up for GitHub

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

[api] Fix Baggage context leak issue #5208

Merged
merged 9 commits into from
Jan 16, 2024
Merged

[api] Fix Baggage context leak issue #5208

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
5101dd3
Remove the wrapper object to prevent Baggage context leaks.
CodeBlanch Jan 10, 2024
f0cf502
CHANGELOG patch.
CodeBlanch Jan 10, 2024
1607820
Remove TryGetBaggage.
CodeBlanch Jan 10, 2024
6cd16a0
CHANGELOG update.
CodeBlanch Jan 10, 2024
8b1bc93
Warning cleanup.
CodeBlanch Jan 10, 2024
570385f
CHANGELOG updates.
CodeBlanch Jan 10, 2024
5ebf697
Code review.
CodeBlanch Jan 11, 2024
386204b
Test tweak.
CodeBlanch Jan 12, 2024
6a4870f
Merge branch 'main' into api-baggage-context
CodeBlanch Jan 16, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 15 additions & 15 deletions src/OpenTelemetry.Api/.publicApi/Stable/PublicAPI.Shipped.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,13 @@
~abstract OpenTelemetry.Context.Propagation.TextMapPropagator.Extract<T>(OpenTelemetry.Context.Propagation.PropagationContext context, T carrier, System.Func<T, string, System.Collections.Generic.IEnumerable<string>> getter) -> OpenTelemetry.Context.Propagation.PropagationContext
~abstract OpenTelemetry.Context.Propagation.TextMapPropagator.Fields.get -> System.Collections.Generic.ISet<string>
~abstract OpenTelemetry.Context.Propagation.TextMapPropagator.Inject<T>(OpenTelemetry.Context.Propagation.PropagationContext context, T carrier, System.Action<T, string, string> setter) -> void
~OpenTelemetry.Baggage.GetBaggage() -> System.Collections.Generic.IReadOnlyDictionary<string, string>
~OpenTelemetry.Baggage.GetBaggage(string name) -> string
~OpenTelemetry.Baggage.GetEnumerator() -> System.Collections.Generic.Dictionary<string, string>.Enumerator
~OpenTelemetry.Baggage.RemoveBaggage(string name) -> OpenTelemetry.Baggage
~OpenTelemetry.Baggage.SetBaggage(params System.Collections.Generic.KeyValuePair<string, string>[] baggageItems) -> OpenTelemetry.Baggage
~OpenTelemetry.Baggage.SetBaggage(string name, string value) -> OpenTelemetry.Baggage
~OpenTelemetry.Baggage.SetBaggage(System.Collections.Generic.IEnumerable<System.Collections.Generic.KeyValuePair<string, string>> baggageItems) -> OpenTelemetry.Baggage
OpenTelemetry.Baggage.GetBaggage() -> System.Collections.Generic.IReadOnlyDictionary<string!, string!>!
OpenTelemetry.Baggage.GetBaggage(string! name) -> string?
OpenTelemetry.Baggage.GetEnumerator() -> System.Collections.Generic.Dictionary<string!, string!>.Enumerator
OpenTelemetry.Baggage.RemoveBaggage(string! name) -> OpenTelemetry.Baggage
OpenTelemetry.Baggage.SetBaggage(params System.Collections.Generic.KeyValuePair<string!, string?>[]! baggageItems) -> OpenTelemetry.Baggage
OpenTelemetry.Baggage.SetBaggage(string! name, string? value) -> OpenTelemetry.Baggage
OpenTelemetry.Baggage.SetBaggage(System.Collections.Generic.IEnumerable<System.Collections.Generic.KeyValuePair<string!, string?>>! baggageItems) -> OpenTelemetry.Baggage
~OpenTelemetry.Context.AsyncLocalRuntimeContextSlot<T>.AsyncLocalRuntimeContextSlot(string name) -> void
~OpenTelemetry.Context.AsyncLocalRuntimeContextSlot<T>.Value.get -> object
~OpenTelemetry.Context.AsyncLocalRuntimeContextSlot<T>.Value.set -> void
Expand All @@ -20,7 +20,7 @@
~OpenTelemetry.Context.ThreadLocalRuntimeContextSlot<T>.ThreadLocalRuntimeContextSlot(string name) -> void
~OpenTelemetry.Context.ThreadLocalRuntimeContextSlot<T>.Value.get -> object
~OpenTelemetry.Context.ThreadLocalRuntimeContextSlot<T>.Value.set -> void
~override OpenTelemetry.Baggage.Equals(object obj) -> bool
override OpenTelemetry.Baggage.Equals(object? obj) -> bool
~override OpenTelemetry.Context.Propagation.B3Propagator.Extract<T>(OpenTelemetry.Context.Propagation.PropagationContext context, T carrier, System.Func<T, string, System.Collections.Generic.IEnumerable<string>> getter) -> OpenTelemetry.Context.Propagation.PropagationContext
~override OpenTelemetry.Context.Propagation.B3Propagator.Fields.get -> System.Collections.Generic.ISet<string>
~override OpenTelemetry.Context.Propagation.B3Propagator.Inject<T>(OpenTelemetry.Context.Propagation.PropagationContext context, T carrier, System.Action<T, string, string> setter) -> void
Expand All @@ -34,13 +34,13 @@
~override OpenTelemetry.Context.Propagation.TraceContextPropagator.Extract<T>(OpenTelemetry.Context.Propagation.PropagationContext context, T carrier, System.Func<T, string, System.Collections.Generic.IEnumerable<string>> getter) -> OpenTelemetry.Context.Propagation.PropagationContext
~override OpenTelemetry.Context.Propagation.TraceContextPropagator.Fields.get -> System.Collections.Generic.ISet<string>
~override OpenTelemetry.Context.Propagation.TraceContextPropagator.Inject<T>(OpenTelemetry.Context.Propagation.PropagationContext context, T carrier, System.Action<T, string, string> setter) -> void
~static OpenTelemetry.Baggage.Create(System.Collections.Generic.Dictionary<string, string> baggageItems = null) -> OpenTelemetry.Baggage
~static OpenTelemetry.Baggage.GetBaggage(OpenTelemetry.Baggage baggage = default(OpenTelemetry.Baggage)) -> System.Collections.Generic.IReadOnlyDictionary<string, string>
~static OpenTelemetry.Baggage.GetBaggage(string name, OpenTelemetry.Baggage baggage = default(OpenTelemetry.Baggage)) -> string
~static OpenTelemetry.Baggage.GetEnumerator(OpenTelemetry.Baggage baggage = default(OpenTelemetry.Baggage)) -> System.Collections.Generic.Dictionary<string, string>.Enumerator
~static OpenTelemetry.Baggage.RemoveBaggage(string name, OpenTelemetry.Baggage baggage = default(OpenTelemetry.Baggage)) -> OpenTelemetry.Baggage
~static OpenTelemetry.Baggage.SetBaggage(string name, string value, OpenTelemetry.Baggage baggage = default(OpenTelemetry.Baggage)) -> OpenTelemetry.Baggage
~static OpenTelemetry.Baggage.SetBaggage(System.Collections.Generic.IEnumerable<System.Collections.Generic.KeyValuePair<string, string>> baggageItems, OpenTelemetry.Baggage baggage = default(OpenTelemetry.Baggage)) -> OpenTelemetry.Baggage
static OpenTelemetry.Baggage.Create(System.Collections.Generic.Dictionary<string!, string!>? baggageItems = null) -> OpenTelemetry.Baggage
static OpenTelemetry.Baggage.GetBaggage(OpenTelemetry.Baggage baggage = default(OpenTelemetry.Baggage)) -> System.Collections.Generic.IReadOnlyDictionary<string!, string!>!
static OpenTelemetry.Baggage.GetBaggage(string! name, OpenTelemetry.Baggage baggage = default(OpenTelemetry.Baggage)) -> string?
static OpenTelemetry.Baggage.GetEnumerator(OpenTelemetry.Baggage baggage = default(OpenTelemetry.Baggage)) -> System.Collections.Generic.Dictionary<string!, string!>.Enumerator
static OpenTelemetry.Baggage.RemoveBaggage(string! name, OpenTelemetry.Baggage baggage = default(OpenTelemetry.Baggage)) -> OpenTelemetry.Baggage
static OpenTelemetry.Baggage.SetBaggage(string! name, string? value, OpenTelemetry.Baggage baggage = default(OpenTelemetry.Baggage)) -> OpenTelemetry.Baggage
static OpenTelemetry.Baggage.SetBaggage(System.Collections.Generic.IEnumerable<System.Collections.Generic.KeyValuePair<string!, string?>>! baggageItems, OpenTelemetry.Baggage baggage = default(OpenTelemetry.Baggage)) -> OpenTelemetry.Baggage
~static OpenTelemetry.Context.Propagation.Propagators.DefaultTextMapPropagator.get -> OpenTelemetry.Context.Propagation.TextMapPropagator
~static OpenTelemetry.Context.RuntimeContext.ContextSlotType.get -> System.Type
~static OpenTelemetry.Context.RuntimeContext.ContextSlotType.set -> void
Expand Down
133 changes: 60 additions & 73 deletions src/OpenTelemetry.Api/Baggage.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
// Copyright The OpenTelemetry Authors
// SPDX-License-Identifier: Apache-2.0

using System.Diagnostics.CodeAnalysis;
#nullable enable

using System.Diagnostics;
using OpenTelemetry.Context;
using OpenTelemetry.Internal;

Expand All @@ -15,17 +17,21 @@ namespace OpenTelemetry;
/// </remarks>
public readonly struct Baggage : IEquatable<Baggage>
{
private static readonly RuntimeContextSlot<BaggageHolder> RuntimeContextSlot = RuntimeContext.RegisterSlot<BaggageHolder>("otel.baggage");
private static readonly Dictionary<string, string> EmptyBaggage = new();
private static readonly RuntimeContextSlot<Dictionary<string, string>> RuntimeContextSlot
= RuntimeContext.RegisterSlot<Dictionary<string, string>>("otel.baggage");

private static readonly Dictionary<string, string> EmptyBaggage = [];

private readonly Dictionary<string, string> baggage;
private readonly Dictionary<string, string>? baggage;

/// <summary>
/// Initializes a new instance of the <see cref="Baggage"/> struct.
/// </summary>
/// <param name="baggage">Baggage key/value pairs.</param>
internal Baggage(Dictionary<string, string> baggage)
{
Debug.Assert(baggage != null, "baggage was null");

this.baggage = baggage;
}

Expand Down Expand Up @@ -59,8 +65,8 @@ internal Baggage(Dictionary<string, string> baggage)
/// </remarks>
public static Baggage Current
{
get => RuntimeContextSlot.Get()?.Baggage ?? default;
set => EnsureBaggageHolder().Baggage = value;
get => new(RuntimeContextSlot.Get() ?? EmptyBaggage);
set => RuntimeContextSlot.Set(value.baggage ?? EmptyBaggage);
}

/// <summary>
Expand All @@ -87,7 +93,7 @@ public static Baggage Current
/// </summary>
/// <param name="baggageItems">Baggage key/value pairs.</param>
/// <returns><see cref="Baggage"/>.</returns>
public static Baggage Create(Dictionary<string, string> baggageItems = null)
public static Baggage Create(Dictionary<string, string>? baggageItems = null)
{
if (baggageItems == null)
{
Expand All @@ -97,13 +103,19 @@ public static Baggage Create(Dictionary<string, string> baggageItems = null)
Dictionary<string, string> baggageCopy = new Dictionary<string, string>(baggageItems.Count, StringComparer.OrdinalIgnoreCase);
foreach (KeyValuePair<string, string> baggageItem in baggageItems)
{
if (string.IsNullOrEmpty(baggageItem.Value))
if (string.IsNullOrEmpty(baggageItem.Key))
{
baggageCopy.Remove(baggageItem.Key);
continue;
}

baggageCopy[baggageItem.Key] = baggageItem.Value;
if (string.IsNullOrEmpty(baggageItem.Value))
{
baggageCopy.Remove(baggageItem.Key);
}
else
{
baggageCopy[baggageItem.Key] = baggageItem.Value;
}
}

return new Baggage(baggageCopy);
Expand All @@ -114,7 +126,6 @@ public static Baggage Create(Dictionary<string, string> baggageItems = null)
/// </summary>
/// <param name="baggage">Optional <see cref="Baggage"/>. <see cref="Current"/> is used if not specified.</param>
/// <returns>Baggage key/value pairs.</returns>
[SuppressMessage("roslyn", "RS0026", Justification = "TODO: fix APIs that violate the backcompt requirement - multiple overloads with optional parameters: https://github.com/dotnet/roslyn/blob/main/docs/Adding%20Optional%20Parameters%20in%20Public%20API.md.")]
public static IReadOnlyDictionary<string, string> GetBaggage(Baggage baggage = default)
=> baggage == default ? Current.GetBaggage() : baggage.GetBaggage();

Expand All @@ -132,8 +143,7 @@ public static Dictionary<string, string>.Enumerator GetEnumerator(Baggage baggag
/// <param name="name">Baggage item name.</param>
/// <param name="baggage">Optional <see cref="Baggage"/>. <see cref="Current"/> is used if not specified.</param>
/// <returns>Baggage item or <see langword="null"/> if nothing was found.</returns>
[SuppressMessage("roslyn", "RS0026", Justification = "TODO: fix APIs that violate the backcompt requirement - multiple overloads with optional parameters: https://github.com/dotnet/roslyn/blob/main/docs/Adding%20Optional%20Parameters%20in%20Public%20API.md.")]
public static string GetBaggage(string name, Baggage baggage = default)
public static string? GetBaggage(string name, Baggage baggage = default)
=> baggage == default ? Current.GetBaggage(name) : baggage.GetBaggage(name);

/// <summary>
Expand All @@ -144,16 +154,11 @@ public static string GetBaggage(string name, Baggage baggage = default)
/// <param name="baggage">Optional <see cref="Baggage"/>. <see cref="Current"/> is used if not specified.</param>
/// <returns>New <see cref="Baggage"/> containing the key/value pair.</returns>
/// <remarks>Note: The <see cref="Baggage"/> returned will be set as the new <see cref="Current"/> instance.</remarks>
[SuppressMessage("roslyn", "RS0026", Justification = "TODO: fix APIs that violate the backcompt requirement - multiple overloads with optional parameters: https://github.com/dotnet/roslyn/blob/main/docs/Adding%20Optional%20Parameters%20in%20Public%20API.md.")]
public static Baggage SetBaggage(string name, string value, Baggage baggage = default)
public static Baggage SetBaggage(string name, string? value, Baggage baggage = default)
{
var baggageHolder = EnsureBaggageHolder();
lock (baggageHolder)
{
return baggageHolder.Baggage = baggage == default
? baggageHolder.Baggage.SetBaggage(name, value)
: baggage.SetBaggage(name, value);
}
return Current = baggage == default
? Current.SetBaggage(name, value)
: baggage.SetBaggage(name, value);
}

/// <summary>
Expand All @@ -163,16 +168,11 @@ public static Baggage SetBaggage(string name, string value, Baggage baggage = de
/// <param name="baggage">Optional <see cref="Baggage"/>. <see cref="Current"/> is used if not specified.</param>
/// <returns>New <see cref="Baggage"/> containing the key/value pair.</returns>
/// <remarks>Note: The <see cref="Baggage"/> returned will be set as the new <see cref="Current"/> instance.</remarks>
[SuppressMessage("roslyn", "RS0026", Justification = "TODO: fix APIs that violate the backcompt requirement - multiple overloads with optional parameters: https://github.com/dotnet/roslyn/blob/main/docs/Adding%20Optional%20Parameters%20in%20Public%20API.md.")]
public static Baggage SetBaggage(IEnumerable<KeyValuePair<string, string>> baggageItems, Baggage baggage = default)
public static Baggage SetBaggage(IEnumerable<KeyValuePair<string, string?>> baggageItems, Baggage baggage = default)
{
var baggageHolder = EnsureBaggageHolder();
lock (baggageHolder)
{
return baggageHolder.Baggage = baggage == default
? baggageHolder.Baggage.SetBaggage(baggageItems)
: baggage.SetBaggage(baggageItems);
}
return Current = baggage == default
? Current.SetBaggage(baggageItems)
: baggage.SetBaggage(baggageItems);
}

/// <summary>
Expand All @@ -184,13 +184,9 @@ public static Baggage SetBaggage(IEnumerable<KeyValuePair<string, string>> bagga
/// <remarks>Note: The <see cref="Baggage"/> returned will be set as the new <see cref="Current"/> instance.</remarks>
public static Baggage RemoveBaggage(string name, Baggage baggage = default)
{
var baggageHolder = EnsureBaggageHolder();
lock (baggageHolder)
{
return baggageHolder.Baggage = baggage == default
? baggageHolder.Baggage.RemoveBaggage(name)
: baggage.RemoveBaggage(name);
}
return Current = baggage == default
? Current.RemoveBaggage(name)
: baggage.RemoveBaggage(name);
}

/// <summary>
Expand All @@ -201,13 +197,9 @@ public static Baggage RemoveBaggage(string name, Baggage baggage = default)
/// <remarks>Note: The <see cref="Baggage"/> returned will be set as the new <see cref="Current"/> instance.</remarks>
public static Baggage ClearBaggage(Baggage baggage = default)
{
var baggageHolder = EnsureBaggageHolder();
lock (baggageHolder)
{
return baggageHolder.Baggage = baggage == default
? baggageHolder.Baggage.ClearBaggage()
: baggage.ClearBaggage();
}
return Current = baggage == default
? Current.ClearBaggage()
: baggage.ClearBaggage();
}

/// <summary>
Expand All @@ -222,11 +214,11 @@ public IReadOnlyDictionary<string, string> GetBaggage()
/// </summary>
/// <param name="name">Baggage item name.</param>
/// <returns>Baggage item or <see langword="null"/> if nothing was found.</returns>
public string GetBaggage(string name)
public string? GetBaggage(string name)
{
Guard.ThrowIfNullOrEmpty(name);

return this.baggage != null && this.baggage.TryGetValue(name, out string value)
return this.baggage != null && this.baggage.TryGetValue(name, out var value)
? value
: null;
}
Expand All @@ -237,8 +229,10 @@ public string GetBaggage(string name)
/// <param name="name">Baggage item name.</param>
/// <param name="value">Baggage item value.</param>
/// <returns>New <see cref="Baggage"/> containing the key/value pair.</returns>
public Baggage SetBaggage(string name, string value)
public Baggage SetBaggage(string name, string? value)
{
Guard.ThrowIfNullOrEmpty(name);

if (string.IsNullOrEmpty(value))
{
return this.RemoveBaggage(name);
Expand All @@ -247,7 +241,7 @@ public Baggage SetBaggage(string name, string value)
return new Baggage(
new Dictionary<string, string>(this.baggage ?? EmptyBaggage, StringComparer.OrdinalIgnoreCase)
{
[name] = value,
[name] = value!,
});
}

Expand All @@ -256,17 +250,19 @@ public Baggage SetBaggage(string name, string value)
/// </summary>
/// <param name="baggageItems">Baggage key/value pairs.</param>
/// <returns>New <see cref="Baggage"/> containing the key/value pair.</returns>
public Baggage SetBaggage(params KeyValuePair<string, string>[] baggageItems)
=> this.SetBaggage((IEnumerable<KeyValuePair<string, string>>)baggageItems);
public Baggage SetBaggage(params KeyValuePair<string, string?>[] baggageItems)
=> this.SetBaggage((IEnumerable<KeyValuePair<string, string?>>)baggageItems);

/// <summary>
/// Returns a new <see cref="Baggage"/> which contains the new key/value pair.
/// </summary>
/// <param name="baggageItems">Baggage key/value pairs.</param>
/// <returns>New <see cref="Baggage"/> containing the key/value pair.</returns>
public Baggage SetBaggage(IEnumerable<KeyValuePair<string, string>> baggageItems)
public Baggage SetBaggage(IEnumerable<KeyValuePair<string, string?>> baggageItems)
{
if (baggageItems?.Any() != true)
Guard.ThrowIfNull(baggageItems);

if (!baggageItems.Any())
{
return this;
}
Expand All @@ -275,13 +271,18 @@ public Baggage SetBaggage(IEnumerable<KeyValuePair<string, string>> baggageItems

foreach (var item in baggageItems)
{
if (string.IsNullOrEmpty(item.Key))
{
continue;
}

if (string.IsNullOrEmpty(item.Value))
{
newBaggage.Remove(item.Key);
}
else
{
newBaggage[item.Key] = item.Value;
newBaggage[item.Key] = item.Value!;
}
}

Expand All @@ -295,7 +296,10 @@ public Baggage SetBaggage(IEnumerable<KeyValuePair<string, string>> baggageItems
/// <returns>New <see cref="Baggage"/> containing the key/value pair.</returns>
public Baggage RemoveBaggage(string name)
{
Guard.ThrowIfNullOrEmpty(name);

var baggage = new Dictionary<string, string>(this.baggage ?? EmptyBaggage, StringComparer.OrdinalIgnoreCase);

baggage.Remove(name);

return new Baggage(baggage);
Expand Down Expand Up @@ -325,11 +329,11 @@ public bool Equals(Baggage other)
return false;
}

return baggageIsNullOrEmpty || this.baggage.SequenceEqual(other.baggage);
return baggageIsNullOrEmpty || this.baggage!.SequenceEqual(other.baggage!);
}

/// <inheritdoc/>
public override bool Equals(object obj)
public override bool Equals(object? obj)
=> (obj is Baggage baggage) && this.Equals(baggage);

/// <inheritdoc/>
Expand All @@ -349,21 +353,4 @@ public override int GetHashCode()

return hash;
}

private static BaggageHolder EnsureBaggageHolder()
{
var baggageHolder = RuntimeContextSlot.Get();
if (baggageHolder == null)
{
baggageHolder = new BaggageHolder();
RuntimeContextSlot.Set(baggageHolder);
}

return baggageHolder;
}

private sealed class BaggageHolder
{
public Baggage Baggage;
}
}
Loading