Merge branch 'alanwest/consistent-extension-methods' of github.com:al…

…anwest/opentelemetry-dotnet into alanwest/consistent-extension-methods

docs/trace/extending-the-sdk/MyFilteringProcessor.cs

@@ -0,0 +1,51 @@
+// <copyright file="MyFilteringProcessor.cs" company="OpenTelemetry Authors">
+// Copyright The OpenTelemetry Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+// </copyright>
+
+using System;
+using System.Diagnostics;
+using OpenTelemetry;
+
+internal class MyFilteringProcessor : BaseProcessor<Activity>
+{
+    private readonly Func<Activity, bool> filter;
+    private readonly BaseProcessor<Activity> processor;
+
+    public MyFilteringProcessor(BaseProcessor<Activity> processor, Func<Activity, bool> filter)
+    {
+        if (filter == null)
+        {
+            throw new ArgumentNullException(nameof(filter));
+        }
+
+        if (processor == null)
+        {
+            throw new ArgumentNullException(nameof(processor));
+        }
+
+        this.filter = filter;
+        this.processor = processor;
+    }
+
+    public override void OnEnd(Activity activity)
+    {
+        // Call the underlying processor
+        // only if the Filter returns true.
+        if (this.filter(activity))
+        {
+            this.processor.OnEnd(activity);
+        }
+    }
+}

docs/trace/extending-the-sdk/README.md

@@ -206,6 +206,30 @@ class MyProcessor : BaseProcessor<Activity>
 
 A demo processor is shown [here](./MyProcessor.cs).
 
+### Filtering Processor
+
+A common use case of writing custom processor is to filter Activities from being
+exported. Such a "FilteringProcessor" can be written as a wrapper around an
+underlying processor. An example "FilteringProcessor" is shown
+[here](./MyFilteringProcessor.cs).
+
+When using such a filtering processor, instead of using extension method to
+register the exporter, they must be registered manually as shown below:
+
+```csharp
+    using var tracerProvider = Sdk.CreateTracerProviderBuilder()
+        .SetSampler(new MySampler())
+        .AddSource("OTel.Demo")
+        .AddProcessor(new MyFilteringProcessor(
+            new SimpleExportProcessor<Activity>(new MyExporter("ExporterX")),
+            (act) => true))
+        .Build();
+```
+
+Most [instrumentation libraries](#instrumentation-library) shipped from this
+repo provides a built-in `Filter` option to achieve the same effect. In such
+cases, it is recommended to use that option as it offers higher performance.
+
 ## Sampler
 
 OpenTelemetry .NET SDK has provided the following built-in samplers:

examples/Console/TestRedis.cs

@@ -95,9 +95,7 @@ private static void DoWork(IDatabase db, ActivitySource activitySource)
                 }
                 catch (ArgumentOutOfRangeException e)
                 {
-                    // Set status upon error
-                    activity.SetTag(SpanAttributeConstants.StatusCodeKey, (int)Status.Error.StatusCode);
-                    activity.SetTag(SpanAttributeConstants.StatusDescriptionKey, e.ToString());
+                    activity.SetStatus(Status.Error.WithDescription(e.ToString()));
                 }
 
                 // Annotate our activity to capture metadata about our operation

src/OpenTelemetry.Api/CHANGELOG.md

@@ -2,6 +2,13 @@
 
 ## Unreleased
 
+* In order to align with the
+  [spec](https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/trace/api.md#set-status)
+  the `Status` (otel.status_code) tag (added on `Activity` using the `SetStatus`
+  extension) will now be set as the `Unset`, `Error`, or `Ok` string
+  representation instead of the `0`, `1`, or `2` integer representation.
+  ([#1579](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1579))
+
 ## 1.0.0-rc1.1
 
 Released 2020-Nov-17

src/OpenTelemetry.Api/Internal/ActivityHelperExtensions.cs

@@ -14,7 +14,6 @@
 // limitations under the License.
 // </copyright>
 
-using System;
 using System.Collections.Generic;
 using System.Diagnostics;
 using System.Reflection;
@@ -34,42 +33,29 @@ internal static class ActivityHelperExtensions
         /// This extension provides a workaround to retrieve Status from special tags with key name otel.status_code and otel.status_description.
         /// </summary>
         /// <param name="activity">Activity instance.</param>
-        /// <returns>Activity execution status.</returns>
+        /// <param name="statusCode"><see cref="StatusCode"/>.</param>
+        /// <param name="statusDescription">Status description.</param>
+        /// <returns><see langword="true"/> if <see cref="Status"/> was found on the supplied Activity.</returns>
         [MethodImpl(MethodImplOptions.AggressiveInlining)]
         [System.Diagnostics.CodeAnalysis.SuppressMessage("Design", "CA1062:Validate arguments of public methods", Justification = "ActivityProcessor is hot path")]
-        public static Status GetStatusHelper(this Activity activity)
+        public static bool TryGetStatus(this Activity activity, out StatusCode statusCode, out string statusDescription)
         {
             Debug.Assert(activity != null, "Activity should not be null");
 
             ActivityStatusTagEnumerator state = default;
 
             ActivityTagsEnumeratorFactory<ActivityStatusTagEnumerator>.Enumerate(activity, ref state);
 
-            if (!state.IsValid)
+            if (!state.StatusCode.HasValue)
             {
-                return default;
+                statusCode = default;
+                statusDescription = null;
+                return false;
             }
 
-            Status status;
-            if (state.StatusCode == StatusCode.Error)
-            {
-                status = Status.Error;
-            }
-            else if (state.StatusCode == StatusCode.Ok)
-            {
-                status = Status.Ok;
-            }
-            else
-            {
-                status = Status.Unset;
-            }
-
-            if (!string.IsNullOrEmpty(state.StatusDescription))
-            {
-                return status.WithDescription(state.StatusDescription);
-            }
-
-            return status;
+            statusCode = state.StatusCode.Value;
+            statusDescription = state.StatusDescription;
+            return true;
         }
 
         /// <summary>
@@ -193,9 +179,7 @@ public bool ForEach(KeyValuePair<string, object> item)
 
         private struct ActivityStatusTagEnumerator : IActivityEnumerator<KeyValuePair<string, object>>
         {
-            public bool IsValid;
-
-            public StatusCode StatusCode;
+            public StatusCode? StatusCode;
 
             public string StatusDescription;
 
@@ -204,15 +188,14 @@ public bool ForEach(KeyValuePair<string, object> item)
                 switch (item.Key)
                 {
                     case SpanAttributeConstants.StatusCodeKey:
-                        this.StatusCode = (StatusCode)item.Value;
-                        this.IsValid = this.StatusCode == StatusCode.Error || this.StatusCode == StatusCode.Ok || this.StatusCode == StatusCode.Unset;
+                        this.StatusCode = StatusHelper.GetStatusCodeForStringName(item.Value as string);
                         break;
                     case SpanAttributeConstants.StatusDescriptionKey:
                         this.StatusDescription = item.Value as string;
                         break;
                 }
 
-                return this.IsValid || this.StatusDescription == null;
+                return !this.StatusCode.HasValue || this.StatusDescription == null;
             }
         }
 

src/OpenTelemetry.Api/Internal/SpanHelper.cs

@@ -29,14 +29,12 @@ internal static class SpanHelper
         /// <returns>Resolved span <see cref="Status"/> for the Http status code.</returns>
         public static Status ResolveSpanStatusForHttpStatusCode(int httpStatusCode)
         {
-            var status = Status.Error;
-
             if (httpStatusCode >= 100 && httpStatusCode <= 399)
             {
-                status = Status.Unset;
+                return Status.Unset;
             }
 
-            return status;
+            return Status.Error;
         }
     }
 }

src/OpenTelemetry.Api/Internal/StatusHelper.cs

@@ -0,0 +1,58 @@
+// <copyright file="StatusHelper.cs" company="OpenTelemetry Authors">
+// Copyright The OpenTelemetry Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+// </copyright>
+
+using System.Runtime.CompilerServices;
+using OpenTelemetry.Trace;
+
+namespace OpenTelemetry.Internal
+{
+    internal static class StatusHelper
+    {
+        [MethodImpl(MethodImplOptions.AggressiveInlining)]
+        public static string GetStringNameForStatusCode(StatusCode statusCode)
+        {
+            return statusCode switch
+            {
+                /*
+                 * Note: Order here does matter for perf. Unset is
+                 * first because assumption is most spans will be
+                 * Unset, then Error. Ok is not set by the SDK.
+                 */
+                StatusCode.Unset => "Unset",
+                StatusCode.Error => "Error",
+                StatusCode.Ok => "Ok",
+                _ => null,
+            };
+        }
+
+        [MethodImpl(MethodImplOptions.AggressiveInlining)]
+        public static StatusCode? GetStatusCodeForStringName(string statusCodeName)
+        {
+            return statusCodeName switch
+            {
+                /*
+                 * Note: Order here does matter for perf. Unset is
+                 * first because assumption is most spans will be
+                 * Unset, then Error. Ok is not set by the SDK.
+                 */
+                "Unset" => StatusCode.Unset,
+                "Error" => StatusCode.Error,
+                "Ok" => StatusCode.Ok,
+                _ => (StatusCode?)null,
+            };
+        }
+    }
+}

src/OpenTelemetry.Api/Trace/ActivityExtensions.cs

@@ -45,7 +45,7 @@ public static void SetStatus(this Activity activity, Status status)
         {
             Debug.Assert(activity != null, "Activity should not be null");
 
-            activity.SetTag(SpanAttributeConstants.StatusCodeKey, (int)status.StatusCode);
+            activity.SetTag(SpanAttributeConstants.StatusCodeKey, StatusHelper.GetStringNameForStatusCode(status.StatusCode));
             activity.SetTag(SpanAttributeConstants.StatusDescriptionKey, status.Description);
         }
 
@@ -60,7 +60,12 @@ public static void SetStatus(this Activity activity, Status status)
         [System.Diagnostics.CodeAnalysis.SuppressMessage("Design", "CA1062:Validate arguments of public methods", Justification = "ActivityProcessor is hot path")]
         public static Status GetStatus(this Activity activity)
         {
-            return activity.GetStatusHelper();
+            if (!activity.TryGetStatus(out StatusCode statusCode, out string statusDescription))
+            {
+                return Status.Unset;
+            }
+
+            return new Status(statusCode, statusDescription);
         }
 
         /// <summary>

src/OpenTelemetry.Exporter.Jaeger/CHANGELOG.md

@@ -2,6 +2,9 @@
 
 ## Unreleased
 
+* Jaeger will now set the `error` tag when `otel.status_code` is set to `Error`.
+  ([#1579](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1579))
+
 ## 1.0.0-rc1.1
 
 Released 2020-Nov-17

src/OpenTelemetry.Exporter.Jaeger/Implementation/JaegerActivityExtensions.cs

@@ -255,6 +255,11 @@ private static void ProcessJaegerTag(ref TagEnumerationState state, string key,
             if (jaegerTag.VStr != null)
             {
                 PeerServiceResolver.InspectTag(ref state, key, jaegerTag.VStr);
+
+                if (key == SpanAttributeConstants.StatusCodeKey && jaegerTag.VStr == "Error")
+                {
+                    PooledList<JaegerTag>.Add(ref state.Tags, new JaegerTag("error", JaegerTagType.BOOL, vBool: true));
+                }
             }
             else if (jaegerTag.VLong.HasValue)
             {

src/OpenTelemetry.Exporter.Jaeger/OpenTelemetry.Exporter.Jaeger.csproj

@@ -15,6 +15,7 @@
     <Compile Include="$(RepoRoot)\src\OpenTelemetry.Api\Internal\SemanticConventions.cs" Link="Includes\SemanticConventions.cs" />
     <Compile Include="$(RepoRoot)\src\OpenTelemetry.Api\Internal\SpanAttributeConstants.cs" Link="Includes\SpanAttributeConstants.cs" />
     <Compile Include="$(RepoRoot)\src\OpenTelemetry.Api\Internal\ActivityHelperExtensions.cs" Link="Includes\ActivityHelperExtensions.cs" />
+    <Compile Include="$(RepoRoot)\src\OpenTelemetry.Api\Internal\StatusHelper.cs" Link="Includes\StatusHelper.cs" />
     <Compile Include="$(RepoRoot)\src\OpenTelemetry.Api\Internal\IActivityEnumerator.cs" Link="Includes\IActivityEnumerator.cs" />
     <Compile Include="$(RepoRoot)\src\OpenTelemetry.Api\Internal\EnumerationHelper.cs" Link="Includes\EnumerationHelper.cs" />
     <Compile Include="$(RepoRoot)\src\OpenTelemetry\Internal\PooledList.cs" Link="Includes\PooledList.cs" />

src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/ActivityExtensions.cs

@@ -239,15 +239,17 @@ internal static OtlpCommon.KeyValue ToOtlpAttribute(this KeyValuePair<string, ob
         [MethodImpl(MethodImplOptions.AggressiveInlining)]
         private static OtlpTrace.Status ToOtlpStatus(ref TagEnumerationState otlpTags)
         {
-            if (!otlpTags.StatusCode.HasValue)
+            var status = StatusHelper.GetStatusCodeForStringName(otlpTags.StatusCode);
+
+            if (!status.HasValue)
             {
                 return null;
             }
 
             var otlpStatus = new OtlpTrace.Status
             {
                 // The numerical values of the two enumerations match, a simple cast is enough.
-                Code = (OtlpTrace.Status.Types.StatusCode)otlpTags.StatusCode,
+                Code = (OtlpTrace.Status.Types.StatusCode)(int)status,
             };
 
             if (otlpStatus.Code != OtlpTrace.Status.Types.StatusCode.Error)
@@ -370,7 +372,7 @@ private struct TagEnumerationState : IActivityEnumerator<KeyValuePair<string, ob
 
             public PooledList<OtlpCommon.KeyValue> Tags;
 
-            public int? StatusCode;
+            public string StatusCode;
 
             public string StatusDescription;
 
@@ -396,7 +398,7 @@ public bool ForEach(KeyValuePair<string, object> activityTag)
                 switch (key)
                 {
                     case SpanAttributeConstants.StatusCodeKey:
-                        this.StatusCode = activityTag.Value as int?;
+                        this.StatusCode = activityTag.Value as string;
                         return true;
                     case SpanAttributeConstants.StatusDescriptionKey:
                         this.StatusDescription = activityTag.Value as string;

src/OpenTelemetry.Exporter.OpenTelemetryProtocol/OpenTelemetry.Exporter.OpenTelemetryProtocol.csproj

@@ -15,6 +15,7 @@
     <Compile Include="$(RepoRoot)\src\OpenTelemetry.Api\Internal\SemanticConventions.cs" Link="Includes\SemanticConventions.cs" />
     <Compile Include="$(RepoRoot)\src\OpenTelemetry.Api\Internal\SpanAttributeConstants.cs" Link="Includes\SpanAttributeConstants.cs" />
     <Compile Include="$(RepoRoot)\src\OpenTelemetry.Api\Internal\ActivityHelperExtensions.cs" Link="Includes\ActivityHelperExtensions.cs" />
+    <Compile Include="$(RepoRoot)\src\OpenTelemetry.Api\Internal\StatusHelper.cs" Link="Includes\StatusHelper.cs" />
     <Compile Include="$(RepoRoot)\src\OpenTelemetry.Api\Internal\IActivityEnumerator.cs" Link="Includes\IActivityEnumerator.cs" />
     <Compile Include="$(RepoRoot)\src\OpenTelemetry.Api\Internal\EnumerationHelper.cs" Link="Includes\EnumerationHelper.cs" />
     <Compile Include="$(RepoRoot)\src\OpenTelemetry\Internal\PooledList.cs" Link="Includes\PooledList.cs" />

src/OpenTelemetry.Exporter.Zipkin/CHANGELOG.md

@@ -2,6 +2,9 @@
 
 ## Unreleased
 
+* Zipkin will now set the `error` tag when `otel.status_code` is set to `Error`.
+  ([#1579](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1579))
+
 ## 1.0.0-rc1.1
 
 Released 2020-Nov-17

src/OpenTelemetry.Exporter.Zipkin/Implementation/ZipkinActivityConversionExtensions.cs

@@ -174,6 +174,11 @@ public bool ForEach(KeyValuePair<string, object> activityTag)
                 if (activityTag.Value is string strVal)
                 {
                     PeerServiceResolver.InspectTag(ref this, key, strVal);
+
+                    if (key == SpanAttributeConstants.StatusCodeKey && strVal == "Error")
+                    {
+                        PooledList<KeyValuePair<string, object>>.Add(ref this.Tags, new KeyValuePair<string, object>("error", "true"));
+                    }
                 }
                 else if (activityTag.Value is int intVal && activityTag.Key == SemanticConventions.AttributeNetPeerPort)
                 {