From 6f2f7e4c3542ac59330096637131fe92f4b40398 Mon Sep 17 00:00:00 2001 From: Andrea Tosato Date: Mon, 3 May 2021 20:40:24 +0200 Subject: [PATCH 1/4] Fix docs (#2022) --- src/OpenTelemetry.Instrumentation.AspNetCore/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/OpenTelemetry.Instrumentation.AspNetCore/README.md b/src/OpenTelemetry.Instrumentation.AspNetCore/README.md index 60a70cce8d6..d439ec06d16 100644 --- a/src/OpenTelemetry.Instrumentation.AspNetCore/README.md +++ b/src/OpenTelemetry.Instrumentation.AspNetCore/README.md @@ -66,7 +66,7 @@ method of you applications `Startup` class as shown below. // Configure services.Configure(options => { - options.Filter = (req) => + options.Filter = (httpContext) => { // only collect telemetry about HTTP GET requests return httpContext.Request.Method.Equals("GET"); From 073ce6cedceb4e9cd1aa946afe7c5e388011c166 Mon Sep 17 00:00:00 2001 From: Austin Tan Date: Mon, 3 May 2021 14:03:58 -0700 Subject: [PATCH 2/4] Move AddLegacySource to API (#2019) --- .../.publicApi/net452/PublicAPI.Unshipped.txt | 1 + .../netstandard2.0/PublicAPI.Unshipped.txt | 1 + src/OpenTelemetry.Api/CHANGELOG.md | 3 ++ .../Trace/TracerProviderBuilder.cs | 11 ++++++ .../.publicApi/net452/PublicAPI.Unshipped.txt | 2 +- .../.publicApi/net46/PublicAPI.Unshipped.txt | 2 +- .../.publicApi/net461/PublicAPI.Unshipped.txt | 2 +- .../netstandard2.0/PublicAPI.Unshipped.txt | 2 +- src/OpenTelemetry/CHANGELOG.md | 4 +++ .../Trace/TracerProviderBuilderBase.cs | 33 +++++++---------- .../Trace/TracerProviderBuilderExtensions.cs | 19 ---------- .../TracerProviderBuilderExtensionsTest.cs | 35 ------------------- .../Trace/TracerProviderSdkTest.cs | 22 ++++++++++++ 13 files changed, 59 insertions(+), 78 deletions(-) diff --git a/src/OpenTelemetry.Api/.publicApi/net452/PublicAPI.Unshipped.txt b/src/OpenTelemetry.Api/.publicApi/net452/PublicAPI.Unshipped.txt index 877fcaee8d0..eb5e650a70e 100644 --- a/src/OpenTelemetry.Api/.publicApi/net452/PublicAPI.Unshipped.txt +++ b/src/OpenTelemetry.Api/.publicApi/net452/PublicAPI.Unshipped.txt @@ -1 +1,2 @@ +abstract OpenTelemetry.Trace.TracerProviderBuilder.AddLegacySource(string operationName) -> OpenTelemetry.Trace.TracerProviderBuilder OpenTelemetry.Trace.TracerProviderBuilder.TracerProviderBuilder() -> void \ No newline at end of file diff --git a/src/OpenTelemetry.Api/.publicApi/netstandard2.0/PublicAPI.Unshipped.txt b/src/OpenTelemetry.Api/.publicApi/netstandard2.0/PublicAPI.Unshipped.txt index 877fcaee8d0..eb5e650a70e 100644 --- a/src/OpenTelemetry.Api/.publicApi/netstandard2.0/PublicAPI.Unshipped.txt +++ b/src/OpenTelemetry.Api/.publicApi/netstandard2.0/PublicAPI.Unshipped.txt @@ -1 +1,2 @@ +abstract OpenTelemetry.Trace.TracerProviderBuilder.AddLegacySource(string operationName) -> OpenTelemetry.Trace.TracerProviderBuilder OpenTelemetry.Trace.TracerProviderBuilder.TracerProviderBuilder() -> void \ No newline at end of file diff --git a/src/OpenTelemetry.Api/CHANGELOG.md b/src/OpenTelemetry.Api/CHANGELOG.md index f364587626f..24a9ad9c7bb 100644 --- a/src/OpenTelemetry.Api/CHANGELOG.md +++ b/src/OpenTelemetry.Api/CHANGELOG.md @@ -9,6 +9,9 @@ please check the latest changes ## Unreleased +* Adds `AddLegacySource()` to `TracerProviderBuilder` + ([#2019](https://github.com/open-telemetry/opentelemetry-dotnet/pull/2019)) + ## 1.1.0-beta2 Released 2021-Apr-23 diff --git a/src/OpenTelemetry.Api/Trace/TracerProviderBuilder.cs b/src/OpenTelemetry.Api/Trace/TracerProviderBuilder.cs index dc8e0d57efb..e902a38722e 100644 --- a/src/OpenTelemetry.Api/Trace/TracerProviderBuilder.cs +++ b/src/OpenTelemetry.Api/Trace/TracerProviderBuilder.cs @@ -14,6 +14,7 @@ // limitations under the License. // using System; +using System.Diagnostics; namespace OpenTelemetry.Trace { @@ -45,5 +46,15 @@ public abstract TracerProviderBuilder AddInstrumentation( /// Activity source names. /// Returns for chaining. public abstract TracerProviderBuilder AddSource(params string[] names); + + /// + /// Adds a listener for objects created with the given operation name to the . + /// + /// + /// This is provided to capture legacy objects created without using the API. + /// + /// Operation name of the objects to capture. + /// Returns for chaining. + public abstract TracerProviderBuilder AddLegacySource(string operationName); } } diff --git a/src/OpenTelemetry/.publicApi/net452/PublicAPI.Unshipped.txt b/src/OpenTelemetry/.publicApi/net452/PublicAPI.Unshipped.txt index 67d8b08882c..434be6c75d6 100644 --- a/src/OpenTelemetry/.publicApi/net452/PublicAPI.Unshipped.txt +++ b/src/OpenTelemetry/.publicApi/net452/PublicAPI.Unshipped.txt @@ -6,7 +6,7 @@ OpenTelemetry.Trace.TracerProviderBuilderBase.AddInstrumentation(string instrume OpenTelemetry.Trace.TracerProviderBuilderBase.Build() -> OpenTelemetry.Trace.TracerProvider OpenTelemetry.Trace.TracerProviderBuilderBase.TracerProviderBuilderBase() -> void override OpenTelemetry.Trace.TracerProviderBuilderBase.AddInstrumentation(System.Func instrumentationFactory) -> OpenTelemetry.Trace.TracerProviderBuilder +override OpenTelemetry.Trace.TracerProviderBuilderBase.AddLegacySource(string operationName) -> OpenTelemetry.Trace.TracerProviderBuilder override OpenTelemetry.Trace.TracerProviderBuilderBase.AddSource(params string[] names) -> OpenTelemetry.Trace.TracerProviderBuilder -static OpenTelemetry.Trace.TracerProviderBuilderExtensions.AddLegacySource(this OpenTelemetry.Trace.TracerProviderBuilder tracerProviderBuilder, string operationName) -> OpenTelemetry.Trace.TracerProviderBuilder static OpenTelemetry.Trace.TracerProviderBuilderExtensions.SetErrorStatusOnException(this OpenTelemetry.Trace.TracerProviderBuilder tracerProviderBuilder, bool enabled = true) -> OpenTelemetry.Trace.TracerProviderBuilder static OpenTelemetry.Trace.TracerProviderExtensions.ForceFlush(this OpenTelemetry.Trace.TracerProvider provider, int timeoutMilliseconds = -1) -> bool diff --git a/src/OpenTelemetry/.publicApi/net46/PublicAPI.Unshipped.txt b/src/OpenTelemetry/.publicApi/net46/PublicAPI.Unshipped.txt index 67d8b08882c..434be6c75d6 100644 --- a/src/OpenTelemetry/.publicApi/net46/PublicAPI.Unshipped.txt +++ b/src/OpenTelemetry/.publicApi/net46/PublicAPI.Unshipped.txt @@ -6,7 +6,7 @@ OpenTelemetry.Trace.TracerProviderBuilderBase.AddInstrumentation(string instrume OpenTelemetry.Trace.TracerProviderBuilderBase.Build() -> OpenTelemetry.Trace.TracerProvider OpenTelemetry.Trace.TracerProviderBuilderBase.TracerProviderBuilderBase() -> void override OpenTelemetry.Trace.TracerProviderBuilderBase.AddInstrumentation(System.Func instrumentationFactory) -> OpenTelemetry.Trace.TracerProviderBuilder +override OpenTelemetry.Trace.TracerProviderBuilderBase.AddLegacySource(string operationName) -> OpenTelemetry.Trace.TracerProviderBuilder override OpenTelemetry.Trace.TracerProviderBuilderBase.AddSource(params string[] names) -> OpenTelemetry.Trace.TracerProviderBuilder -static OpenTelemetry.Trace.TracerProviderBuilderExtensions.AddLegacySource(this OpenTelemetry.Trace.TracerProviderBuilder tracerProviderBuilder, string operationName) -> OpenTelemetry.Trace.TracerProviderBuilder static OpenTelemetry.Trace.TracerProviderBuilderExtensions.SetErrorStatusOnException(this OpenTelemetry.Trace.TracerProviderBuilder tracerProviderBuilder, bool enabled = true) -> OpenTelemetry.Trace.TracerProviderBuilder static OpenTelemetry.Trace.TracerProviderExtensions.ForceFlush(this OpenTelemetry.Trace.TracerProvider provider, int timeoutMilliseconds = -1) -> bool diff --git a/src/OpenTelemetry/.publicApi/net461/PublicAPI.Unshipped.txt b/src/OpenTelemetry/.publicApi/net461/PublicAPI.Unshipped.txt index aae8523dcf5..848249793be 100644 --- a/src/OpenTelemetry/.publicApi/net461/PublicAPI.Unshipped.txt +++ b/src/OpenTelemetry/.publicApi/net461/PublicAPI.Unshipped.txt @@ -17,8 +17,8 @@ OpenTelemetry.Trace.TracerProviderBuilderBase.AddInstrumentation(string instrume OpenTelemetry.Trace.TracerProviderBuilderBase.Build() -> OpenTelemetry.Trace.TracerProvider OpenTelemetry.Trace.TracerProviderBuilderBase.TracerProviderBuilderBase() -> void override OpenTelemetry.Trace.TracerProviderBuilderBase.AddInstrumentation(System.Func instrumentationFactory) -> OpenTelemetry.Trace.TracerProviderBuilder +override OpenTelemetry.Trace.TracerProviderBuilderBase.AddLegacySource(string operationName) -> OpenTelemetry.Trace.TracerProviderBuilder override OpenTelemetry.Trace.TracerProviderBuilderBase.AddSource(params string[] names) -> OpenTelemetry.Trace.TracerProviderBuilder override OpenTelemetry.Logs.OpenTelemetryLoggerProvider.Dispose(bool disposing) -> void -static OpenTelemetry.Trace.TracerProviderBuilderExtensions.AddLegacySource(this OpenTelemetry.Trace.TracerProviderBuilder tracerProviderBuilder, string operationName) -> OpenTelemetry.Trace.TracerProviderBuilder static OpenTelemetry.Trace.TracerProviderBuilderExtensions.SetErrorStatusOnException(this OpenTelemetry.Trace.TracerProviderBuilder tracerProviderBuilder, bool enabled = true) -> OpenTelemetry.Trace.TracerProviderBuilder static OpenTelemetry.Trace.TracerProviderExtensions.ForceFlush(this OpenTelemetry.Trace.TracerProvider provider, int timeoutMilliseconds = -1) -> bool diff --git a/src/OpenTelemetry/.publicApi/netstandard2.0/PublicAPI.Unshipped.txt b/src/OpenTelemetry/.publicApi/netstandard2.0/PublicAPI.Unshipped.txt index aae8523dcf5..848249793be 100644 --- a/src/OpenTelemetry/.publicApi/netstandard2.0/PublicAPI.Unshipped.txt +++ b/src/OpenTelemetry/.publicApi/netstandard2.0/PublicAPI.Unshipped.txt @@ -17,8 +17,8 @@ OpenTelemetry.Trace.TracerProviderBuilderBase.AddInstrumentation(string instrume OpenTelemetry.Trace.TracerProviderBuilderBase.Build() -> OpenTelemetry.Trace.TracerProvider OpenTelemetry.Trace.TracerProviderBuilderBase.TracerProviderBuilderBase() -> void override OpenTelemetry.Trace.TracerProviderBuilderBase.AddInstrumentation(System.Func instrumentationFactory) -> OpenTelemetry.Trace.TracerProviderBuilder +override OpenTelemetry.Trace.TracerProviderBuilderBase.AddLegacySource(string operationName) -> OpenTelemetry.Trace.TracerProviderBuilder override OpenTelemetry.Trace.TracerProviderBuilderBase.AddSource(params string[] names) -> OpenTelemetry.Trace.TracerProviderBuilder override OpenTelemetry.Logs.OpenTelemetryLoggerProvider.Dispose(bool disposing) -> void -static OpenTelemetry.Trace.TracerProviderBuilderExtensions.AddLegacySource(this OpenTelemetry.Trace.TracerProviderBuilder tracerProviderBuilder, string operationName) -> OpenTelemetry.Trace.TracerProviderBuilder static OpenTelemetry.Trace.TracerProviderBuilderExtensions.SetErrorStatusOnException(this OpenTelemetry.Trace.TracerProviderBuilder tracerProviderBuilder, bool enabled = true) -> OpenTelemetry.Trace.TracerProviderBuilder static OpenTelemetry.Trace.TracerProviderExtensions.ForceFlush(this OpenTelemetry.Trace.TracerProvider provider, int timeoutMilliseconds = -1) -> bool diff --git a/src/OpenTelemetry/CHANGELOG.md b/src/OpenTelemetry/CHANGELOG.md index e24138fd7bd..dd2b7d9ec4a 100644 --- a/src/OpenTelemetry/CHANGELOG.md +++ b/src/OpenTelemetry/CHANGELOG.md @@ -9,6 +9,10 @@ please check the latest changes ## Unreleased +* `AddLegacySource()` moved out of `TracerProviderBuilderExtensions` and into + public API + ([#2019](https://github.com/open-telemetry/opentelemetry-dotnet/pull/2019)) + ## 1.1.0-beta2 Released 2021-Apr-23 diff --git a/src/OpenTelemetry/Trace/TracerProviderBuilderBase.cs b/src/OpenTelemetry/Trace/TracerProviderBuilderBase.cs index f0e6fc3b50f..0b752dd15cd 100644 --- a/src/OpenTelemetry/Trace/TracerProviderBuilderBase.cs +++ b/src/OpenTelemetry/Trace/TracerProviderBuilderBase.cs @@ -79,6 +79,19 @@ public override TracerProviderBuilder AddSource(params string[] names) return this; } + /// + public override TracerProviderBuilder AddLegacySource(string operationName) + { + if (string.IsNullOrWhiteSpace(operationName)) + { + throw new ArgumentException($"{nameof(operationName)} contains null or whitespace string."); + } + + this.legacyActivityOperationNames[operationName] = true; + + return this; + } + /// /// Sets whether the status of /// should be set to Status.Error when it ended abnormally due to an unhandled exception. @@ -160,26 +173,6 @@ internal TracerProviderBuilder AddProcessor(BaseProcessor processor) return this; } - /// - /// Adds a listener for objects created with the given operation name to the . - /// - /// - /// This is provided to capture legacy objects created without using the API. - /// - /// Operation name of the objects to capture. - /// Returns for chaining. - internal TracerProviderBuilder AddLegacySource(string operationName) - { - if (string.IsNullOrWhiteSpace(operationName)) - { - throw new ArgumentException($"{nameof(operationName)} contains null or whitespace string."); - } - - this.legacyActivityOperationNames[operationName] = true; - - return this; - } - /// /// Adds instrumentation to the provider. /// diff --git a/src/OpenTelemetry/Trace/TracerProviderBuilderExtensions.cs b/src/OpenTelemetry/Trace/TracerProviderBuilderExtensions.cs index 9db834cff2e..f6b261e12cf 100644 --- a/src/OpenTelemetry/Trace/TracerProviderBuilderExtensions.cs +++ b/src/OpenTelemetry/Trace/TracerProviderBuilderExtensions.cs @@ -91,25 +91,6 @@ public static TracerProviderBuilder AddProcessor(this TracerProviderBuilder trac return tracerProviderBuilder; } - /// - /// Adds a listener for objects created with the given operation name to the . - /// - /// - /// This is provided to capture legacy objects created without using the API. - /// - /// instance. - /// Operation name of the objects to capture. - /// Returns for chaining. - public static TracerProviderBuilder AddLegacySource(this TracerProviderBuilder tracerProviderBuilder, string operationName) - { - if (tracerProviderBuilder is TracerProviderBuilderBase tracerProviderBuilderBase) - { - tracerProviderBuilderBase.AddLegacySource(operationName); - } - - return tracerProviderBuilder; - } - /// /// Run the given actions to initialize the . /// diff --git a/test/OpenTelemetry.Tests/Trace/TracerProviderBuilderExtensionsTest.cs b/test/OpenTelemetry.Tests/Trace/TracerProviderBuilderExtensionsTest.cs index dc528bf20ee..067dbcc2937 100644 --- a/test/OpenTelemetry.Tests/Trace/TracerProviderBuilderExtensionsTest.cs +++ b/test/OpenTelemetry.Tests/Trace/TracerProviderBuilderExtensionsTest.cs @@ -25,41 +25,6 @@ public class TracerProviderBuilderExtensionsTest { private const string ActivitySourceName = "TracerProviderBuilderExtensionsTest"; - [Fact] - public void AddLegacyOperationName_NullBuilder_Noop() - { - TracerProviderBuilder builder = null; - - // No exception is thrown on executing this line - builder.AddLegacySource("TestOperationName"); - using var provider = builder.Build(); - - var emptyActivitySource = new ActivitySource(string.Empty); - Assert.False(emptyActivitySource.HasListeners()); // Check if AddLegacyOperationName was noop after TracerProviderBuilder.Build - } - - [Theory] - [InlineData(null)] - [InlineData("")] - [InlineData(" ")] - public void AddLegacyOperationName_BadArgs(string operationName) - { - var builder = Sdk.CreateTracerProviderBuilder(); - Assert.Throws(() => builder.AddLegacySource(operationName)); - } - - [Fact] - public void AddLegacyOperationNameAddsActivityListenerForEmptyActivitySource() - { - var emptyActivitySource = new ActivitySource(string.Empty); - var builder = Sdk.CreateTracerProviderBuilder(); - builder.AddLegacySource("TestOperationName"); - - Assert.False(emptyActivitySource.HasListeners()); - using var provider = builder.Build(); - Assert.True(emptyActivitySource.HasListeners()); - } - [Fact] public void SetErrorStatusOnExceptionEnabled() { diff --git a/test/OpenTelemetry.Tests/Trace/TracerProviderSdkTest.cs b/test/OpenTelemetry.Tests/Trace/TracerProviderSdkTest.cs index 89f6f0603cc..a1560738739 100644 --- a/test/OpenTelemetry.Tests/Trace/TracerProviderSdkTest.cs +++ b/test/OpenTelemetry.Tests/Trace/TracerProviderSdkTest.cs @@ -896,6 +896,28 @@ public void TracerProviderSdkBuildsWithDefaultResource() Assert.Contains("unknown_service", (string)resource.Attributes.FirstOrDefault().Value); } + [Theory] + [InlineData(null)] + [InlineData("")] + [InlineData(" ")] + public void AddLegacyOperationName_BadArgs(string operationName) + { + var builder = Sdk.CreateTracerProviderBuilder(); + Assert.Throws(() => builder.AddLegacySource(operationName)); + } + + [Fact] + public void AddLegacyOperationNameAddsActivityListenerForEmptyActivitySource() + { + var emptyActivitySource = new ActivitySource(string.Empty); + var builder = Sdk.CreateTracerProviderBuilder(); + builder.AddLegacySource("TestOperationName"); + + Assert.False(emptyActivitySource.HasListeners()); + using var provider = builder.Build(); + Assert.True(emptyActivitySource.HasListeners()); + } + [Fact] public void TracerProviderSdkBuildsWithSDKResource() { From c28efc3f408cc2cef0df52fc392710d6d42b0b0b Mon Sep 17 00:00:00 2001 From: Utkarsh Umesan Pillai <66651184+utpilla@users.noreply.github.com> Date: Mon, 3 May 2021 21:04:34 -0700 Subject: [PATCH 3/4] Update HttpInListener to add gRPC tags - By creating a new activity with the OperationName used by the framework (#1879) --- .../Internal/ActivityHelperExtensions.cs | 49 +++++++++++ .../CHANGELOG.md | 6 ++ .../Implementation/HttpInListener.cs | 8 +- .../TracerProviderBuilderExtensions.cs | 1 - .../BasicTests.cs | 36 ++------ .../GrpcTests.server.cs | 88 +++++++++++++++++++ .../Trace/ActivityExtensionsTest.cs | 23 +++++ 7 files changed, 180 insertions(+), 31 deletions(-) diff --git a/src/OpenTelemetry.Api/Internal/ActivityHelperExtensions.cs b/src/OpenTelemetry.Api/Internal/ActivityHelperExtensions.cs index cdd5d92cd07..6cc238b8c23 100644 --- a/src/OpenTelemetry.Api/Internal/ActivityHelperExtensions.cs +++ b/src/OpenTelemetry.Api/Internal/ActivityHelperExtensions.cs @@ -77,6 +77,32 @@ public static object GetTagValue(this Activity activity, string tagName) return state.Value; } + /// + /// Checks if the user provided tag name is the first tag of the and retrieves the tag value. + /// + /// Activity instance. + /// Tag name. + /// Tag value. + /// if the first tag of the supplied Activity matches the user provide tag name. + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public static bool TryCheckFirstTag(this Activity activity, string tagName, out object tagValue) + { + Debug.Assert(activity != null, "Activity should not be null"); + + ActivityFirstTagEnumerator state = new ActivityFirstTagEnumerator(tagName); + + ActivityTagsEnumeratorFactory.Enumerate(activity, ref state); + + if (state.Value == null) + { + tagValue = null; + return false; + } + + tagValue = state.Value; + return true; + } + /// /// Enumerates all the key/value pairs on an without performing an allocation. /// @@ -199,6 +225,29 @@ public bool ForEach(KeyValuePair item) } } + private struct ActivityFirstTagEnumerator : IActivityEnumerator> + { + public object Value; + + private readonly string tagName; + + public ActivityFirstTagEnumerator(string tagName) + { + this.tagName = tagName; + this.Value = null; + } + + public bool ForEach(KeyValuePair item) + { + if (item.Key == this.tagName) + { + this.Value = item.Value; + } + + return false; + } + } + private static class ActivityTagsEnumeratorFactory where TState : struct, IActivityEnumerator> { diff --git a/src/OpenTelemetry.Instrumentation.AspNetCore/CHANGELOG.md b/src/OpenTelemetry.Instrumentation.AspNetCore/CHANGELOG.md index 9f062cec753..2e6374632ec 100644 --- a/src/OpenTelemetry.Instrumentation.AspNetCore/CHANGELOG.md +++ b/src/OpenTelemetry.Instrumentation.AspNetCore/CHANGELOG.md @@ -2,6 +2,12 @@ ## Unreleased +* Fixes bug + [#1740](https://github.com/open-telemetry/opentelemetry-dotnet/issues/1740): + Instrumentation.AspNetCore for gRPC services omits ALL rpc.* attributes under + certain conditions + ([#1879](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1879)) + ## 1.0.0-rc4 Released 2021-Apr-23 diff --git a/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInListener.cs b/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInListener.cs index 9cfeffde1a7..b9ff96a4edf 100644 --- a/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInListener.cs +++ b/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInListener.cs @@ -31,7 +31,6 @@ namespace OpenTelemetry.Instrumentation.AspNetCore.Implementation internal class HttpInListener : ListenerHandler { internal const string ActivityOperationName = "Microsoft.AspNetCore.Hosting.HttpRequestIn"; - internal const string ActivityNameByHttpInListener = "ActivityCreatedByHttpInListener"; internal static readonly AssemblyName AssemblyName = typeof(HttpInListener).Assembly.GetName(); internal static readonly string ActivitySourceName = AssemblyName.Name; internal static readonly Version Version = AssemblyName.Version; @@ -92,10 +91,12 @@ public override void OnStartActivity(Activity activity, object payload) // 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); + Activity newOne = new Activity(ActivityOperationName); newOne.SetParentId(ctx.ActivityContext.TraceId, ctx.ActivityContext.SpanId, ctx.ActivityContext.TraceFlags); newOne.TraceStateString = ctx.ActivityContext.TraceState; + newOne.SetTag("IsCreatedByInstrumentation", bool.TrueString); + // Starting the new activity make it the Activity.Current one. newOne.Start(); @@ -214,10 +215,11 @@ public override void OnStopActivity(Activity activity, object payload) } } - if (activity.OperationName.Equals(ActivityNameByHttpInListener, StringComparison.Ordinal)) + if (activity.TryCheckFirstTag("IsCreatedByInstrumentation", out var tagValue) && ReferenceEquals(tagValue, bool.TrueString)) { // If instrumentation started a new Activity, it must // be stopped here. + activity.SetTag("IsCreatedByInstrumentation", null); activity.Stop(); // After the activity.Stop() code, Activity.Current becomes null. diff --git a/src/OpenTelemetry.Instrumentation.AspNetCore/TracerProviderBuilderExtensions.cs b/src/OpenTelemetry.Instrumentation.AspNetCore/TracerProviderBuilderExtensions.cs index faba8be5714..8d48fa00a33 100644 --- a/src/OpenTelemetry.Instrumentation.AspNetCore/TracerProviderBuilderExtensions.cs +++ b/src/OpenTelemetry.Instrumentation.AspNetCore/TracerProviderBuilderExtensions.cs @@ -57,7 +57,6 @@ private static TracerProviderBuilder AddAspNetCoreInstrumentation(TracerProvider var instrumentation = new AspNetCoreInstrumentation(options); builder.AddSource(HttpInListener.ActivitySourceName); builder.AddLegacySource(HttpInListener.ActivityOperationName); // for the activities created by AspNetCore - builder.AddLegacySource(HttpInListener.ActivityNameByHttpInListener); // for the sibling activities created by the instrumentation library return builder.AddInstrumentation(() => instrumentation); } } diff --git a/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/BasicTests.cs b/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/BasicTests.cs index c55f5f02da4..9288cf323f7 100644 --- a/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/BasicTests.cs +++ b/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/BasicTests.cs @@ -177,22 +177,14 @@ public async Task SuccessfulTemplateControllerCallUsesParentContext() // List of invocations // 1. SetParentProvider for TracerProviderSdk // 2. OnStart for the activity created by AspNetCore with the OperationName: Microsoft.AspNetCore.Hosting.HttpRequestIn - // 3. OnStart for the sibling activity created by the instrumentation library with the OperationName: ActivityCreatedByHttpInListener - // 4. OnEnd for the sibling activity created by the instrumentation library with the OperationName: ActivityCreatedByHttpInListener + // 3. OnStart for the sibling activity created by the instrumentation library with the OperationName: Microsoft.AspNetCore.Hosting.HttpRequestIn and the first tag that is added is (IsCreatedByInstrumentation, bool.TrueString) + // 4. OnEnd for the sibling activity created by the instrumentation library with the OperationName: Microsoft.AspNetCore.Hosting.HttpRequestIn and the first tag that is added is (IsCreatedByInstrumentation, bool.TrueString) - // we should only call Processor.OnEnd once for the sibling activity with the OperationName ActivityCreatedByHttpInListener + // we should only call Processor.OnEnd once for the sibling activity Assert.Single(activityProcessor.Invocations, invo => invo.Method.Name == "OnEnd"); var activity = activityProcessor.Invocations.FirstOrDefault(invo => invo.Method.Name == "OnEnd").Arguments[0] as Activity; -#if !NETCOREAPP2_1 - // ASP.NET Core after 2.x is W3C aware and hence Activity created by it - // must be used. Assert.Equal("Microsoft.AspNetCore.Hosting.HttpRequestIn", activity.OperationName); -#else - // ASP.NET Core before 3.x is not W3C aware and hence Activity created by it - // is always ignored and new one is created by the Instrumentation - Assert.Equal("ActivityCreatedByHttpInListener", activity.OperationName); -#endif Assert.Equal("api/Values/{id}", activity.DisplayName); Assert.Equal(expectedTraceId, activity.Context.TraceId); @@ -242,8 +234,8 @@ public async Task CustomPropagator() // List of invocations on the processor // 1. SetParentProvider for TracerProviderSdk // 2. OnStart for the activity created by AspNetCore with the OperationName: Microsoft.AspNetCore.Hosting.HttpRequestIn - // 3. OnStart for the sibling activity created by the instrumentation library with the OperationName: ActivityCreatedByHttpInListener - // 4. OnEnd for the sibling activity created by the instrumentation library with the OperationName: ActivityCreatedByHttpInListener + // 3. OnStart for the sibling activity created by the instrumentation library with the OperationName: Microsoft.AspNetCore.Hosting.HttpRequestIn and the first tag that is added is (IsCreatedByInstrumentation, bool.TrueString) + // 4. OnEnd for the sibling activity created by the instrumentation library with the OperationName: Microsoft.AspNetCore.Hosting.HttpRequestIn and the first tag that is added is (IsCreatedByInstrumentation, bool.TrueString) Assert.Equal(4, activityProcessor.Invocations.Count); var startedActivities = activityProcessor.Invocations.Where(invo => invo.Method.Name == "OnStart"); @@ -252,24 +244,14 @@ public async Task CustomPropagator() Assert.Single(stoppedActivities); // The activity created by the framework and the sibling activity are both sent to Processor.OnStart - Assert.Contains(startedActivities, item => + Assert.Equal(2, startedActivities.Count(item => { var startedActivity = item.Arguments[0] as Activity; return startedActivity.OperationName == HttpInListener.ActivityOperationName; - }); - - Assert.Contains(startedActivities, item => - { - var startedActivity = item.Arguments[0] as Activity; - return startedActivity.OperationName == HttpInListener.ActivityNameByHttpInListener; - }); + })); - // Only the sibling activity is sent to Processor.OnEnd - Assert.Contains(stoppedActivities, item => - { - var stoppedActivity = item.Arguments[0] as Activity; - return stoppedActivity.OperationName == HttpInListener.ActivityNameByHttpInListener; - }); + // we should only call Processor.OnEnd once for the sibling activity + Assert.Single(activityProcessor.Invocations, invo => invo.Method.Name == "OnEnd"); var activity = activityProcessor.Invocations.FirstOrDefault(invo => invo.Method.Name == "OnEnd").Arguments[0] as Activity; Assert.True(activity.Duration != TimeSpan.Zero); diff --git a/test/OpenTelemetry.Instrumentation.Grpc.Tests/GrpcTests.server.cs b/test/OpenTelemetry.Instrumentation.Grpc.Tests/GrpcTests.server.cs index c8f313352ba..bfe23c2c71c 100644 --- a/test/OpenTelemetry.Instrumentation.Grpc.Tests/GrpcTests.server.cs +++ b/test/OpenTelemetry.Instrumentation.Grpc.Tests/GrpcTests.server.cs @@ -20,12 +20,15 @@ using System.Net; using System.Threading; using Greet; +using Grpc.Core; using Grpc.Net.Client; using Moq; +using OpenTelemetry.Context.Propagation; using OpenTelemetry.Instrumentation.Grpc.Tests.Services; using OpenTelemetry.Instrumentation.GrpcNetClient; using OpenTelemetry.Trace; using Xunit; +using Status = OpenTelemetry.Trace.Status; namespace OpenTelemetry.Instrumentation.Grpc.Tests { @@ -109,6 +112,91 @@ public void GrpcAspNetCoreInstrumentationAddsCorrectAttributes(bool? enableGrpcA Assert.StartsWith("grpc-dotnet", activity.GetTagValue(SemanticConventions.AttributeHttpUserAgent) as string); } + [Theory] + [InlineData(null)] + [InlineData(true)] + [InlineData(false)] + public void GrpcAspNetCoreInstrumentationAddsCorrectAttributesWhenItCreatesNewActivity(bool? enableGrpcAspNetCoreSupport) + { + try + { + // B3Propagator along with the headers passed to the client.SayHello ensure that the instrumentation creates a sibling activity + Sdk.SetDefaultTextMapPropagator(new B3Propagator()); + var processor = new Mock>(); + var tracerProviderBuilder = Sdk.CreateTracerProviderBuilder(); + + if (enableGrpcAspNetCoreSupport.HasValue) + { + tracerProviderBuilder.AddAspNetCoreInstrumentation(options => + { + options.EnableGrpcAspNetCoreSupport = enableGrpcAspNetCoreSupport.Value; + }); + } + else + { + tracerProviderBuilder.AddAspNetCoreInstrumentation(); + } + + using var tracerProvider = tracerProviderBuilder + .AddProcessor(processor.Object) + .Build(); + + var clientLoopbackAddresses = new[] { IPAddress.Loopback.ToString(), IPAddress.IPv6Loopback.ToString() }; + var uri = new Uri($"http://localhost:{this.server.Port}"); + + using var channel = GrpcChannel.ForAddress(uri); + var client = new Greeter.GreeterClient(channel); + var headers = new Metadata(); + headers.Add("traceparent", "00-120dc44db5b736468afb112197b0dbd3-5dfbdf27ec544544-01"); + headers.Add("x-b3-traceid", "120dc44db5b736468afb112197b0dbd3"); + headers.Add("x-b3-spanid", "b0966f651b9e0126"); + headers.Add("x-b3-sampled", "1"); + client.SayHello(new HelloRequest(), headers); + + WaitForProcessorInvocations(processor, 4); + + Assert.Equal(4, processor.Invocations.Count); // SetParentProvider, OnStart (framework activity), OnStart (instrumentation activity), OnStop (instrumentation activity) + var activity = GetActivityFromProcessorInvocation(processor, nameof(processor.Object.OnEnd), OperationNameHttpRequestIn); + + Assert.Equal(ActivityKind.Server, activity.Kind); + + if (!enableGrpcAspNetCoreSupport.HasValue || enableGrpcAspNetCoreSupport.Value) + { + Assert.Equal("grpc", activity.GetTagValue(SemanticConventions.AttributeRpcSystem)); + Assert.Equal("greet.Greeter", activity.GetTagValue(SemanticConventions.AttributeRpcService)); + Assert.Equal("SayHello", activity.GetTagValue(SemanticConventions.AttributeRpcMethod)); + Assert.Contains(activity.GetTagValue(SemanticConventions.AttributeNetPeerIp), clientLoopbackAddresses); + Assert.NotEqual(0, activity.GetTagValue(SemanticConventions.AttributeNetPeerPort)); + Assert.Null(activity.GetTagValue(GrpcTagHelper.GrpcMethodTagName)); + Assert.Null(activity.GetTagValue(GrpcTagHelper.GrpcStatusCodeTagName)); + Assert.Equal(0, activity.GetTagValue(SemanticConventions.AttributeRpcGrpcStatusCode)); + } + else + { + Assert.NotNull(activity.GetTagValue(GrpcTagHelper.GrpcMethodTagName)); + Assert.NotNull(activity.GetTagValue(GrpcTagHelper.GrpcStatusCodeTagName)); + } + + Assert.Equal(Status.Unset, activity.GetStatus()); + + // The following are http.* attributes that are also included on the span for the gRPC invocation. + Assert.Equal($"localhost:{this.server.Port}", activity.GetTagValue(SemanticConventions.AttributeHttpHost)); + Assert.Equal("POST", activity.GetTagValue(SemanticConventions.AttributeHttpMethod)); + Assert.Equal("/greet.Greeter/SayHello", activity.GetTagValue(SpanAttributeConstants.HttpPathKey)); + Assert.Equal($"http://localhost:{this.server.Port}/greet.Greeter/SayHello", activity.GetTagValue(SemanticConventions.AttributeHttpUrl)); + Assert.StartsWith("grpc-dotnet", activity.GetTagValue(SemanticConventions.AttributeHttpUserAgent) as string); + } + finally + { + // Set the SDK to use the default propagator for other unit tests + Sdk.SetDefaultTextMapPropagator(new CompositeTextMapPropagator(new TextMapPropagator[] + { + new TraceContextPropagator(), + new BaggagePropagator(), + })); + } + } + public void Dispose() { this.server.Dispose(); diff --git a/test/OpenTelemetry.Tests/Trace/ActivityExtensionsTest.cs b/test/OpenTelemetry.Tests/Trace/ActivityExtensionsTest.cs index 35e55ec5be8..a0590ebccd6 100644 --- a/test/OpenTelemetry.Tests/Trace/ActivityExtensionsTest.cs +++ b/test/OpenTelemetry.Tests/Trace/ActivityExtensionsTest.cs @@ -171,6 +171,29 @@ public void GetTagValue() Assert.Null(activity.GetTagValue("Tag2")); } + [Theory] + [InlineData("Key", "Value", true)] + [InlineData("CustomTag", null, false)] + public void TryCheckFirstTag(string tagName, object expectedTagValue, bool expectedResult) + { + Activity activity = new Activity("Test"); + activity.SetTag("Key", "Value"); + + var result = activity.TryCheckFirstTag(tagName, out var tagValue); + Assert.Equal(expectedResult, result); + Assert.Equal(expectedTagValue, tagValue); + } + + [Fact] + public void TryCheckFirstTagReturnsFalseForActivityWithNoTags() + { + Activity activity = new Activity("Test"); + + var result = activity.TryCheckFirstTag("Key", out var tagValue); + Assert.False(result); + Assert.Null(tagValue); + } + [Fact] public void EnumerateTagValuesEmpty() { From 98055871511d0b56df75cb7443a1ed9a49918014 Mon Sep 17 00:00:00 2001 From: Reiley Yang Date: Mon, 3 May 2021 22:26:21 -0700 Subject: [PATCH 4/4] show release version on badges (instead of prerelease) (#2029) --- README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index cad33dac0d7..6f874282f22 100644 --- a/README.md +++ b/README.md @@ -2,8 +2,8 @@ [![Slack](https://img.shields.io/badge/slack-@cncf/otel/dotnet-brightgreen.svg?logo=slack)](https://cloud-native.slack.com/archives/C01N3BC2W7Q) [![codecov.io](https://codecov.io/gh/open-telemetry/opentelemetry-dotnet/branch/main/graphs/badge.svg?)](https://codecov.io/gh/open-telemetry/opentelemetry-dotnet/) -[![Release](https://img.shields.io/github/v/release/open-telemetry/opentelemetry-dotnet?include_prereleases&style=)](https://github.com/open-telemetry/opentelemetry-dotnet/releases/) -[![Nuget](https://img.shields.io/nuget/vpre/OpenTelemetry.svg)](https://www.nuget.org/profiles/OpenTelemetry) +[![Release](https://img.shields.io/github/v/release/open-telemetry/opentelemetry-dotnet)](https://github.com/open-telemetry/opentelemetry-dotnet/releases/) +[![Nuget](https://img.shields.io/nuget/v/OpenTelemetry.svg)](https://www.nuget.org/profiles/OpenTelemetry) [![NuGet](https://img.shields.io/nuget/dt/OpenTelemetry.svg)](https://www.nuget.org/profiles/OpenTelemetry) The .NET [OpenTelemetry](https://opentelemetry.io/) client.