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

Update diagnosticsource to rc1 #1203

Merged
merged 10 commits into from
Sep 1, 2020
1 change: 1 addition & 0 deletions NuGet.config
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
<!--
<add key="Dotnet5" value="https://pkgs.dev.azure.com/dnceng/public/_packaging/dotnet5/nuget/v3/index.json" />
-->
<add key="Dotnet5" value="https://pkgs.dev.azure.com/dnceng/public/_packaging/dotnet5/nuget/v3/index.json" />
Copy link
Member

Choose a reason for hiding this comment

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

nit: Looks like the same entry is right above the new one (commented out).

</packageSources>
<disabledPackageSources />
</configuration>
2 changes: 1 addition & 1 deletion build/Common.props
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@
<StackExchangeRedisPkgVer>[2.1.58,3.0)</StackExchangeRedisPkgVer>
<StyleCopAnalyzersPkgVer>[1.1.118,2.0)</StyleCopAnalyzersPkgVer>
<SystemCollectionsImmutablePkgVer>[1.4.0,2.0)</SystemCollectionsImmutablePkgVer>
<SystemDiagnosticSourcePkgVer>[5.0.0-preview.8.20407.11]</SystemDiagnosticSourcePkgVer>
<SystemDiagnosticSourcePkgVer>[5.0.0-rc.1.20428.3]</SystemDiagnosticSourcePkgVer>
<SystemReflectionEmitLightweightPkgVer>[4.7.0,5.0)</SystemReflectionEmitLightweightPkgVer>
<SystemTextJsonPkgVer>[4.7.0,5.0)</SystemTextJsonPkgVer>
<SystemThreadingTasksExtensionsPkgVer>[4.5.3,5.0)</SystemThreadingTasksExtensionsPkgVer>
Expand Down
2 changes: 1 addition & 1 deletion examples/AspNet/Examples.AspNet.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@
<PackageReference Include="Microsoft.AspNet.Mvc" Version="5.2.7" />
<PackageReference Include="Microsoft.AspNet.WebPages" Version="3.2.7" />
<PackageReference Include="System.Runtime.CompilerServices.Unsafe">
<Version>5.0.0-preview.8.20407.11</Version>
<Version>5.0.0-rc.1.20428.3</Version>
</PackageReference>
</ItemGroup>
<ItemGroup>
Expand Down
2 changes: 2 additions & 0 deletions src/OpenTelemetry.Api/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

## Unreleased

* Updated System.Diagnostics.DiagnosticSource to version 5.0.0-rc.1.20428.3

## 0.5.0-beta.2

Released 2020-08-28
Expand Down
3 changes: 3 additions & 0 deletions src/OpenTelemetry/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@

## Unreleased

* Fixes [953](https://github.com/open-telemetry/opentelemetry-dotnet/issues/953)
* Changes arising from DiagnosticSource changes ([#1203](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1203/files))

## 0.5.0-beta.2

Released 2020-08-28
Expand Down
57 changes: 24 additions & 33 deletions src/OpenTelemetry/Trace/TracerProviderSdk.cs
Original file line number Diff line number Diff line change
Expand Up @@ -93,30 +93,23 @@ static TracerProviderSdk()
this.processor?.OnEnd(activity);
}
},

// Setting this to true means TraceId will be always
// available in sampling callbacks and will be the actual
// traceid used, if activity ends up getting created.
AutoGenerateRootContextTraceId = true,
};

if (sampler is AlwaysOnSampler)
{
listener.GetRequestedDataUsingContext = (ref ActivityCreationOptions<ActivityContext> options) =>
!Sdk.SuppressInstrumentation ? ActivityDataRequest.AllDataAndRecorded : ActivityDataRequest.None;
listener.Sample = (ref ActivityCreationOptions<ActivityContext> options) =>
!Sdk.SuppressInstrumentation ? ActivitySamplingResult.AllDataAndRecorded : ActivitySamplingResult.None;
}
else if (sampler is AlwaysOffSampler)
{
/*TODO: Change options.Parent.SpanId to options.Parent.TraceId
once AutoGenerateRootContextTraceId is removed.*/
listener.GetRequestedDataUsingContext = (ref ActivityCreationOptions<ActivityContext> options) =>
!Sdk.SuppressInstrumentation ? PropagateOrIgnoreData(options.Parent.SpanId) : ActivityDataRequest.None;
listener.Sample = (ref ActivityCreationOptions<ActivityContext> options) =>
!Sdk.SuppressInstrumentation ? PropagateOrIgnoreData(options.Parent.TraceId) : ActivitySamplingResult.None;
}
else
{
// This delegate informs ActivitySource about sampling decision when the parent context is an ActivityContext.
listener.GetRequestedDataUsingContext = (ref ActivityCreationOptions<ActivityContext> options) =>
!Sdk.SuppressInstrumentation ? ComputeActivityDataRequest(options, sampler) : ActivityDataRequest.None;
listener.Sample = (ref ActivityCreationOptions<ActivityContext> options) =>
!Sdk.SuppressInstrumentation ? ComputeActivitySamplingResult(options, sampler) : ActivitySamplingResult.None;
}

if (sources.Any())
Expand Down Expand Up @@ -219,52 +212,50 @@ protected override void Dispose(bool disposing)
base.Dispose(disposing);
}

private static ActivityDataRequest ComputeActivityDataRequest(
private static ActivitySamplingResult ComputeActivitySamplingResult(
in ActivityCreationOptions<ActivityContext> options,
Sampler sampler)
{
// As we set ActivityListener.AutoGenerateRootContextTraceId = true,
// Parent.TraceId will always be the TraceId of the to-be-created Activity,
// if it get created.
ActivityTraceId traceId = options.Parent.TraceId;

var samplingParameters = new SamplingParameters(
options.Parent,
traceId,
options.TraceId,
options.Name,
options.Kind,
options.Tags,
options.Links);

var shouldSample = sampler.ShouldSample(samplingParameters);

var activityDataRequest = shouldSample.Decision switch
var activitySamplingResult = shouldSample.Decision switch

Choose a reason for hiding this comment

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

would this callback be called for legacy (http-in/out) DiagnosticSource instrumentations?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point!
I missed it. Let me add it to the other refactor on ActivitySourceAdapter (which deals with Activity created using DiagnosticSource methods)
Tracking here #1210

{
SamplingDecision.RecordAndSampled => ActivityDataRequest.AllDataAndRecorded,
SamplingDecision.Record => ActivityDataRequest.AllData,
_ => ActivityDataRequest.PropagationData
SamplingDecision.RecordAndSampled => ActivitySamplingResult.AllDataAndRecorded,
SamplingDecision.Record => ActivitySamplingResult.AllData,
_ => ActivitySamplingResult.PropagationData
};

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

return activitySamplingResult;
}

/*TODO: Change options.Parent.SpanId to options.Parent.TraceId
once AutoGenerateRootContextTraceId is removed.*/
return PropagateOrIgnoreData(options.Parent.SpanId);
return PropagateOrIgnoreData(options.Parent.TraceId);
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
private static ActivityDataRequest PropagateOrIgnoreData(ActivitySpanId spanId)
private static ActivitySamplingResult PropagateOrIgnoreData(ActivityTraceId traceId)
{
var isRootSpan = spanId == default;
var isRootSpan = traceId == default;

// If it is the root span select PropagationData so the trace ID is preserved
// even if no activity of the trace is recorded (sampled per OpenTelemetry parlance).
return isRootSpan
? ActivityDataRequest.PropagationData
: ActivityDataRequest.None;
? ActivitySamplingResult.PropagationData
: ActivitySamplingResult.None;
}
}
}
9 changes: 3 additions & 6 deletions test/Benchmarks/Tracing/TraceBenchmarks.cs
Original file line number Diff line number Diff line change
Expand Up @@ -39,26 +39,23 @@ public TraceBenchmarks()
ActivityStarted = null,
ActivityStopped = null,
ShouldListenTo = (activitySource) => activitySource.Name == this.sourceWithPropagationDataListner.Name,
GetRequestedDataUsingParentId = (ref ActivityCreationOptions<string> options) => ActivityDataRequest.PropagationData,
GetRequestedDataUsingContext = (ref ActivityCreationOptions<ActivityContext> options) => ActivityDataRequest.PropagationData,
Sample = (ref ActivityCreationOptions<ActivityContext> options) => ActivitySamplingResult.PropagationData,
});

ActivitySource.AddActivityListener(new ActivityListener
{
ActivityStarted = null,
ActivityStopped = null,
ShouldListenTo = (activitySource) => activitySource.Name == this.sourceWithAllDataListner.Name,
GetRequestedDataUsingParentId = (ref ActivityCreationOptions<string> options) => ActivityDataRequest.AllData,
GetRequestedDataUsingContext = (ref ActivityCreationOptions<ActivityContext> options) => ActivityDataRequest.AllData,
Sample = (ref ActivityCreationOptions<ActivityContext> options) => ActivitySamplingResult.AllData,
});

ActivitySource.AddActivityListener(new ActivityListener
{
ActivityStarted = null,
ActivityStopped = null,
ShouldListenTo = (activitySource) => activitySource.Name == this.sourceWithAllDataAndRecordedListner.Name,
GetRequestedDataUsingParentId = (ref ActivityCreationOptions<string> options) => ActivityDataRequest.AllDataAndRecorded,
GetRequestedDataUsingContext = (ref ActivityCreationOptions<ActivityContext> options) => ActivityDataRequest.AllDataAndRecorded,
Sample = (ref ActivityCreationOptions<ActivityContext> options) => ActivitySamplingResult.AllDataAndRecorded,
});

Sdk.CreateTracerProviderBuilder()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,7 @@ static JaegerActivityConversionTest()
var listener = new ActivityListener
{
ShouldListenTo = _ => true,
GetRequestedDataUsingParentId = (ref ActivityCreationOptions<string> options) => ActivityDataRequest.AllData,
GetRequestedDataUsingContext = (ref ActivityCreationOptions<ActivityContext> options) => ActivityDataRequest.AllData,
Sample = (ref ActivityCreationOptions<ActivityContext> options) => ActivitySamplingResult.AllData,
};

ActivitySource.AddActivityListener(listener);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,7 @@ static JaegerUdpBatcherTests()
var listener = new ActivityListener
{
ShouldListenTo = _ => true,
GetRequestedDataUsingParentId = (ref ActivityCreationOptions<string> options) => ActivityDataRequest.AllData,
GetRequestedDataUsingContext = (ref ActivityCreationOptions<ActivityContext> options) => ActivityDataRequest.AllData,
Sample = (ref ActivityCreationOptions<ActivityContext> options) => ActivitySamplingResult.AllData,
};

ActivitySource.AddActivityListener(listener);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,7 @@ static OtlpExporterTest()
var listener = new ActivityListener
{
ShouldListenTo = _ => true,
GetRequestedDataUsingParentId = (ref ActivityCreationOptions<string> options) => ActivityDataRequest.AllData,
GetRequestedDataUsingContext = (ref ActivityCreationOptions<ActivityContext> options) => ActivityDataRequest.AllData,
Sample = (ref ActivityCreationOptions<ActivityContext> options) => ActivitySamplingResult.AllData,
};

ActivitySource.AddActivityListener(listener);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,7 @@ static ZPagesExporterTests()
var listener = new ActivityListener
{
ShouldListenTo = _ => true,
GetRequestedDataUsingParentId = (ref ActivityCreationOptions<string> options) => ActivityDataRequest.AllData,
GetRequestedDataUsingContext = (ref ActivityCreationOptions<ActivityContext> options) => ActivityDataRequest.AllData,
Sample = (ref ActivityCreationOptions<ActivityContext> options) => ActivitySamplingResult.AllData,
};

ActivitySource.AddActivityListener(listener);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,7 @@ static ZipkinExporterTests()
var listener = new ActivityListener
{
ShouldListenTo = _ => true,
GetRequestedDataUsingParentId = (ref ActivityCreationOptions<string> options) => ActivityDataRequest.AllData,
GetRequestedDataUsingContext = (ref ActivityCreationOptions<ActivityContext> options) => ActivityDataRequest.AllData,
Sample = (ref ActivityCreationOptions<ActivityContext> options) => ActivitySamplingResult.AllData,
};

ActivitySource.AddActivityListener(listener);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ public async Task TestBasicReceiveAndResponseEvents(string method, string queryS
[InlineData("POST")]
public async Task TestBasicReceiveAndResponseEventsWithoutSampling(string method)
{
using var eventRecords = new ActivitySourceRecorder(activityDataRequest: ActivityDataRequest.None);
using var eventRecords = new ActivitySourceRecorder(activitySamplingResult: ActivitySamplingResult.None);

// Send a random Http request to generate some events
using (var client = new HttpClient())
Expand Down Expand Up @@ -889,14 +889,14 @@ private class ActivitySourceRecorder : IDisposable
private readonly Action<KeyValuePair<string, Activity>> onEvent;
private readonly ActivityListener activityListener;

public ActivitySourceRecorder(Action<KeyValuePair<string, Activity>> onEvent = null, ActivityDataRequest activityDataRequest = ActivityDataRequest.AllDataAndRecorded)
public ActivitySourceRecorder(Action<KeyValuePair<string, Activity>> onEvent = null, ActivitySamplingResult activitySamplingResult = ActivitySamplingResult.AllDataAndRecorded)
{
this.activityListener = new ActivityListener
{
ShouldListenTo = (activitySource) => activitySource.Name == HttpWebRequestActivitySource.ActivitySourceName,
ActivityStarted = this.ActivityStarted,
ActivityStopped = this.ActivityStopped,
GetRequestedDataUsingContext = (ref ActivityCreationOptions<ActivityContext> options) => activityDataRequest,
Sample = (ref ActivityCreationOptions<ActivityContext> options) => activitySamplingResult,
};

ActivitySource.AddActivityListener(this.activityListener);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,7 @@ static SqlClientInstrumentationOptionsTests()
var listener = new ActivityListener
{
ShouldListenTo = _ => true,
GetRequestedDataUsingParentId = (ref ActivityCreationOptions<string> options) => ActivityDataRequest.AllData,
GetRequestedDataUsingContext = (ref ActivityCreationOptions<ActivityContext> options) => ActivityDataRequest.AllData,
Sample = (ref ActivityCreationOptions<ActivityContext> options) => ActivitySamplingResult.AllData,
};

ActivitySource.AddActivityListener(listener);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,7 @@ static ScopeManagerShimTests()
var listener = new ActivityListener
{
ShouldListenTo = _ => true,
GetRequestedDataUsingParentId = (ref ActivityCreationOptions<string> options) => ActivityDataRequest.AllData,
GetRequestedDataUsingContext = (ref ActivityCreationOptions<ActivityContext> options) => ActivityDataRequest.AllData,
Sample = (ref ActivityCreationOptions<ActivityContext> options) => ActivitySamplingResult.AllData,
};

ActivitySource.AddActivityListener(listener);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,7 @@ static SpanBuilderShimTests()
var listener = new ActivityListener
{
ShouldListenTo = _ => true,
GetRequestedDataUsingParentId = (ref ActivityCreationOptions<string> options) => ActivityDataRequest.AllData,
GetRequestedDataUsingContext = (ref ActivityCreationOptions<ActivityContext> options) => ActivityDataRequest.AllData,
Sample = (ref ActivityCreationOptions<ActivityContext> options) => ActivitySamplingResult.AllData,
};

ActivitySource.AddActivityListener(listener);
Expand Down
3 changes: 1 addition & 2 deletions test/OpenTelemetry.Shims.OpenTracing.Tests/SpanShimTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,7 @@ static SpanShimTests()
var listener = new ActivityListener
{
ShouldListenTo = _ => true,
GetRequestedDataUsingParentId = (ref ActivityCreationOptions<string> options) => ActivityDataRequest.AllData,
GetRequestedDataUsingContext = (ref ActivityCreationOptions<ActivityContext> options) => ActivityDataRequest.AllData,
Sample = (ref ActivityCreationOptions<ActivityContext> options) => ActivitySamplingResult.AllData,
};

ActivitySource.AddActivityListener(listener);
Expand Down