diff --git a/src/OpenTelemetry.Api/CHANGELOG.md b/src/OpenTelemetry.Api/CHANGELOG.md index db15d7b076d..1db25f648e8 100644 --- a/src/OpenTelemetry.Api/CHANGELOG.md +++ b/src/OpenTelemetry.Api/CHANGELOG.md @@ -20,6 +20,10 @@ changed to match the to be implemented (`IsInjected`) * Added `CompositePropagator` that accepts a list of `ITextFormat` following [specification](https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/context/api-propagators.md#create-a-composite-propagator) +* Added `StartRootSpan`, `StartActiveSpan` and modified `StartSpan` API. + ([#989](https://github.com/open-telemetry/opentelemetry-dotnet/issues/989)). + Modified StartSpan not to set the created span as Active to match + [spec](https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/trace/api.md#span-creation). ## 0.4.0-beta.2 diff --git a/src/OpenTelemetry.Api/Trace/Tracer.cs b/src/OpenTelemetry.Api/Trace/Tracer.cs index 35f38c40bd0..bcf84592b58 100644 --- a/src/OpenTelemetry.Api/Trace/Tracer.cs +++ b/src/OpenTelemetry.Api/Trace/Tracer.cs @@ -54,62 +54,54 @@ public TelemetrySpan CurrentSpan } /// - /// Starts span. + /// Starts root span. /// /// Span name. /// Kind. + /// Initial attributes for the span. + /// for the span. + /// Start time for the span. /// Span instance. [MethodImpl(MethodImplOptions.AggressiveInlining)] - public TelemetrySpan StartSpan(string name, SpanKind kind = SpanKind.Internal) + public TelemetrySpan StartRootSpan(string name, SpanKind kind = SpanKind.Internal, IEnumerable> attributes = null, IEnumerable links = null, DateTimeOffset startTime = default) { - // TODO: Open Question - should we have both StartSpan and StartActiveSpan? - // Or should we call this method StartActiveSpan? - // This method StartSpan is starting a Span and making it Active. - // OTel spec calls for StartSpan, and StartActiveSpan being separate. - // Need to see if it makes sense for .NET to strictly adhere to it. - // Some discussions in Spec: https://github.com/open-telemetry/opentelemetry-specification/pull/485 - if (!this.ActivitySource.HasListeners()) - { - return TelemetrySpan.NoopInstance; - } - - var activityKind = this.ConvertToActivityKind(kind); - var activity = this.ActivitySource.StartActivity(name, activityKind); - if (activity == null) - { - return TelemetrySpan.NoopInstance; - } - - return new TelemetrySpan(activity); + return this.StartSpanHelper(false, name, kind, default, attributes, links, startTime); } /// - /// Starts span. + /// Starts a span and does not make it as current span. /// /// Span name. /// Kind. - /// Parent for new span. + /// Parent for new span. + /// Initial attributes for the span. + /// for the span. + /// Start time for the span. /// Span instance. [MethodImpl(MethodImplOptions.AggressiveInlining)] - public TelemetrySpan StartSpan(string name, SpanKind kind, in SpanContext parent) + public TelemetrySpan StartSpan(string name, SpanKind kind, in TelemetrySpan parentSpan, IEnumerable> attributes = null, IEnumerable links = null, DateTimeOffset startTime = default) { - if (!this.ActivitySource.HasListeners()) - { - return TelemetrySpan.NoopInstance; - } - - var activityKind = this.ConvertToActivityKind(kind); - var activity = this.ActivitySource.StartActivity(name, activityKind, parent.ActivityContext); - if (activity == null) - { - return TelemetrySpan.NoopInstance; - } + return this.StartSpan(name, kind, parentSpan?.Context ?? default, attributes, links, startTime); + } - return new TelemetrySpan(activity); + /// + /// Starts a span and does not make it as current span. + /// + /// Span name. + /// Kind. + /// Parent Context for new span. + /// Initial attributes for the span. + /// for the span. + /// Start time for the span. + /// Span instance. + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public TelemetrySpan StartSpan(string name, SpanKind kind = SpanKind.Internal, in SpanContext parentContext = default, IEnumerable> attributes = null, IEnumerable links = null, DateTimeOffset startTime = default) + { + return this.StartSpanHelper(false, name, kind, parentContext, attributes, links, startTime); } /// - /// Starts span. + /// Starts a span and make it the current active span. /// /// Span name. /// Kind. @@ -119,13 +111,13 @@ public TelemetrySpan StartSpan(string name, SpanKind kind, in SpanContext parent /// Start time for the span. /// Span instance. [MethodImpl(MethodImplOptions.AggressiveInlining)] - public TelemetrySpan StartSpan(string name, SpanKind kind, in TelemetrySpan parentSpan, IEnumerable> attributes = null, IEnumerable links = null, DateTimeOffset startTime = default) + public TelemetrySpan StartActiveSpan(string name, SpanKind kind, in TelemetrySpan parentSpan, IEnumerable> attributes = null, IEnumerable links = null, DateTimeOffset startTime = default) { - return this.StartSpan(name, kind, parentSpan.Context, attributes, links, startTime); + return this.StartActiveSpan(name, kind, parentSpan?.Context ?? default, attributes, links, startTime); } /// - /// Starts span. + /// Starts a span and make it the current active span. /// /// Span name. /// Kind. @@ -135,7 +127,25 @@ public TelemetrySpan StartSpan(string name, SpanKind kind, in TelemetrySpan pare /// Start time for the span. /// Span instance. [MethodImpl(MethodImplOptions.AggressiveInlining)] - public TelemetrySpan StartSpan(string name, SpanKind kind, in SpanContext parentContext, IEnumerable> attributes = null, IEnumerable links = null, DateTimeOffset startTime = default) + public TelemetrySpan StartActiveSpan(string name, SpanKind kind = SpanKind.Internal, in SpanContext parentContext = default, IEnumerable> attributes = null, IEnumerable links = null, DateTimeOffset startTime = default) + { + return this.StartSpanHelper(true, name, kind, parentContext, attributes, links, startTime); + } + + /// + /// Makes the given span as the current one. + /// + /// The span to be made current. + /// The current span. + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public TelemetrySpan WithSpan(TelemetrySpan span) + { + span.Activate(); + return span; + } + + [MethodImpl(MethodImplOptions.AggressiveInlining)] + private TelemetrySpan StartSpanHelper(bool isActiveSpan, string name, SpanKind kind, in SpanContext parentContext = default, IEnumerable> attributes = null, IEnumerable links = null, DateTimeOffset startTime = default) { if (!this.ActivitySource.HasListeners()) { @@ -143,15 +153,12 @@ public TelemetrySpan StartSpan(string name, SpanKind kind, in SpanContext parent } var activityKind = this.ConvertToActivityKind(kind); + var activityLinks = links?.Select(l => l.ActivityLink); - IList activityLinks = null; - if (links != null && links.Any()) + Activity previousActivity = null; + if (!isActiveSpan) { - activityLinks = new List(); - foreach (var link in links) - { - activityLinks.Add(link.ActivityLink); - } + previousActivity = Activity.Current; } // TODO: @@ -164,39 +171,26 @@ public TelemetrySpan StartSpan(string name, SpanKind kind, in SpanContext parent return TelemetrySpan.NoopInstance; } - return new TelemetrySpan(activity); - } + if (!isActiveSpan) + { + Activity.Current = previousActivity; + } - /// - /// Makes the given span as the current one. - /// - /// The span to be made current. - /// The current span. - [MethodImpl(MethodImplOptions.AggressiveInlining)] - public TelemetrySpan WithSpan(TelemetrySpan span) - { - span.Activate(); - return span; + return new TelemetrySpan(activity); } [MethodImpl(MethodImplOptions.AggressiveInlining)] private ActivityKind ConvertToActivityKind(SpanKind kind) { - switch (kind) + return kind switch { - case SpanKind.Client: - return ActivityKind.Client; - case SpanKind.Consumer: - return ActivityKind.Consumer; - case SpanKind.Internal: - return ActivityKind.Internal; - case SpanKind.Producer: - return ActivityKind.Producer; - case SpanKind.Server: - return ActivityKind.Server; - default: - return ActivityKind.Internal; - } + SpanKind.Client => ActivityKind.Client, + SpanKind.Consumer => ActivityKind.Consumer, + SpanKind.Internal => ActivityKind.Internal, + SpanKind.Producer => ActivityKind.Producer, + SpanKind.Server => ActivityKind.Server, + _ => ActivityKind.Internal, + }; } } } diff --git a/src/OpenTelemetry.Shims.OpenTracing/SpanBuilderShim.cs b/src/OpenTelemetry.Shims.OpenTracing/SpanBuilderShim.cs index 99050c810ba..674d19808a5 100644 --- a/src/OpenTelemetry.Shims.OpenTracing/SpanBuilderShim.cs +++ b/src/OpenTelemetry.Shims.OpenTracing/SpanBuilderShim.cs @@ -16,6 +16,7 @@ using System; using System.Collections.Generic; +using System.Diagnostics; using global::OpenTracing; using OpenTelemetry.Trace; @@ -160,7 +161,7 @@ public ISpan Start() // If specified, this takes precedence. if (this.ignoreActiveSpan) { - span = this.tracer.StartSpan(this.spanName, this.spanKind, default(SpanContext), null, this.links, this.explicitStartTime ?? default); + span = this.tracer.StartRootSpan(this.spanName, this.spanKind, null, this.links, this.explicitStartTime ?? default); } else if (this.parentSpan != null) { @@ -170,19 +171,12 @@ public ISpan Start() { span = this.tracer.StartSpan(this.spanName, this.spanKind, this.parentSpanContext, null, this.links, this.explicitStartTime ?? default); } - else if (this.parentSpan == null && !this.parentSpanContext.IsValid && (this.tracer.CurrentSpan == null || !this.tracer.CurrentSpan.Context.IsValid)) + else if (this.parentSpan == null && !this.parentSpanContext.IsValid && Activity.Current != null && Activity.Current.IdFormat == ActivityIdFormat.W3C) { - // TODO: We need to know if we should inherit an existing Activity-based context or start a new one. - /* - if (System.Diagnostics.Activity.Current != null && System.Diagnostics.Activity.Current.IdFormat == System.Diagnostics.ActivityIdFormat.W3C) + if (this.rootOperationNamesForActivityBasedAutoInstrumentations.Contains(Activity.Current.OperationName)) { - var currentActivity = System.Diagnostics.Activity.Current; - if (this.rootOperationNamesForActivityBasedAutoInstrumentations.Contains(currentActivity.OperationName)) - { - this.tracer.StartSpanFromActivity(this.spanName, currentActivity, this.spanKind, this.links); - span = this.tracer.CurrentSpan; - } - }*/ + span = this.tracer.CurrentSpan; + } } if (span == null) diff --git a/test/OpenTelemetry.Shims.OpenTracing.Tests/SpanBuilderShimTests.cs b/test/OpenTelemetry.Shims.OpenTracing.Tests/SpanBuilderShimTests.cs index 6f781060dc3..7a97e12b320 100644 --- a/test/OpenTelemetry.Shims.OpenTracing.Tests/SpanBuilderShimTests.cs +++ b/test/OpenTelemetry.Shims.OpenTracing.Tests/SpanBuilderShimTests.cs @@ -121,19 +121,24 @@ public void AsChildOf_WithSpan() [Fact] public void Start_ActivityOperationRootSpanChecks() { + // Create an activity + var activity = new Activity("foo") + .SetIdFormat(System.Diagnostics.ActivityIdFormat.W3C) + .Start(); + // matching root operation name var tracer = TracerProvider.Default.GetTracer(TracerName); var shim = new SpanBuilderShim(tracer, "foo", new List { "foo" }); - var spanShim = (SpanShim)shim.Start(); + var spanShim1 = (SpanShim)shim.Start(); - Assert.Equal("foo", spanShim.Span.Activity.OperationName); + Assert.Equal("foo", spanShim1.Span.Activity.OperationName); // mis-matched root operation name shim = new SpanBuilderShim(tracer, "foo", new List { "bar" }); - spanShim = (SpanShim)shim.Start(); + var spanShim2 = (SpanShim)shim.Start(); - Assert.Equal("foo", spanShim.Span.Activity.OperationName); - Assert.Equal("foo", spanShim.Span.Activity.Parent.OperationName); + Assert.Equal("foo", spanShim2.Span.Activity.OperationName); + Assert.Equal(spanShim1.Context.TraceId, spanShim2.Context.TraceId); } [Fact] diff --git a/test/OpenTelemetry.Tests/Implementation/Trace/TracerTest.cs b/test/OpenTelemetry.Tests/Implementation/Trace/TracerTest.cs index a9e8cec5a19..5ff799b3eae 100644 --- a/test/OpenTelemetry.Tests/Implementation/Trace/TracerTest.cs +++ b/test/OpenTelemetry.Tests/Implementation/Trace/TracerTest.cs @@ -13,9 +13,10 @@ // See the License for the specific language governing permissions and // limitations under the License. // + using System; -using System.Collections.Generic; using System.Diagnostics; +using OpenTelemetry.Trace.Samplers; using Xunit; namespace OpenTelemetry.Trace.Test @@ -55,6 +56,183 @@ public void TracerStartReturnsNoOpSpanWhenNoSdk() Assert.False(span.IsRecording); } + [Fact] + public void Tracer_StartRootSpan_BadArgs_NullSpanName() + { + using var openTelemetry = Sdk.CreateTracerProvider( + (builder) => builder.AddActivitySource("tracername")); + + var span1 = this.tracer.StartRootSpan(null); + Assert.Null(span1.Activity.DisplayName); + + var span2 = this.tracer.StartRootSpan(null, SpanKind.Client); + Assert.Null(span2.Activity.DisplayName); + + var span3 = this.tracer.StartRootSpan(null, SpanKind.Client, null); + Assert.Null(span3.Activity.DisplayName); + } + + [Fact] + public void Tracer_StartSpan_BadArgs_NullSpanName() + { + using var openTelemetry = Sdk.CreateTracerProvider( + (builder) => builder.AddActivitySource("tracername")); + + var span1 = this.tracer.StartSpan(null); + Assert.Null(span1.Activity.DisplayName); + + var span2 = this.tracer.StartSpan(null, SpanKind.Client); + Assert.Null(span2.Activity.DisplayName); + + var span3 = this.tracer.StartSpan(null, SpanKind.Client, null); + Assert.Null(span3.Activity.DisplayName); + } + + [Fact] + public void Tracer_StartActiveSpan_BadArgs_NullSpanName() + { + using var openTelemetry = Sdk.CreateTracerProvider( + (builder) => builder.AddActivitySource("tracername")); + + var span1 = this.tracer.StartActiveSpan(null); + Assert.Null(span1.Activity.DisplayName); + + var span2 = this.tracer.StartActiveSpan(null, SpanKind.Client); + Assert.Null(span2.Activity.DisplayName); + + var span3 = this.tracer.StartActiveSpan(null, SpanKind.Client, null); + Assert.Null(span3.Activity.DisplayName); + } + + [Fact] + public void Tracer_StartSpan_FromParent_BadArgs_NullSpanName() + { + using var openTelemetry = Sdk.CreateTracerProvider( + (builder) => builder.AddActivitySource("tracername")); + + var span1 = this.tracer.StartSpan(null, SpanKind.Client, TelemetrySpan.NoopInstance); + Assert.Null(span1.Activity.DisplayName); + + var span2 = this.tracer.StartSpan(null, SpanKind.Client, TelemetrySpan.NoopInstance, null); + Assert.Null(span2.Activity.DisplayName); + } + + [Fact] + public void Tracer_StartSpan_FromParentContext_BadArgs_NullSpanName() + { + using var openTelemetry = Sdk.CreateTracerProvider( + (builder) => builder.AddActivitySource("tracername")); + + var blankContext = default(SpanContext); + + var span1 = this.tracer.StartSpan(null, SpanKind.Client, blankContext); + Assert.Null(span1.Activity.DisplayName); + + var span2 = this.tracer.StartSpan(null, SpanKind.Client, blankContext, null); + Assert.Null(span2.Activity.DisplayName); + } + + [Fact] + public void Tracer_StartActiveSpan_FromParent_BadArgs_NullSpanName() + { + using var openTelemetry = Sdk.CreateTracerProvider( + (builder) => builder.AddActivitySource("tracername")); + + var span1 = this.tracer.StartActiveSpan(null, SpanKind.Client, TelemetrySpan.NoopInstance); + Assert.Null(span1.Activity.DisplayName); + + var span2 = this.tracer.StartActiveSpan(null, SpanKind.Client, TelemetrySpan.NoopInstance, null); + Assert.Null(span2.Activity.DisplayName); + } + + [Fact] + public void Tracer_StartActiveSpan_FromParentContext_BadArgs_NullSpanName() + { + using var openTelemetry = Sdk.CreateTracerProvider( + (builder) => builder.AddActivitySource("tracername")); + + var blankContext = default(SpanContext); + + var span1 = this.tracer.StartActiveSpan(null, SpanKind.Client, blankContext); + Assert.Null(span1.Activity.DisplayName); + + var span2 = this.tracer.StartActiveSpan(null, SpanKind.Client, blankContext, null); + Assert.Null(span2.Activity.DisplayName); + } + + [Fact] + public void Tracer_StartActiveSpan_CreatesActiveSpan() + { + using var openTelemetry = Sdk.CreateTracerProvider( + (builder) => builder.AddActivitySource("tracername")); + + var span1 = this.tracer.StartActiveSpan("Test"); + Assert.Equal(span1.Activity.SpanId, this.tracer.CurrentSpan.Context.SpanId); + + var span2 = this.tracer.StartActiveSpan("Test", SpanKind.Client); + Assert.Equal(span2.Activity.SpanId, this.tracer.CurrentSpan.Context.SpanId); + + var span = this.tracer.StartSpan("foo"); + this.tracer.WithSpan(span); + + var span3 = this.tracer.StartActiveSpan("Test", SpanKind.Client, span); + Assert.Equal(span3.Activity.SpanId, this.tracer.CurrentSpan.Context.SpanId); + + var spanContext = new SpanContext(ActivityTraceId.CreateRandom(), ActivitySpanId.CreateRandom(), ActivityTraceFlags.Recorded); + var span4 = this.tracer.StartActiveSpan("Test", SpanKind.Client, spanContext); + Assert.Equal(span4.Activity.SpanId, this.tracer.CurrentSpan.Context.SpanId); + } + + [Fact] + public void GetCurrentSpanBlank() + { + using var openTelemetry = Sdk.CreateTracerProvider( + (builder) => builder.AddActivitySource("tracername")); + Assert.False(this.tracer.CurrentSpan.Context.IsValid); + } + + [Fact] + public void GetCurrentSpanBlankWontThrowOnEnd() + { + using var openTelemetry = Sdk.CreateTracerProvider( + (builder) => builder.AddActivitySource("tracername")); + var current = this.tracer.CurrentSpan; + current.End(); + } + + [Fact] + public void GetCurrentSpan() + { + using var openTelemetry = Sdk.CreateTracerProvider( + (builder) => builder.AddActivitySource("tracername")); + + var span = this.tracer.StartSpan("foo"); + this.tracer.WithSpan(span); + + Assert.Equal(span.Context.SpanId, this.tracer.CurrentSpan.Context.SpanId); + Assert.True(this.tracer.CurrentSpan.Context.IsValid); + } + + [Fact] + public void CreateSpan_Sampled() + { + using var openTelemetry = Sdk.CreateTracerProvider( + (builder) => builder.AddActivitySource("tracername")); + var span = this.tracer.StartSpan("foo"); + Assert.True(span.IsRecording); + } + + [Fact] + public void CreateSpan_NotSampled() + { + using var openTelemetry = Sdk.CreateTracerProvider(b => b + .AddActivitySource("tracername") + .SetSampler(new AlwaysOffSampler())); + + var span = this.tracer.StartSpan("foo"); + Assert.False(span.IsRecording); + } + public void Dispose() { Activity.Current = null;