Quick implementation for AzureClients - not validated as there are un…

…it tests and this is not planned to be in this repo as well.

src/OpenTelemetry.Instrumentation.Dependencies/AzureClientsInstrumentation.cs

@@ -14,12 +14,15 @@
 // limitations under the License.
 // </copyright>
 using System;
-using OpenTelemetry.Trace;
 
 namespace OpenTelemetry.Instrumentation.Dependencies
 {
     /// <summary>
-    /// Dependencies instrumentation.
+    /// AzureClients instrumentation.
+    /// TODO: Azure specific listeners would be moved out of this repo.
+    /// I believe this was initially put here for quick validation.
+    /// There were no unit tests covering this feature, so
+    /// cannot validate after Span is replaced with Activity.
     /// </summary>
     public class AzureClientsInstrumentation : IDisposable
     {
@@ -28,11 +31,10 @@ public class AzureClientsInstrumentation : IDisposable
         /// <summary>
         /// Initializes a new instance of the <see cref="AzureClientsInstrumentation"/> class.
         /// </summary>
-        /// <param name="tracer">Tracer to record traced with.</param>
-        public AzureClientsInstrumentation(Tracer tracer)
+        public AzureClientsInstrumentation()
         {
             this.diagnosticSourceSubscriber = new DiagnosticSourceSubscriber(
-                name => new AzureSdkDiagnosticListener(name, tracer),
+                name => new AzureSdkDiagnosticListener(name),
                 listener => listener.Name.StartsWith("Azure."),
                 null);
             this.diagnosticSourceSubscriber.Subscribe();

src/OpenTelemetry.Instrumentation.Dependencies/AzurePipelineInstrumentation.cs

@@ -14,12 +14,15 @@
 // limitations under the License.
 // </copyright>
 using System;
-using OpenTelemetry.Trace;
 
 namespace OpenTelemetry.Instrumentation.Dependencies
 {
     /// <summary>
-    /// Dependencies instrumentation.
+    /// AzurePipeline instrumentation.
+    /// TODO: Azure specific listeners would be moved out of this repo.
+    /// I believe this was initially put here for quick validation.
+    /// There were no unit tests covering this feature, so
+    /// cannot validate after Span is replaced with Activity.
     /// </summary>
     public class AzurePipelineInstrumentation : IDisposable
     {
@@ -28,10 +31,9 @@ public class AzurePipelineInstrumentation : IDisposable
         /// <summary>
         /// Initializes a new instance of the <see cref="AzurePipelineInstrumentation"/> class.
         /// </summary>
-        /// <param name="tracer">Tracer to record traced with.</param>
-        public AzurePipelineInstrumentation(Tracer tracer)
+        public AzurePipelineInstrumentation()
         {
-            this.diagnosticSourceSubscriber = new DiagnosticSourceSubscriber(new AzureSdkDiagnosticListener("Azure.Pipeline", tracer), null);
+            this.diagnosticSourceSubscriber = new DiagnosticSourceSubscriber(new AzureSdkDiagnosticListener("Azure.Pipeline"), null);
             this.diagnosticSourceSubscriber.Subscribe();
         }
 

src/OpenTelemetry.Instrumentation.Dependencies/Implementation/AzureSdkDiagnosticListener.cs

@@ -24,11 +24,16 @@ namespace OpenTelemetry.Instrumentation.Dependencies
 {
     internal class AzureSdkDiagnosticListener : ListenerHandler
     {
+        internal const string ActivitySourceName = "AzureSDK";
+        internal const string ActivityName = ActivitySourceName + ".HttpRequestOut";
+        private static readonly Version Version = typeof(AzureSdkDiagnosticListener).Assembly.GetName().Version;
+        private static readonly ActivitySource AzureSDKActivitySource = new ActivitySource(ActivitySourceName, Version.ToString());
+
         // all fetchers must not be reused between DiagnosticSources.
         private readonly PropertyFetcher linksPropertyFetcher = new PropertyFetcher("Links");
 
-        public AzureSdkDiagnosticListener(string sourceName, Tracer tracer)
-            : base(sourceName, tracer)
+        public AzureSdkDiagnosticListener(string sourceName)
+            : base(sourceName, null)
         {
         }
 
@@ -40,91 +45,65 @@ public void OnError(Exception error)
         {
         }
 
-        public override void OnStartActivity(Activity current, object valueValue)
+        public override void OnStartActivity(Activity activity, object valueValue)
         {
             string operationName = null;
-            var spanKind = SpanKind.Internal;
+            var activityKind = ActivityKind.Internal;
 
-            foreach (var keyValuePair in current.Tags)
+            foreach (var keyValuePair in activity.Tags)
             {
                 if (keyValuePair.Key == "http.url")
                 {
                     operationName = keyValuePair.Value;
-                    spanKind = SpanKind.Client;
+                    activityKind = ActivityKind.Client;
                     break;
                 }
 
                 if (keyValuePair.Key == "kind")
                 {
-                    if (Enum.TryParse(keyValuePair.Value, true, out SpanKind parsedSpanKind))
+                    if (Enum.TryParse(keyValuePair.Value, true, out ActivityKind parsedActivityKind))
                     {
-                        spanKind = parsedSpanKind;
+                        activityKind = parsedActivityKind;
                     }
                 }
             }
 
             if (operationName == null)
             {
-                operationName = this.GetOperationName(current);
+                operationName = this.GetOperationName(activity);
             }
 
-            List<Link> parentLinks = null;
+            List<ActivityLink> links = null;
             if (this.linksPropertyFetcher.Fetch(valueValue) is IEnumerable<Activity> activityLinks)
             {
                 if (activityLinks.Any())
                 {
-                    parentLinks = new List<Link>();
+                    links = new List<ActivityLink>();
                     foreach (var link in activityLinks)
                     {
                         if (link != null)
                         {
-                            parentLinks.Add(new Link(new SpanContext(link.TraceId, link.ParentSpanId, link.ActivityTraceFlags)));
+                            links.Add(new ActivityLink(new ActivityContext(link.TraceId, link.ParentSpanId, link.ActivityTraceFlags)));
                         }
                     }
                 }
             }
 
-            this.Tracer.StartSpanFromActivity(operationName, current, spanKind, parentLinks);
+            // Ignore the activity and create a new one using ActivitySource.
+            // The new one will have Sampling decision made using extracted Links as well.
+            AzureSDKActivitySource.StartActivity(operationName, activityKind, activity.Id, activity.Tags, links);
         }
 
         public override void OnStopActivity(Activity current, object valueValue)
         {
-            var span = this.Tracer.CurrentSpan;
-            try
-            {
-                if (span == null || !span.Context.IsValid)
-                {
-                    InstrumentationEventSource.Log.NullOrBlankSpan(this.SourceName + ".OnStopActivity");
-                    return;
-                }
-
-                if (span.IsRecording)
-                {
-                    foreach (var keyValuePair in current.Tags)
-                    {
-                        span.SetAttribute(keyValuePair.Key, keyValuePair.Value);
-                    }
-                }
-            }
-            finally
-            {
-                span?.End();
-            }
+            // nothing to be done.
         }
 
-        public override void OnException(Activity current, object valueValue)
+        public override void OnException(Activity activity, object valueValue)
         {
-            var span = this.Tracer.CurrentSpan;
-
-            if (span == null || !span.Context.IsValid)
-            {
-                InstrumentationEventSource.Log.NullOrBlankSpan(this.SourceName + ".OnException");
-                return;
-            }
-
-            span.Status = Status.Unknown.WithDescription(valueValue?.ToString());
-
-            // Note: Span.End() is not called here on purpose, OnStopActivity is called after OnException for this listener.
+            Status status = Status.Unknown;
+            activity.AddTag(SpanAttributeConstants.StatusCodeKey, SpanHelper.GetCachedCanonicalCodeString(status.CanonicalCode));
+            activity.AddTag(SpanAttributeConstants.StatusDescriptionKey, valueValue?.ToString());
         }
 
         private string GetOperationName(Activity activity)

src/OpenTelemetry.Instrumentation.Dependencies/Implementation/HttpHandlerDiagnosticListener.cs

@@ -154,7 +154,6 @@ public override void OnStopActivity(Activity activity, object payload)
         public override void OnException(Activity activity, object payload)
         {
             const string EventNameSuffix = ".OnException";
-            var span = this.Tracer.CurrentSpan;
 
             if (activity.IsAllDataRequested)
             {
@@ -180,7 +179,6 @@ public override void OnException(Activity activity, object payload)
 
                     if (exc.InnerException != null)
                     {
-                        span.Status = Status.Unknown.WithDescription(exc.Message);
                         Status status = Status.Unknown;
                         activity.AddTag(SpanAttributeConstants.StatusCodeKey, SpanHelper.GetCachedCanonicalCodeString(status.CanonicalCode));
                         activity.AddTag(SpanAttributeConstants.StatusDescriptionKey, exc.Message);

src/OpenTelemetry.Instrumentation.Dependencies/OpenTelemetryBuilderExtensions.cs

@@ -39,6 +39,7 @@ public static OpenTelemetryBuilder AddDependencyInstrumentation(this OpenTelemet
 
             builder.AddHttpClientDependencyInstrumentation(null);
             builder.AddSqlClientDependencyInstrumentation(null);
+            builder.AddAzureClientsDependencyInstrumentation();
 #if NET461
             builder.AddHttpWebRequestDependencyInstrumentation();
 #endif
@@ -64,6 +65,7 @@ public static OpenTelemetryBuilder AddDependencyInstrumentation(this OpenTelemet
 
             builder.AddHttpClientDependencyInstrumentation(configureHttpClientInstrumentationOptions);
             builder.AddSqlClientDependencyInstrumentation(configureSqlClientInstrumentationOptions);
+            builder.AddAzureClientsDependencyInstrumentation();
 #if NET461
             builder.AddHttpWebRequestDependencyInstrumentation();
 #endif
@@ -85,6 +87,8 @@ public static OpenTelemetryBuilder AddDependencyInstrumentation(this OpenTelemet
                 throw new ArgumentNullException(nameof(builder));
             }
 
+            // HttpClient is not instrumented with ActivitySource, hence
+            // it'll have a default ActivitySource with name string.Empty.
             builder.AddActivitySource(string.Empty);
             var httpClientOptions = new HttpClientInstrumentationOptions();
             configureHttpClientInstrumentationOptions?.Invoke(httpClientOptions);
@@ -109,6 +113,8 @@ public static OpenTelemetryBuilder AddDependencyInstrumentation(this OpenTelemet
                 throw new ArgumentNullException(nameof(builder));
             }
 
+            // HttpClient is not instrumented with ActivitySource, hence
+            // it'll have a default ActivitySource with name string.Empty.
             builder.AddActivitySource(string.Empty);
             var sqlOptions = new SqlClientInstrumentationOptions();
             configureSqlClientInstrumentationOptions?.Invoke(sqlOptions);
@@ -118,6 +124,26 @@ public static OpenTelemetryBuilder AddDependencyInstrumentation(this OpenTelemet
             return builder;
         }
 
+        /// <summary>
+        /// Enables instrumentation for Azure clients.
+        /// </summary>
+        /// <param name="builder"><see cref="OpenTelemetryBuilder"/> being configured.</param>
+        /// <returns>The instance of <see cref="OpenTelemetryBuilder"/> to chain the calls.</returns>
+        public static OpenTelemetryBuilder AddAzureClientsDependencyInstrumentation(
+            this OpenTelemetryBuilder builder)
+        {
+            if (builder == null)
+            {
+                throw new ArgumentNullException(nameof(builder));
+            }
+
+            builder.AddActivitySource(AzureSdkDiagnosticListener.ActivitySourceName);
+
+            // TODO: decide who is responsible for dispose upon shutdown.
+            new AzureClientsInstrumentation();
+            return builder;
+        }
+
 #if NET461
         /// <summary>
         /// Enables the outgoing requests automatic data collection for .NET Framework HttpWebRequest activity source.

src/OpenTelemetry.Instrumentation.Dependencies/TracerBuilderExtensions.cs

@@ -37,7 +37,6 @@ public static TracerBuilder AddDependencyInstrumentation(this TracerBuilder buil
             }
 
             return builder
-                .AddInstrumentation((t) => new AzureClientsInstrumentation(t))
                 .AddInstrumentation((t) => new AzurePipelineInstrumentation(t));
         }
 
@@ -58,12 +57,7 @@ public static TracerBuilder AddDependencyInstrumentation(this TracerBuilder buil
                 throw new ArgumentNullException(nameof(builder));
             }
 
-            var httpOptions = new HttpClientInstrumentationOptions();
-            configureHttpInstrumentationOptions?.Invoke(httpOptions);
-
-            return builder
-                .AddInstrumentation((t) => new AzureClientsInstrumentation(t))
-                .AddInstrumentation((t) => new AzurePipelineInstrumentation(t));
+            return builder.AddInstrumentation((t) => new AzurePipelineInstrumentation(t));
         }
     }
 }