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.

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

Add support for samplers to modify Tracestate #3610

Merged
2 changes: 2 additions & 0 deletions .vscode/settings.json
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
"contoso",
"cref",
"grafana",
"inheritdoc",
"janotti",
"kanzhelev",
"liudmila",
Expand Down Expand Up @@ -36,6 +37,7 @@
"struct",
"tbody",
"thead",
"Tracestate",
"tracestate",
"triager",
"umesan",
Expand Down
3 changes: 3 additions & 0 deletions src/OpenTelemetry/.publicApi/net462/PublicAPI.Unshipped.txt
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@ OpenTelemetry.Logs.OpenTelemetryLoggerOptionsExtensions
OpenTelemetry.Logs.OpenTelemetryLoggerProvider.AddProcessor(OpenTelemetry.BaseProcessor<OpenTelemetry.Logs.LogRecord!>! processor) -> OpenTelemetry.Logs.OpenTelemetryLoggerProvider!
OpenTelemetry.Logs.OpenTelemetryLoggerProvider.ForceFlush(int timeoutMilliseconds = -1) -> bool
OpenTelemetry.Logs.OpenTelemetryLoggerProvider.OpenTelemetryLoggerProvider() -> void
~OpenTelemetry.Trace.SamplingResult.TraceStateString.get -> string
~OpenTelemetry.Trace.SamplingResult.SamplingResult(OpenTelemetry.Trace.SamplingDecision decision, string traceStateString) -> void
~OpenTelemetry.Trace.SamplingResult.SamplingResult(OpenTelemetry.Trace.SamplingDecision decision, System.Collections.Generic.IEnumerable<System.Collections.Generic.KeyValuePair<string, object>> attributes, string traceStateString) -> void
*REMOVED*static Microsoft.Extensions.Logging.OpenTelemetryLoggingExtensions.AddOpenTelemetry(this Microsoft.Extensions.Logging.ILoggingBuilder! builder, System.Action<OpenTelemetry.Logs.OpenTelemetryLoggerOptions!>? configure = null) -> Microsoft.Extensions.Logging.ILoggingBuilder!
OpenTelemetry.Trace.ExportActivityProcessorOptions
OpenTelemetry.Trace.ExportActivityProcessorOptions.BatchExportProcessorOptions.get -> OpenTelemetry.Trace.BatchExportActivityProcessorOptions!
Expand Down
3 changes: 3 additions & 0 deletions src/OpenTelemetry/.publicApi/net6.0/PublicAPI.Unshipped.txt
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@ OpenTelemetry.Logs.OpenTelemetryLoggerOptionsExtensions
OpenTelemetry.Logs.OpenTelemetryLoggerProvider.AddProcessor(OpenTelemetry.BaseProcessor<OpenTelemetry.Logs.LogRecord!>! processor) -> OpenTelemetry.Logs.OpenTelemetryLoggerProvider!
OpenTelemetry.Logs.OpenTelemetryLoggerProvider.ForceFlush(int timeoutMilliseconds = -1) -> bool
OpenTelemetry.Logs.OpenTelemetryLoggerProvider.OpenTelemetryLoggerProvider() -> void
~OpenTelemetry.Trace.SamplingResult.TraceStateString.get -> string
~OpenTelemetry.Trace.SamplingResult.SamplingResult(OpenTelemetry.Trace.SamplingDecision decision, string traceStateString) -> void
~OpenTelemetry.Trace.SamplingResult.SamplingResult(OpenTelemetry.Trace.SamplingDecision decision, System.Collections.Generic.IEnumerable<System.Collections.Generic.KeyValuePair<string, object>> attributes, string traceStateString) -> void
*REMOVED*static Microsoft.Extensions.Logging.OpenTelemetryLoggingExtensions.AddOpenTelemetry(this Microsoft.Extensions.Logging.ILoggingBuilder! builder, System.Action<OpenTelemetry.Logs.OpenTelemetryLoggerOptions!>? configure = null) -> Microsoft.Extensions.Logging.ILoggingBuilder!
OpenTelemetry.Trace.ExportActivityProcessorOptions
OpenTelemetry.Trace.ExportActivityProcessorOptions.BatchExportProcessorOptions.get -> OpenTelemetry.Trace.BatchExportActivityProcessorOptions!
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@ OpenTelemetry.Logs.OpenTelemetryLoggerProvider.AddProcessor(OpenTelemetry.BasePr
OpenTelemetry.Logs.OpenTelemetryLoggerProvider.ForceFlush(int timeoutMilliseconds = -1) -> bool
OpenTelemetry.Logs.OpenTelemetryLoggerProvider.OpenTelemetryLoggerProvider() -> void
OpenTelemetry.Logs.OpenTelemetryLoggerOptions.ConfigureResource(System.Action<OpenTelemetry.Resources.ResourceBuilder!>! configure) -> OpenTelemetry.Logs.OpenTelemetryLoggerOptions!
~OpenTelemetry.Trace.SamplingResult.TraceStateString.get -> string
~OpenTelemetry.Trace.SamplingResult.SamplingResult(OpenTelemetry.Trace.SamplingDecision decision, string traceStateString) -> void
~OpenTelemetry.Trace.SamplingResult.SamplingResult(OpenTelemetry.Trace.SamplingDecision decision, System.Collections.Generic.IEnumerable<System.Collections.Generic.KeyValuePair<string, object>> attributes, string traceStateString) -> void
*REMOVED*static Microsoft.Extensions.Logging.OpenTelemetryLoggingExtensions.AddOpenTelemetry(this Microsoft.Extensions.Logging.ILoggingBuilder! builder, System.Action<OpenTelemetry.Logs.OpenTelemetryLoggerOptions!>? configure = null) -> Microsoft.Extensions.Logging.ILoggingBuilder!
OpenTelemetry.Trace.ExportActivityProcessorOptions
OpenTelemetry.Trace.ExportActivityProcessorOptions.BatchExportProcessorOptions.get -> OpenTelemetry.Trace.BatchExportActivityProcessorOptions!
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@ OpenTelemetry.Logs.OpenTelemetryLoggerOptionsExtensions
OpenTelemetry.Logs.OpenTelemetryLoggerProvider.AddProcessor(OpenTelemetry.BaseProcessor<OpenTelemetry.Logs.LogRecord!>! processor) -> OpenTelemetry.Logs.OpenTelemetryLoggerProvider!
OpenTelemetry.Logs.OpenTelemetryLoggerProvider.ForceFlush(int timeoutMilliseconds = -1) -> bool
OpenTelemetry.Logs.OpenTelemetryLoggerProvider.OpenTelemetryLoggerProvider() -> void
~OpenTelemetry.Trace.SamplingResult.TraceStateString.get -> string
~OpenTelemetry.Trace.SamplingResult.SamplingResult(OpenTelemetry.Trace.SamplingDecision decision, string traceStateString) -> void
~OpenTelemetry.Trace.SamplingResult.SamplingResult(OpenTelemetry.Trace.SamplingDecision decision, System.Collections.Generic.IEnumerable<System.Collections.Generic.KeyValuePair<string, object>> attributes, string traceStateString) -> void
*REMOVED*static Microsoft.Extensions.Logging.OpenTelemetryLoggingExtensions.AddOpenTelemetry(this Microsoft.Extensions.Logging.ILoggingBuilder! builder, System.Action<OpenTelemetry.Logs.OpenTelemetryLoggerOptions!>? configure = null) -> Microsoft.Extensions.Logging.ILoggingBuilder!
OpenTelemetry.Trace.ExportActivityProcessorOptions
OpenTelemetry.Trace.ExportActivityProcessorOptions.BatchExportProcessorOptions.get -> OpenTelemetry.Trace.BatchExportActivityProcessorOptions!
Expand Down
46 changes: 43 additions & 3 deletions src/OpenTelemetry/Trace/SamplingResult.cs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
namespace OpenTelemetry.Trace
{
/// <summary>
/// Sampling decision.
/// Sampling result.
/// </summary>
public readonly struct SamplingResult : System.IEquatable<SamplingResult>
{
Expand All @@ -32,6 +32,7 @@ public SamplingResult(SamplingDecision decision)
{
this.Decision = decision;
this.Attributes = Enumerable.Empty<KeyValuePair<string, object>>();
this.TraceStateString = null;
}

/// <summary>
Expand All @@ -42,6 +43,7 @@ public SamplingResult(bool isSampled)
{
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we not need an overload for this one?

Copy link
Member Author

Choose a reason for hiding this comment

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

its optional. if there is a need, we can always add more overloads in future.

this.Decision = isSampled ? SamplingDecision.RecordAndSample : SamplingDecision.Drop;
this.Attributes = Enumerable.Empty<KeyValuePair<string, object>>();
this.TraceStateString = null;
}

/// <summary>
Expand All @@ -58,6 +60,38 @@ public SamplingResult(SamplingDecision decision, IEnumerable<KeyValuePair<string
// Current implementation has no means to ensure the collection will not be modified by the caller.
// If this behavior will be abused we must switch to cloning of the collection.
this.Attributes = attributes;
this.TraceStateString = null;
}

/// <summary>
/// Initializes a new instance of the <see cref="SamplingResult"/> struct.
/// </summary>
/// <param name="decision">indicates whether an activity object is recorded and sampled.</param>
/// <param name="traceStateString">traceStateString associated with the created Activity.</param>
public SamplingResult(SamplingDecision decision, string traceStateString)
{
this.Decision = decision;
this.Attributes = Enumerable.Empty<KeyValuePair<string, object>>();
this.TraceStateString = traceStateString;
}

/// <summary>
/// Initializes a new instance of the <see cref="SamplingResult"/> struct.
/// </summary>
/// <param name="decision">indicates whether an activity object is recorded and sampled.</param>
/// <param name="attributes">Attributes associated with the sampling decision. Attributes list passed to
cijothomas marked this conversation as resolved.
Show resolved Hide resolved
/// this method must be immutable. Mutations of the collection and/or attribute values may lead to unexpected behavior.</param>
/// <param name="traceStateString">traceStateString associated with the created Activity.</param>
public SamplingResult(SamplingDecision decision, IEnumerable<KeyValuePair<string, object>> attributes, string traceStateString)
{
this.Decision = decision;

// Note: Decision object takes ownership of the collection.
// Current implementation has no means to ensure the collection will not be modified by the caller.
// If this behavior will be abused we must switch to cloning of the collection.
this.Attributes = attributes;

this.TraceStateString = traceStateString;
}

/// <summary>
Expand All @@ -70,6 +104,11 @@ public SamplingResult(SamplingDecision decision, IEnumerable<KeyValuePair<string
/// </summary>
public IEnumerable<KeyValuePair<string, object>> Attributes { get; }

/// <summary>
/// Gets the Tracestate.
cijothomas marked this conversation as resolved.
Show resolved Hide resolved
/// </summary>
public string TraceStateString { get; }

/// <summary>
/// Compare two <see cref="SamplingResult"/> for equality.
/// </summary>
Expand All @@ -93,7 +132,7 @@ public override bool Equals(object obj)
}

var that = (SamplingResult)obj;
return this.Decision == that.Decision && this.Attributes == that.Attributes;
return this.Decision == that.Decision && this.Attributes == that.Attributes && this.TraceStateString == that.TraceStateString;
}

/// <inheritdoc/>
Expand All @@ -102,13 +141,14 @@ public override int GetHashCode()
var result = 1;
result = (31 * result) + this.Decision.GetHashCode();
result = (31 * result) + this.Attributes.GetHashCode();
result = this.TraceStateString == null ? result : (result * 31) + this.TraceStateString.GetHashCode();
return result;
}

/// <inheritdoc/>
public bool Equals(SamplingResult other)
{
return this.Decision == other.Decision && this.Attributes.SequenceEqual(other.Attributes);
return this.Decision == other.Decision && this.Attributes.SequenceEqual(other.Attributes) && this.TraceStateString == other.TraceStateString;
}
}
}
32 changes: 27 additions & 5 deletions src/OpenTelemetry/Trace/TracerProviderSdk.cs
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@ internal TracerProviderSdk(
{
// This delegate informs ActivitySource about sampling decision when the parent context is an ActivityContext.
listener.Sample = (ref ActivityCreationOptions<ActivityContext> options) =>
!Sdk.SuppressInstrumentation ? ComputeActivitySamplingResult(options, this.sampler) : ActivitySamplingResult.None;
!Sdk.SuppressInstrumentation ? ComputeActivitySamplingResult(ref options, this.sampler) : ActivitySamplingResult.None;
this.getRequestedDataAction = this.RunGetRequestedDataOtherSampler;
}

Expand Down Expand Up @@ -380,7 +380,7 @@ protected override void Dispose(bool disposing)
}

private static ActivitySamplingResult ComputeActivitySamplingResult(
in ActivityCreationOptions<ActivityContext> options,
ref ActivityCreationOptions<ActivityContext> options,
Sampler sampler)
{
var samplingParameters = new SamplingParameters(
Expand All @@ -391,9 +391,9 @@ private static ActivitySamplingResult ComputeActivitySamplingResult(
options.Tags,
options.Links);

var shouldSample = sampler.ShouldSample(samplingParameters);
var samplingResult = sampler.ShouldSample(samplingParameters);

var activitySamplingResult = shouldSample.Decision switch
var activitySamplingResult = samplingResult.Decision switch
{
SamplingDecision.RecordAndSample => ActivitySamplingResult.AllDataAndRecorded,
SamplingDecision.RecordOnly => ActivitySamplingResult.AllData,
Expand All @@ -402,11 +402,28 @@ private static ActivitySamplingResult ComputeActivitySamplingResult(

if (activitySamplingResult != ActivitySamplingResult.PropagationData)
{
foreach (var att in shouldSample.Attributes)
foreach (var att in samplingResult.Attributes)
{
options.SamplingTags.Add(att.Key, att.Value);
}

// https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/sdk.md#sampler
// Spec requires clearing Tracestate if empty Tracestate is returned.
// Since .NET did not have this capability, it'll break
// existing samplers if we did that. So the following is
// adopted to remain spec-compliant and backward compat.
// The behavior is:
// if sampler returns null, its treated as if it has no intend
// to change Tracestate. Existing SamplingResult ctors will put null as default TraceStateString,
// so all existing samplers will get this behavior.
// if sampler returns non-null, then it'll be used as the
// new value for Tracestate
// A sampler can return string.Empty if it intends to clear the state.
if (samplingResult.TraceStateString != null)
{
options = options with { TraceState = samplingResult.TraceStateString };
}

return activitySamplingResult;
}

Expand Down Expand Up @@ -493,6 +510,11 @@ private void RunGetRequestedDataOtherSampler(Activity activity)
{
activity.SetTag(att.Key, att.Value);
}

if (samplingResult.TraceStateString != null)
{
activity.TraceStateString = samplingResult.TraceStateString;
}
}
}
}
Expand Down
Loading