remove sampling from instrumentation

open-telemetry · Jun 2, 2020 · 9b840f0 · 9b840f0
1 parent 737f9b6
commit 9b840f0
Show file tree

Hide file tree

Showing 2 changed files with 23 additions and 29 deletions.
diff --git a/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInListener.cs b/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInListener.cs
@@ -22,7 +22,6 @@
 using Microsoft.AspNetCore.Http.Features;
 using OpenTelemetry.Context.Propagation;
 using OpenTelemetry.Trace;
-using OpenTelemetry.Trace.Samplers;
 
 namespace OpenTelemetry.Instrumentation.AspNetCore.Implementation
 {
@@ -36,7 +35,6 @@ internal class HttpInListener : ListenerHandler
         private readonly PropertyFetcher beforeActionTemplateFetcher = new PropertyFetcher("Template");
         private readonly bool hostingSupportsW3C = false;
         private readonly AspNetCoreInstrumentationOptions options;
-        private readonly ActivitySampler sampler = new AlwaysOnActivitySampler();
 
         public HttpInListener(string name, Tracer tracer, AspNetCoreInstrumentationOptions options)
             : base(name, tracer)
@@ -68,44 +66,28 @@ public override void OnStartActivity(Activity activity, object payload)
 
             var request = context.Request;
 
-            // see the spec https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/data-semantic-conventions.md
-            var path = (request.PathBase.HasValue || request.Path.HasValue) ? (request.PathBase + request.Path).ToString() : "/";
-            if (activity != null)
+            if (!this.hostingSupportsW3C || !(this.options.TextFormat is TraceContextFormat))
             {
-                activity.DisplayName = path;
-            }
+                // This requires to ignore the current activity and create a new one
+                // using the context extracted from w3ctraceprent header or
+                // using whatever the TextFormat offers.
+                // TODO: actually implement code doing the above.
 
-            var samplingDecision = this.sampler.ShouldSample(activity.Context, activity.TraceId, default, path, activity.Kind, activity.Tags, activity.Links);
-            activity.IsAllDataRequested = samplingDecision.IsSampled;
-
-            TelemetrySpan span;
-            if (this.hostingSupportsW3C && this.options.TextFormat is TraceContextFormat)
-            {
-                this.Tracer.StartActiveSpanFromActivity(path, Activity.Current, SpanKind.Server, out span);
-            }
-            else
-            {
+                /*
                 var ctx = this.options.TextFormat.Extract<HttpRequest>(
                     request,
                     (r, name) => r.Headers[name]);
 
                 this.Tracer.StartActiveSpan(path, ctx, SpanKind.Server, out span);
-            }
-
-            if (span.IsRecording)
-            {
-                // Note, route is missing at this stage. It will be available later
-                span.PutHttpHostAttribute(request.Host.Host, request.Host.Port ?? 80);
-                span.PutHttpMethodAttribute(request.Method);
-                span.PutHttpPathAttribute(path);
-
-                var userAgent = request.Headers["User-Agent"].FirstOrDefault();
-                span.PutHttpUserAgentAttribute(userAgent);
-                span.PutHttpRawUrlAttribute(GetUri(request));
+                */
             }
 
             if (activity.IsAllDataRequested)
             {
+                // see the spec https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/data-semantic-conventions.md
+                var path = (request.PathBase.HasValue || request.Path.HasValue) ? (request.PathBase + request.Path).ToString() : "/";
+                activity.DisplayName = path;
+
                 if (request.Host.Port == 80 || request.Host.Port == 443)
                 {
                     activity.AddTag("http.host", request.Host.Host);

diff --git a/src/OpenTelemetry/Trace/Export/SimpleActivityProcessor.cs b/src/OpenTelemetry/Trace/Export/SimpleActivityProcessor.cs
@@ -18,6 +18,7 @@
 using System.Threading;
 using System.Threading.Tasks;
 using OpenTelemetry.Internal;
+using OpenTelemetry.Trace.Samplers;
 
 namespace OpenTelemetry.Trace.Export
 {
@@ -27,6 +28,7 @@ namespace OpenTelemetry.Trace.Export
     public class SimpleActivityProcessor : ActivityProcessor, IDisposable
     {
         private readonly ActivityExporter exporter;
+        private readonly ActivitySampler sampler = new AlwaysOnActivitySampler();
         private bool disposed = false;
 
         /// <summary>
@@ -41,6 +43,16 @@ public SimpleActivityProcessor(ActivityExporter exporter)
         /// <inheritdoc />
         public override void OnStart(Activity activity)
         {
+            // Perform sampling decision if source is empty.
+            // This occurs for activities created outside of ActivitySource
+            // i.e new Activity() directly.
+            // Note: This also can occur if one creates an ActivitySource of the name string.empty.
+            // In that case, the sampling logic would be run twice!
+            if (activity.Source.Name.Equals(string.Empty))
+            {
+                var samplingDecision = this.sampler.ShouldSample(activity.Context, activity.TraceId, default, activity.DisplayName, activity.Kind, activity.Tags, activity.Links);
+                activity.IsAllDataRequested = samplingDecision.IsSampled;
+            }
         }
 
         /// <inheritdoc />