Skip to content

Commit

Permalink
[api] Fix Baggage context leak issue (#5208)
Browse files Browse the repository at this point in the history
  • Loading branch information
@CodeBlanch
CodeBlanch authored Jan 16, 2024
1 parent 3a5134f commit 90a9c6c
Show file tree
Hide file tree
Showing 4 changed files with 156 additions and 113 deletions.
30 changes: 15 additions & 15 deletions src/OpenTelemetry.Api/.publicApi/Stable/PublicAPI.Shipped.txt
View file
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
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

0 comments on commit 90a9c6c

Please sign in to comment.