open-telemetry
/
opentelemetry-dotnet
mirror of https://github.com/open-telemetry/opentelemetry-dotnet.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Default Sampler update + ParentOrElse fix (#1013) 

* Updated default sampler to match the spec. Fixed broken ParentOrElseSampler.

* Fixed http-in instrumentation creating Activity objects with invalid parents.

Co-authored-by: Cijo Thomas <cithomas@microsoft.com>
This commit is contained in:
Mikel Blanchard 2020-08-05 15:35:11 -07:00 committed by GitHub
parent 1758e32222
commit 73bff75ef6
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 51 additions and 43 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

32
src/OpenTelemetry.Instrumentation.AspNet/Implementation/HttpInListener.cs
View File

@ -64,23 +64,25 @@ namespace OpenTelemetry.Instrumentation.AspNet.Implementation
// This requires to ignore the current activity and create a new one
// using the context extracted using the format TextFormat supports.
var ctx = this.options.TextFormat.Extract(default, request, HttpRequestHeaderValuesGetter);
if (ctx != default)
{
// Create a new activity with its parent set from the extracted context.
// This makes the new activity as a "sibling" of the activity created by
// Asp.Net.
Activity newOne = new Activity(ActivityNameByHttpInListener);
newOne.SetParentId(ctx.TraceId, ctx.SpanId, ctx.TraceFlags);
newOne.TraceStateString = ctx.TraceState;
// Create a new activity with its parent set from the extracted context.
// This makes the new activity as a "sibling" of the activity created by
// Asp.Net.
Activity newOne = new Activity(ActivityNameByHttpInListener);
newOne.SetParentId(ctx.TraceId, ctx.SpanId, ctx.TraceFlags);
newOne.TraceStateString = ctx.TraceState;
// Starting the new activity make it the Activity.Current one.
newOne.Start();
// Starting the new activity make it the Activity.Current one.
newOne.Start();
// Both new activity and old one store the other activity
// inside them. This is required in the Stop step to
// correctly stop and restore Activity.Current.
newOne.SetCustomProperty("ActivityByAspNet", activity);
activity.SetCustomProperty("ActivityByHttpInListener", newOne);
activity = newOne;
// Both new activity and old one store the other activity
// inside them. This is required in the Stop step to
// correctly stop and restore Activity.Current.
newOne.SetCustomProperty("ActivityByAspNet", activity);
activity.SetCustomProperty("ActivityByHttpInListener", newOne);
activity = newOne;
}
}
// see the spec https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/data-semantic-conventions.md

22
src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInListener.cs
View File

@ -72,17 +72,19 @@ namespace OpenTelemetry.Instrumentation.AspNetCore.Implementation
// using the format TextFormat supports.
var ctx = this.options.TextFormat.Extract(default, request, HttpRequestHeaderValuesGetter);
if (ctx != default)
{
// Create a new activity with its parent set from the extracted context.
// This makes the new activity as a "sibling" of the activity created by
// Asp.Net Core.
Activity newOne = new Activity(ActivityNameByHttpInListener);
newOne.SetParentId(ctx.TraceId, ctx.SpanId, ctx.TraceFlags);
newOne.TraceStateString = ctx.TraceState;
// Create a new activity with its parent set from the extracted context.
// This makes the new activity as a "sibling" of the activity created by
// Asp.Net Core.
Activity newOne = new Activity(ActivityNameByHttpInListener);
newOne.SetParentId(ctx.TraceId, ctx.SpanId, ctx.TraceFlags);
newOne.TraceStateString = ctx.TraceState;
// Starting the new activity make it the Activity.Current one.
newOne.Start();
activity = newOne;
// Starting the new activity make it the Activity.Current one.
newOne.Start();
activity = newOne;
}
}
activity.SetKind(ActivityKind.Server);

2
src/OpenTelemetry/CHANGELOG.md
View File

@ -12,6 +12,8 @@
disposes the containing exporter.
* `BroadcastActivityProcessor`is disposable and it disposes the processors.
* Samplers now get the actual TraceId of the Activity to be created.
* Default Sampler changed from AlwaysOn to ParentOrElse(AlwaysOn) to match the
spec.
## 0.3.0-beta

7
src/OpenTelemetry/Sdk.cs
View File

@ -64,7 +64,7 @@ namespace OpenTelemetry
meterRegistry,
metricProcessor,
metricExporter,
meterBuilder.MetricPushInterval == default(TimeSpan) ? DefaultPushInterval : meterBuilder.MetricPushInterval,
meterBuilder.MetricPushInterval == default ? DefaultPushInterval : meterBuilder.MetricPushInterval,
cancellationTokenSource);
var meterProviderSdk = new MeterProviderSdk(metricProcessor, meterRegistry, controller, cancellationTokenSource);
@ -85,7 +85,7 @@ namespace OpenTelemetry
configureTracerProviderBuilder?.Invoke(tracerProviderBuilder);
var tracerProviderSdk = new TracerProviderSdk();
Sampler sampler = tracerProviderBuilder.Sampler ?? new AlwaysOnSampler();
Sampler sampler = tracerProviderBuilder.Sampler ?? new ParentOrElseSampler(new AlwaysOnSampler());
ActivityProcessor activityProcessor;
if (tracerProviderBuilder.ProcessingPipelines == null || !tracerProviderBuilder.ProcessingPipelines.Any())
@ -165,7 +165,8 @@ namespace OpenTelemetry
in ActivityCreationOptions<ActivityContext> options,
Sampler sampler)
{
var isRootSpan = options.Parent.SpanId == default;
var isRootSpan = /*TODO: Put back once AutoGenerateRootContextTraceId is removed.
options.Parent.TraceId == default ||*/ options.Parent.SpanId == default;
// As we set ActivityListener.AutoGenerateRootContextTraceId = true,
// Parent.TraceId will always be the TraceId of the to-be-created Activity,

25
src/OpenTelemetry/Trace/ActivitySourceAdapter.cs
View File

@ -76,23 +76,20 @@ namespace OpenTelemetry.Trace
ActivityContext parentContext;
if (string.IsNullOrEmpty(activity.ParentId))
{
parentContext = default(ActivityContext);
parentContext = default;
}
else if (activity.Parent != null)
{
parentContext = activity.Parent.Context;
}
else
{
if (activity.Parent != null)
{
parentContext = activity.Parent.Context;
}
else
{
parentContext = new ActivityContext(
activity.TraceId,
activity.ParentSpanId,
activity.ActivityTraceFlags,
activity.TraceStateString,
isRemote: true);
}
parentContext = new ActivityContext(
activity.TraceId,
activity.ParentSpanId,
activity.ActivityTraceFlags,
activity.TraceStateString,
isRemote: true);
}
var samplingParameters = new SamplingParameters(

3
src/OpenTelemetry/Trace/Samplers/ParentOrElseSampler.cs
View File

@ -41,7 +41,8 @@ namespace OpenTelemetry.Trace.Samplers
public override SamplingResult ShouldSample(in SamplingParameters samplingParameters)
{
var parentContext = samplingParameters.ParentContext;
if (parentContext == default)
if (/* TODO: TraceId is always provided due to AutoGenerateRootContextTraceId. That is being removed in RC1 and this can be put back.
parentContext.TraceId == default ||*/ parentContext.SpanId == default)
{
// If no parent, use the delegate to determine sampling.
return this.delegateSampler.ShouldSample(samplingParameters);

3
test/OpenTelemetry.Instrumentation.Grpc.Tests/GrpcTests.client.cs
View File

@ -21,6 +21,7 @@ using Grpc.Net.Client;
using Moq;
using OpenTelemetry.Instrumentation.Grpc.Tests.Services;
using OpenTelemetry.Trace;
using OpenTelemetry.Trace.Samplers;
using Xunit;
namespace OpenTelemetry.Instrumentation.Grpc.Tests
@ -44,6 +45,7 @@ namespace OpenTelemetry.Instrumentation.Grpc.Tests
using (Sdk.CreateTracerProvider(
(builder) => builder
.SetSampler(new AlwaysOnSampler())
.AddGrpcClientInstrumentation()
.SetResource(expectedResource)
.AddProcessorPipeline(p => p.AddProcessor(n => spanProcessor.Object))))
@ -95,6 +97,7 @@ namespace OpenTelemetry.Instrumentation.Grpc.Tests
using (Sdk.CreateTracerProvider(
(builder) => builder
.SetSampler(new AlwaysOnSampler())
.AddHttpClientInstrumentation()
.AddGrpcClientInstrumentation()
.AddProcessorPipeline(p => p.AddProcessor(n => spanProcessor.Object))))