Skip to content

Commit

Permalink
WCF: action is assigned with null if operation name is not defined(op…
Browse files Browse the repository at this point in the history 
…en-telemetry#169) (open-telemetry#170)

* action is assigned with null if operation name is not defined(open-telemetry#169)

* adds unit test

* Code review + changelog update.

Co-authored-by: Mikel Blanchard <[email protected]>
  • Loading branch information
@tamilaban @CodeBlanch
tamilaban and CodeBlanch authored Feb 5, 2022
1 parent fa906bd commit 667fc49
Show file tree
Hide file tree
Showing 8 changed files with 118 additions and 20 deletions.
6 changes: 6 additions & 0 deletions src/OpenTelemetry.Contrib.Instrumentation.Wcf/CHANGELOG.md
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
# Changelog

## 1.0.0-rc5

* Fixed an `ArgumentNullException` setting `Activity`.`DisplayName` when
processing service requests with empty actions
([#170](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/170))

## 1.0.0-rc4

* Removed `Propagator` property on `WcfInstrumentationOptions`
Expand Down
12 changes: 10 additions & 2 deletions src/OpenTelemetry.Contrib.Instrumentation.Wcf/TelemetryClientMessageInspector.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,16 @@ public object BeforeSendRequest(ref Message request, IClientChannel channel)

if (activity != null)
{
string action = request.Headers.Action;
activity.DisplayName = action;
string action;
if (!string.IsNullOrEmpty(request.Headers.Action))
{
action = request.Headers.Action;
activity.DisplayName = action;
}
else
{
action = string.Empty;
}

Propagators.DefaultTextMapPropagator.Inject(
new PropagationContext(activity.Context, Baggage.Current),
Expand Down
12 changes: 10 additions & 2 deletions src/OpenTelemetry.Contrib.Instrumentation.Wcf/TelemetryDispatchMessageInspector.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,16 @@ public object AfterReceiveRequest(ref Message request, IClientChannel channel, I

if (activity != null)
{
string action = request.Headers.Action;
activity.DisplayName = action;
string action;
if (!string.IsNullOrEmpty(request.Headers.Action))
{
action = request.Headers.Action;
activity.DisplayName = action;
}
else
{
action = string.Empty;
}

if (activity.IsAllDataRequested)
{
Expand Down
51 changes: 43 additions & 8 deletions test/OpenTelemetry.Contrib.Instrumentation.Wcf.Tests/TelemetryClientMessageInspectorTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,12 +89,23 @@ async void Listener()

var ctx = await ctxTask.ConfigureAwait(false);

using StreamReader reader = new StreamReader(ctx.Request.InputStream);

string request = reader.ReadToEnd();

ctx.Response.StatusCode = 200;
ctx.Response.ContentType = "text/xml; charset=utf-8";

using (StreamWriter writer = new StreamWriter(ctx.Response.OutputStream))
{
writer.Write(@"<s:Envelope xmlns:s=""http://schemas.xmlsoap.org/soap/envelope/""><s:Body><ExecuteResponse xmlns=""http://opentelemetry.io/""><ExecuteResult xmlns:a=""http://schemas.datacontract.org/2004/07/OpenTelemetry.Contrib.Instrumentation.Wcf.Tests"" xmlns:i=""http://www.w3.org/2001/XMLSchema-instance""><a:Payload>RSP: Hello Open Telemetry!</a:Payload></ExecuteResult></ExecuteResponse></s:Body></s:Envelope>");
if (request.Contains("ExecuteWithEmptyActionName"))
{
writer.Write(@"<s:Envelope xmlns:s=""http://schemas.xmlsoap.org/soap/envelope/""><s:Body><ExecuteWithEmptyActionNameResponse xmlns=""http://opentelemetry.io/""><ExecuteResult xmlns:a=""http://schemas.datacontract.org/2004/07/OpenTelemetry.Contrib.Instrumentation.Wcf.Tests"" xmlns:i=""http://www.w3.org/2001/XMLSchema-instance""><a:Payload>RSP: Hello Open Telemetry!</a:Payload></ExecuteResult></ExecuteWithEmptyActionNameResponse></s:Body></s:Envelope>");
}
else
{
writer.Write(@"<s:Envelope xmlns:s=""http://schemas.xmlsoap.org/soap/envelope/""><s:Body><ExecuteResponse xmlns=""http://opentelemetry.io/""><ExecuteResult xmlns:a=""http://schemas.datacontract.org/2004/07/OpenTelemetry.Contrib.Instrumentation.Wcf.Tests"" xmlns:i=""http://www.w3.org/2001/XMLSchema-instance""><a:Payload>RSP: Hello Open Telemetry!</a:Payload></ExecuteResult></ExecuteResponse></s:Body></s:Envelope>");
}
}

ctx.Response.Close();
Expand Down Expand Up @@ -128,13 +139,15 @@ public void Dispose()
[InlineData(true, false, true, true)]
[InlineData(true, false, true, true, true)]
[InlineData(true, false, true, true, true, true)]
[InlineData(true, false, true, true, true, true, true)]
public async Task OutgoingRequestInstrumentationTest(
bool instrument,
bool filter = false,
bool suppressDownstreamInstrumentation = true,
bool includeVersion = false,
bool enrich = false,
bool enrichmentException = false)
bool enrichmentException = false,
bool emptyOrNullAction = false)
{
#if NETFRAMEWORK
const string OutgoingHttpOperationName = "OpenTelemetry.HttpWebRequest.HttpRequestOut";
Expand Down Expand Up @@ -190,11 +203,22 @@ public async Task OutgoingRequestInstrumentationTest(
{
client.Endpoint.EndpointBehaviors.Add(new TelemetryEndpointBehavior());

var response = await client.ExecuteAsync(
new ServiceRequest
{
Payload = "Hello Open Telemetry!",
}).ConfigureAwait(false);
if (emptyOrNullAction)
{
await client.ExecuteWithEmptyActionNameAsync(
new ServiceRequest
{
Payload = "Hello Open Telemetry!",
}).ConfigureAwait(false);
}
else
{
await client.ExecuteAsync(
new ServiceRequest
{
Payload = "Hello Open Telemetry!",
}).ConfigureAwait(false);
}
}
finally
{
Expand Down Expand Up @@ -238,10 +262,21 @@ public async Task OutgoingRequestInstrumentationTest(
Assert.Single(stoppedActivities);

Activity activity = stoppedActivities[0];

if (emptyOrNullAction)
{
Assert.Equal(WcfInstrumentationActivitySource.OutgoingRequestActivityName, activity.DisplayName);
Assert.Equal("ExecuteWithEmptyActionName", activity.TagObjects.FirstOrDefault(t => t.Key == WcfInstrumentationConstants.RpcMethodTag).Value);
}
else
{
Assert.Equal("http://opentelemetry.io/Service/Execute", activity.DisplayName);
Assert.Equal("Execute", activity.TagObjects.FirstOrDefault(t => t.Key == WcfInstrumentationConstants.RpcMethodTag).Value);
}

Assert.Equal(WcfInstrumentationActivitySource.OutgoingRequestActivityName, 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("Execute", activity.TagObjects.FirstOrDefault(t => t.Key == WcfInstrumentationConstants.RpcMethodTag).Value);
Assert.Equal(this.serviceBaseUri.Host, activity.TagObjects.FirstOrDefault(t => t.Key == WcfInstrumentationConstants.NetPeerNameTag).Value);
Assert.Equal(this.serviceBaseUri.Port, activity.TagObjects.FirstOrDefault(t => t.Key == WcfInstrumentationConstants.NetPeerPortTag).Value);
Assert.Equal("http", activity.TagObjects.FirstOrDefault(t => t.Key == WcfInstrumentationConstants.WcfChannelSchemeTag).Value);
Expand Down
42 changes: 34 additions & 8 deletions ...lemetry.Contrib.Instrumentation.Wcf.Tests/TelemetryDispatchMessageInspectorTests.netfx.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,12 +89,14 @@ public void Dispose()
[InlineData(true, false, true)]
[InlineData(true, false, true, true)]
[InlineData(true, false, true, true, true)]
[InlineData(true, false, true, true, true, true)]
public async Task IncomingRequestInstrumentationTest(
bool instrument,
bool filter = false,
bool includeVersion = false,
bool enrich = false,
bool enrichmentExcecption = false)
bool enrichmentExcecption = false,
bool emptyOrNullAction = false)
{
List<Activity> stoppedActivities = new List<Activity>();

Expand Down Expand Up @@ -156,11 +158,22 @@ public async Task IncomingRequestInstrumentationTest(
new EndpointAddress(new Uri(this.serviceBaseUri, "/Service")));
try
{
var response = await client.ExecuteAsync(
new ServiceRequest
{
Payload = "Hello Open Telemetry!",
}).ConfigureAwait(false);
if (emptyOrNullAction)
{
await client.ExecuteWithEmptyActionNameAsync(
new ServiceRequest
{
Payload = "Hello Open Telemetry!",
}).ConfigureAwait(false);
}
else
{
await client.ExecuteAsync(
new ServiceRequest
{
Payload = "Hello Open Telemetry!",
}).ConfigureAwait(false);
}
}
finally
{
Expand All @@ -185,15 +198,28 @@ public async Task IncomingRequestInstrumentationTest(
Assert.Single(stoppedActivities);

Activity activity = stoppedActivities[0];

if (emptyOrNullAction)
{
Assert.Equal(WcfInstrumentationActivitySource.IncomingRequestActivityName, activity.DisplayName);
Assert.Equal("ExecuteWithEmptyActionName", activity.TagObjects.FirstOrDefault(t => t.Key == WcfInstrumentationConstants.RpcMethodTag).Value);
Assert.Equal("http://opentelemetry.io/Service/ExecuteWithEmptyActionNameResponse", activity.TagObjects.FirstOrDefault(t => t.Key == WcfInstrumentationConstants.SoapReplyActionTag).Value);
}
else
{
Assert.Equal("http://opentelemetry.io/Service/Execute", activity.DisplayName);
Assert.Equal("Execute", activity.TagObjects.FirstOrDefault(t => t.Key == WcfInstrumentationConstants.RpcMethodTag).Value);
Assert.Equal("http://opentelemetry.io/Service/ExecuteResponse", activity.TagObjects.FirstOrDefault(t => t.Key == WcfInstrumentationConstants.SoapReplyActionTag).Value);
}

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("Execute", activity.TagObjects.FirstOrDefault(t => t.Key == WcfInstrumentationConstants.RpcMethodTag).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);
Assert.Equal("http://opentelemetry.io/Service/ExecuteResponse", activity.TagObjects.FirstOrDefault(t => t.Key == WcfInstrumentationConstants.SoapReplyActionTag).Value);

if (includeVersion)
{
Assert.Equal("Soap12 (http://www.w3.org/2003/05/soap-envelope) Addressing10 (http://www.w3.org/2005/08/addressing)", activity.TagObjects.FirstOrDefault(t => t.Key == WcfInstrumentationConstants.SoapMessageVersionTag).Value);
Expand Down
3 changes: 3 additions & 0 deletions test/OpenTelemetry.Contrib.Instrumentation.Wcf.Tests/WCF/IServiceContract.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,5 +24,8 @@ public interface IServiceContract
{
[OperationContract]
Task<ServiceResponse> ExecuteAsync(ServiceRequest request);

[OperationContract(Action = "")]
Task<ServiceResponse> ExecuteWithEmptyActionNameAsync(ServiceRequest request);
}
}
9 changes: 9 additions & 0 deletions test/OpenTelemetry.Contrib.Instrumentation.Wcf.Tests/WCF/Service.netfx.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,15 @@ public Task<ServiceResponse> ExecuteAsync(ServiceRequest request)
Payload = $"RSP: {request.Payload}",
});
}

public Task<ServiceResponse> ExecuteWithEmptyActionNameAsync(ServiceRequest request)
{
return Task.FromResult(
new ServiceResponse
{
Payload = $"RSP: {request.Payload}",
});
}
}
}
#endif
3 changes: 3 additions & 0 deletions test/OpenTelemetry.Contrib.Instrumentation.Wcf.Tests/WCF/ServiceClient.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,5 +29,8 @@ public ServiceClient(Binding binding, EndpointAddress remoteAddress)

public Task<ServiceResponse> ExecuteAsync(ServiceRequest request)
=> this.Channel.ExecuteAsync(request);

public Task<ServiceResponse> ExecuteWithEmptyActionNameAsync(ServiceRequest request)
=> this.Channel.ExecuteWithEmptyActionNameAsync(request);
}
}

0 comments on commit 667fc49

Please sign in to comment.