open-telemetry#229: NullReferenceException in TelemetryDispatchMessag…

…eInspector.BeforeSendReply when operation is OneWay (open-telemetry#237)

* Update TelemetryDispatchMessageInspector.cs

open-telemetry#229: Check if reply is not null

* open-telemetry#229: Added TelemetryDispatchMessageInspectorForOneWayOperationsTests

* open-telemetry#229: Rename parameters

* open-telemetry#229: Added new "1.0.0-rc6" version to CHANGELOG.md

* open-telemetry#229: Fix text formatting

* open-telemetry#229: Fix CHANGELOG formatting. Added extra asserts to the test

* open-telemetry#229: Fix formatting

* open-telemetry#229: Fix formatting in GHANGELOG.md

Co-authored-by: iepl <[email protected]>
Co-authored-by: Ievgen Platonov <[email protected]>

src/OpenTelemetry.Contrib.Instrumentation.Wcf/CHANGELOG.md

@@ -1,5 +1,11 @@
 # Changelog
 
+## 1.0.0-rc6
+
+* Fixed a `NullReferenceException` in
+  `TelemetryDispatchMessageInspector.BeforeSendReply` when operation is OneWay
+  ([#237](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/237))
+
 ## 1.0.0-rc5
 
 * Fixed an `ArgumentNullException` setting `Activity`.`DisplayName` when

src/OpenTelemetry.Contrib.Instrumentation.Wcf/TelemetryDispatchMessageInspector.cs

@@ -131,7 +131,7 @@ public void BeforeSendReply(ref Message reply, object correlationState)
         {
             if (correlationState is Activity activity)
             {
-                if (activity.IsAllDataRequested)
+                if (activity.IsAllDataRequested && reply != null)
                 {
                     if (reply.IsFault)
                     {

test/OpenTelemetry.Contrib.Instrumentation.Wcf.Tests/TelemetryDispatchMessageInspectorForOneWayOperationsTests.netfx.cs

@@ -0,0 +1,158 @@
+// <copyright file="TelemetryDispatchMessageInspectorForOneWayOperationsTests.netfx.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>
+#if NETFRAMEWORK
+using System;
+using System.Collections.Generic;
+using System.Diagnostics;
+using System.Linq;
+using System.ServiceModel;
+using System.Threading;
+using OpenTelemetry.Contrib.Instrumentation.Wcf.Tests.Tools;
+using OpenTelemetry.Trace;
+using Xunit;
+using Xunit.Abstractions;
+
+namespace OpenTelemetry.Contrib.Instrumentation.Wcf.Tests
+{
+    [Collection("WCF")]
+    public class TelemetryDispatchMessageInspectorForOneWayOperationsTests : IDisposable
+    {
+        private readonly ITestOutputHelper output;
+        private readonly Uri serviceBaseUri;
+        private readonly ServiceHost serviceHost;
+
+        private readonly EventWaitHandle thrownExceptionsHandle = new EventWaitHandle(false, EventResetMode.ManualReset);
+        private readonly List<Exception> thrownExceptions = new List<Exception>();
+
+        public TelemetryDispatchMessageInspectorForOneWayOperationsTests(ITestOutputHelper outputHelper)
+        {
+            this.output = outputHelper;
+
+            Random random = new Random();
+            var retryCount = 5;
+            while (retryCount > 0)
+            {
+                try
+                {
+                    this.serviceBaseUri = new Uri($"net.tcp://localhost:{random.Next(2000, 5000)}/");
+                    this.serviceHost = new ServiceHost(new Service(), this.serviceBaseUri);
+
+                    var endpoint = this.serviceHost.AddServiceEndpoint(
+                        typeof(IServiceContract),
+                        new NetTcpBinding(),
+                        "/Service");
+                    endpoint.Behaviors.Add(new TelemetryEndpointBehavior());
+
+                    this.serviceHost.Description.Behaviors.Add(
+                        new ErrorHandlerServiceBehavior(this.thrownExceptionsHandle, ex => this.thrownExceptions.Add(ex)));
+
+                    this.serviceHost.Open();
+
+                    break;
+                }
+                catch (Exception ex)
+                {
+                    this.output.WriteLine(ex.ToString());
+                    if (this.serviceHost.State == CommunicationState.Faulted)
+                    {
+                        this.serviceHost.Abort();
+                    }
+                    else
+                    {
+                        this.serviceHost.Close();
+                    }
+
+                    this.serviceHost = null;
+                    retryCount--;
+                }
+            }
+
+            if (this.serviceHost == null)
+            {
+                throw new InvalidOperationException("ServiceHost could not be started.");
+            }
+        }
+
+        public void Dispose()
+        {
+            this.serviceHost?.Close();
+            this.thrownExceptionsHandle?.Dispose();
+        }
+
+        [Fact]
+        public void IncomingRequestOneWayOperationInstrumentationTest()
+        {
+            List<Activity> stoppedActivities = new List<Activity>();
+
+            using ActivityListener activityListener = new ActivityListener
+            {
+                ShouldListenTo = activitySource => true,
+                ActivityStopped = activity => stoppedActivities.Add(activity),
+            };
+
+            ActivitySource.AddActivityListener(activityListener);
+            TracerProvider tracerProvider = Sdk.CreateTracerProviderBuilder()
+                    .AddWcfInstrumentation()
+                    .Build();
+
+            ServiceClient client = new ServiceClient(
+               new NetTcpBinding(),
+               new EndpointAddress(new Uri(this.serviceBaseUri, "/Service")));
+
+            try
+            {
+                client.ExecuteWithOneWay(new ServiceRequest());
+                this.thrownExceptionsHandle.WaitOne(3000);
+            }
+            finally
+            {
+                if (client.State == CommunicationState.Faulted)
+                {
+                    client.Abort();
+                }
+                else
+                {
+                    client.Close();
+                }
+
+                tracerProvider?.Shutdown();
+                tracerProvider?.Dispose();
+
+                WcfInstrumentationActivitySource.Options = null;
+            }
+
+            // Assert
+            Assert.Empty(this.thrownExceptions);
+
+            Assert.NotEmpty(stoppedActivities);
+            Assert.Single(stoppedActivities);
+
+            Activity activity = stoppedActivities[0];
+            Assert.Equal("http://opentelemetry.io/Service/ExecuteWithOneWay", activity.DisplayName);
+            Assert.Equal("ExecuteWithOneWay", activity.TagObjects.FirstOrDefault(t => t.Key == WcfInstrumentationConstants.RpcMethodTag).Value);
+            Assert.DoesNotContain(activity.TagObjects, t => t.Key == WcfInstrumentationConstants.SoapReplyActionTag);
+
+            Assert.Equal(WcfInstrumentationActivitySource.IncomingRequestActivityName, activity.OperationName);
+            Assert.Equal(WcfInstrumentationConstants.WcfSystemValue, activity.TagObjects.FirstOrDefault(t => t.Key == WcfInstrumentationConstants.RpcSystemTag).Value);
+            Assert.Equal("http://opentelemetry.io/Service", activity.TagObjects.FirstOrDefault(t => t.Key == WcfInstrumentationConstants.RpcServiceTag).Value);
+            Assert.Equal(this.serviceBaseUri.Host, activity.TagObjects.FirstOrDefault(t => t.Key == WcfInstrumentationConstants.NetHostNameTag).Value);
+            Assert.Equal(this.serviceBaseUri.Port, activity.TagObjects.FirstOrDefault(t => t.Key == WcfInstrumentationConstants.NetHostPortTag).Value);
+            Assert.Equal("net.tcp", activity.TagObjects.FirstOrDefault(t => t.Key == WcfInstrumentationConstants.WcfChannelSchemeTag).Value);
+            Assert.Equal("/Service", activity.TagObjects.FirstOrDefault(t => t.Key == WcfInstrumentationConstants.WcfChannelPathTag).Value);
+        }
+    }
+}
+#endif

test/OpenTelemetry.Contrib.Instrumentation.Wcf.Tests/Tools/ErrorHandler.netfx.cs

@@ -0,0 +1,48 @@
+// <copyright file="ErrorHandler.netfx.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>
+#if NETFRAMEWORK
+using System;
+using System.ServiceModel.Channels;
+using System.ServiceModel.Dispatcher;
+using System.Threading;
+
+namespace OpenTelemetry.Contrib.Instrumentation.Wcf.Tests.Tools
+{
+    internal class ErrorHandler : IErrorHandler
+    {
+        private readonly EventWaitHandle handle;
+        private readonly Action<Exception> log;
+
+        public ErrorHandler(EventWaitHandle handle, Action<Exception> log)
+        {
+            this.handle = handle;
+            this.log = log;
+        }
+
+        public bool HandleError(Exception error)
+        {
+            this.log(error);
+            this.handle.Set();
+
+            return true;
+        }
+
+        public void ProvideFault(Exception error, MessageVersion version, ref Message fault)
+        {
+        }
+    }
+}
+#endif

test/OpenTelemetry.Contrib.Instrumentation.Wcf.Tests/Tools/ErrorHandlerServiceBehavior.netfx.cs

@@ -0,0 +1,55 @@
+// <copyright file="ErrorHandlerServiceBehavior.netfx.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>
+#if NETFRAMEWORK
+using System;
+using System.Collections.ObjectModel;
+using System.ServiceModel;
+using System.ServiceModel.Channels;
+using System.ServiceModel.Description;
+using System.ServiceModel.Dispatcher;
+using System.Threading;
+
+namespace OpenTelemetry.Contrib.Instrumentation.Wcf.Tests.Tools
+{
+    internal class ErrorHandlerServiceBehavior : IServiceBehavior
+    {
+        private readonly EventWaitHandle handle;
+        private readonly Action<Exception> action;
+
+        public ErrorHandlerServiceBehavior(EventWaitHandle handle, Action<Exception> action)
+        {
+            this.handle = handle;
+            this.action = action;
+        }
+
+        public void AddBindingParameters(ServiceDescription serviceDescription, ServiceHostBase serviceHostBase, Collection<ServiceEndpoint> endpoints, BindingParameterCollection bindingParameters)
+        {
+        }
+
+        public void ApplyDispatchBehavior(ServiceDescription serviceDescription, ServiceHostBase serviceHostBase)
+        {
+            foreach (ChannelDispatcher dispatcher in serviceHostBase.ChannelDispatchers)
+            {
+                dispatcher.ErrorHandlers.Add(new ErrorHandler(this.handle, this.action));
+            }
+        }
+
+        public void Validate(ServiceDescription serviceDescription, ServiceHostBase serviceHostBase)
+        {
+        }
+    }
+}
+#endif

test/OpenTelemetry.Contrib.Instrumentation.Wcf.Tests/WCF/IServiceContract.cs

@@ -27,5 +27,8 @@ public interface IServiceContract
 
         [OperationContract(Action = "")]
         Task<ServiceResponse> ExecuteWithEmptyActionNameAsync(ServiceRequest request);
+
+        [OperationContract(IsOneWay = true)]
+        void ExecuteWithOneWay(ServiceRequest request);
     }
 }

test/OpenTelemetry.Contrib.Instrumentation.Wcf.Tests/WCF/Service.netfx.cs

@@ -44,6 +44,10 @@ public Task<ServiceResponse> ExecuteWithEmptyActionNameAsync(ServiceRequest requ
                    Payload = $"RSP: {request.Payload}",
                });
         }
+
+        public void ExecuteWithOneWay(ServiceRequest request)
+        {
+        }
     }
 }
 #endif

test/OpenTelemetry.Contrib.Instrumentation.Wcf.Tests/WCF/ServiceClient.cs

@@ -32,5 +32,8 @@ public Task<ServiceResponse> ExecuteAsync(ServiceRequest request)
 
         public Task<ServiceResponse> ExecuteWithEmptyActionNameAsync(ServiceRequest request)
             => this.Channel.ExecuteWithEmptyActionNameAsync(request);
+
+        public void ExecuteWithOneWay(ServiceRequest request)
+            => this.Channel.ExecuteWithOneWay(request);
     }
 }