diff --git a/src/OpenTelemetry.Instrumentation.AspNet/Implementation/HttpInListener.cs b/src/OpenTelemetry.Instrumentation.AspNet/Implementation/HttpInListener.cs index ac16a2f20ec..90f6dd0dbb7 100644 --- a/src/OpenTelemetry.Instrumentation.AspNet/Implementation/HttpInListener.cs +++ b/src/OpenTelemetry.Instrumentation.AspNet/Implementation/HttpInListener.cs @@ -64,23 +64,25 @@ public override void OnStartActivity(Activity activity, object payload) // 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); - - // 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(); - - // 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; + 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; + + // 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; + } } // see the spec https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/data-semantic-conventions.md diff --git a/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInListener.cs b/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInListener.cs index 38ed2ff82c4..9b37bbc05be 100644 --- a/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInListener.cs +++ b/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInListener.cs @@ -72,17 +72,19 @@ public override void OnStartActivity(Activity activity, object payload) // using the format TextFormat supports. var ctx = this.options.TextFormat.Extract(default, request, HttpRequestHeaderValuesGetter); - - // 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; + 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; + + // Starting the new activity make it the Activity.Current one. + newOne.Start(); + activity = newOne; + } } activity.SetKind(ActivityKind.Server); diff --git a/src/OpenTelemetry/CHANGELOG.md b/src/OpenTelemetry/CHANGELOG.md index 6b820f988e0..f5cc05ea1d0 100644 --- a/src/OpenTelemetry/CHANGELOG.md +++ b/src/OpenTelemetry/CHANGELOG.md @@ -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 diff --git a/src/OpenTelemetry/Sdk.cs b/src/OpenTelemetry/Sdk.cs index fda490cd529..610f963d1c7 100644 --- a/src/OpenTelemetry/Sdk.cs +++ b/src/OpenTelemetry/Sdk.cs @@ -64,7 +64,7 @@ public static MeterProvider CreateMeterProvider(Action configure) 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 @@ public static TracerProvider CreateTracerProvider(Action 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 @@ internal static ActivityDataRequest ComputeActivityDataRequest( in ActivityCreationOptions 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, diff --git a/src/OpenTelemetry/Trace/ActivitySourceAdapter.cs b/src/OpenTelemetry/Trace/ActivitySourceAdapter.cs index fc3ca46e45a..cc3eb187463 100644 --- a/src/OpenTelemetry/Trace/ActivitySourceAdapter.cs +++ b/src/OpenTelemetry/Trace/ActivitySourceAdapter.cs @@ -76,23 +76,20 @@ private void RunGetRequestedData(Activity activity) 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( diff --git a/src/OpenTelemetry/Trace/Samplers/ParentOrElseSampler.cs b/src/OpenTelemetry/Trace/Samplers/ParentOrElseSampler.cs index c01618be413..54a9892344b 100644 --- a/src/OpenTelemetry/Trace/Samplers/ParentOrElseSampler.cs +++ b/src/OpenTelemetry/Trace/Samplers/ParentOrElseSampler.cs @@ -41,7 +41,8 @@ public ParentOrElseSampler(Sampler delegateSampler) 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); diff --git a/test/OpenTelemetry.Instrumentation.Grpc.Tests/GrpcTests.client.cs b/test/OpenTelemetry.Instrumentation.Grpc.Tests/GrpcTests.client.cs index 58eca8970b7..7f72539ad65 100644 --- a/test/OpenTelemetry.Instrumentation.Grpc.Tests/GrpcTests.client.cs +++ b/test/OpenTelemetry.Instrumentation.Grpc.Tests/GrpcTests.client.cs @@ -21,6 +21,7 @@ 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 @@ public void GrpcClientCallsAreCollectedSuccessfully(string baseAddress) using (Sdk.CreateTracerProvider( (builder) => builder + .SetSampler(new AlwaysOnSampler()) .AddGrpcClientInstrumentation() .SetResource(expectedResource) .AddProcessorPipeline(p => p.AddProcessor(n => spanProcessor.Object)))) @@ -95,6 +97,7 @@ public void GrpcAndHttpClientInstrumentationIsInvoked() using (Sdk.CreateTracerProvider( (builder) => builder + .SetSampler(new AlwaysOnSampler()) .AddHttpClientInstrumentation() .AddGrpcClientInstrumentation() .AddProcessorPipeline(p => p.AddProcessor(n => spanProcessor.Object))))