open-telemetry
/
opentelemetry-dotnet
mirror of https://github.com/open-telemetry/opentelemetry-dotnet.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Don't throw when checking if an outbound HttpRequestMessage request is injected with the traceparent header (#869) 

* If an outbound HttpRequestMessage does not include the traceparent header then don't throw.

Made the header values getters and setter static to avoid the Func and Action allocations per invocation.

* Missed a getter usage in HttpInListener

* Extending an existing unit test to validate the new HttpRequestMessage header value getter.

Co-authored-by: Cijo Thomas <cithomas@microsoft.com>
This commit is contained in:
Peter Wiese 2020-07-21 17:13:33 -07:00 committed by GitHub
parent 609c57c31f
commit c1dedf0589
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 55 additions and 16 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
src/OpenTelemetry.Instrumentation.AspNet/Implementation/HttpInListener.cs
View File

@ -15,6 +15,7 @@
// </copyright>
using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.Web;
using System.Web.Routing;
@ -26,6 +27,7 @@ namespace OpenTelemetry.Instrumentation.AspNet.Implementation
internal class HttpInListener : ListenerHandler
{
private static readonly string ActivityNameByHttpInListener = "ActivityCreatedByHttpInListener";
private static readonly Func<HttpRequest, string, IEnumerable<string>> HttpRequestHeaderValuesGetter = (request, name) => request.Headers.GetValues(name);
private readonly PropertyFetcher routeFetcher = new PropertyFetcher("Route");
private readonly PropertyFetcher routeTemplateFetcher = new PropertyFetcher("RouteTemplate");
private readonly AspNetInstrumentationOptions options;
@ -61,9 +63,7 @@ namespace OpenTelemetry.Instrumentation.AspNet.Implementation
{
// This requires to ignore the current activity and create a new one
// using the context extracted using the format TextFormat supports.
var ctx = this.options.TextFormat.Extract<HttpRequest>(
request,
(r, name) => requestValues.Headers.GetValues(name));
var ctx = this.options.TextFormat.Extract(request, HttpRequestHeaderValuesGetter);
// Create a new activity with its parent set from the extracted context.
// This makes the new activity as a "sibling" of the activity created by

6
src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInListener.cs
View File

@ -15,6 +15,7 @@
// </copyright>
using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.Linq;
using System.Text;
@ -29,6 +30,7 @@ namespace OpenTelemetry.Instrumentation.AspNetCore.Implementation
{
private static readonly string UnknownHostName = "UNKNOWN-HOST";
private static readonly string ActivityNameByHttpInListener = "ActivityCreatedByHttpInListener";
private static readonly Func<HttpRequest, string, IEnumerable<string>> HttpRequestHeaderValuesGetter = (request, name) => request.Headers[name];
private readonly PropertyFetcher startContextFetcher = new PropertyFetcher("HttpContext");
private readonly PropertyFetcher stopContextFetcher = new PropertyFetcher("HttpContext");
private readonly PropertyFetcher beforeActionActionDescriptorFetcher = new PropertyFetcher("actionDescriptor");
@ -70,9 +72,7 @@ namespace OpenTelemetry.Instrumentation.AspNetCore.Implementation
// using the context extracted from w3ctraceparent header or
// using the format TextFormat supports.
var ctx = this.options.TextFormat.Extract<HttpRequest>(
request,
(r, name) => r.Headers[name]);
var ctx = this.options.TextFormat.Extract(request, HttpRequestHeaderValuesGetter);
// Create a new activity with its parent set from the extracted context.
// This makes the new activity as a "sibling" of the activity created by

17
src/OpenTelemetry.Instrumentation.Dependencies/Implementation/HttpHandlerDiagnosticListener.cs
View File

@ -14,6 +14,7 @@
// limitations under the License.
// </copyright>
using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.Net.Http;
using System.Net.Sockets;
@ -28,6 +29,18 @@ namespace OpenTelemetry.Instrumentation.Dependencies.Implementation
{
internal class HttpHandlerDiagnosticListener : ListenerHandler
{
private static readonly Func<HttpRequestMessage, string, IEnumerable<string>> HttpRequestMessageHeaderValuesGetter = (request, name) =>
{
if (request.Headers.TryGetValues(name, out var values))
{
return values;
}
return null;
};
private static readonly Action<HttpRequestMessage, string, string> HttpRequestMessageHeaderValueSetter = (request, name, value) => request.Headers.Add(name, value);
private static readonly Regex CoreAppMajorVersionCheckRegex = new Regex("^\\.NETCoreApp,Version=v(\\d+)\\.", RegexOptions.Compiled | RegexOptions.IgnoreCase);
private readonly ActivitySourceAdapter activitySource;
@ -68,7 +81,7 @@ namespace OpenTelemetry.Instrumentation.Dependencies.Implementation
return;
}
if (this.options.TextFormat.IsInjected(request, (r, h) => r.Headers.GetValues(h)))
if (this.options.TextFormat.IsInjected(request, HttpRequestMessageHeaderValuesGetter))
{
// this request is already instrumented, we should back off
activity.IsAllDataRequested = false;
@ -94,7 +107,7 @@ namespace OpenTelemetry.Instrumentation.Dependencies.Implementation
if (!(this.httpClientSupportsW3C && this.options.TextFormat is TraceContextFormatActivity))
{
this.options.TextFormat.Inject(activity.Context, request, (r, k, v) => r.Headers.Add(k, v));
this.options.TextFormat.Inject(activity.Context, request, HttpRequestMessageHeaderValueSetter);
}
}

6
src/OpenTelemetry.Instrumentation.Dependencies/Implementation/HttpTagHelper.cs
View File

@ -35,7 +35,7 @@ namespace OpenTelemetry.Instrumentation.Dependencies.Implementation
private static readonly Func<string, string> ConvertMethodToOperationNameRef = ConvertMethodToOperationName;
private static readonly Func<HttpMethod, string> ConvertHttpMethodToOperationNameRef = ConvertHttpMethodToOperationName;
private static readonly Func<HttpMethod, string> ConvertHttpMethodToNameRef = ConvertHttpMethodToName;
private static readonly Func<Version, string> ConvertConvertProtcolVersionToStringRef = ConvertProtcolVersionToString;
private static readonly Func<Version, string> ConvertProtocolVersionToStringRef = ConvertProtocolVersionToString;
private static readonly Func<HttpStatusCode, string> ConvertHttpStatusCodeToStringRef = ConvertHttpStatusCodeToString;
/// <summary>
@ -64,7 +64,7 @@ namespace OpenTelemetry.Instrumentation.Dependencies.Implementation
/// </summary>
/// <param name="protocolVersion"><see cref="Version"/>.</param>
/// <returns>Span flavor value.</returns>
public static string GetFlavorTagValueFromProtocolVersion(Version protocolVersion) => ProtocolVersionToStringCache.GetOrAdd(protocolVersion, ConvertConvertProtcolVersionToStringRef);
public static string GetFlavorTagValueFromProtocolVersion(Version protocolVersion) => ProtocolVersionToStringCache.GetOrAdd(protocolVersion, ConvertProtocolVersionToStringRef);
/// <summary>
/// Gets the OpenTelemetry standard status code tag value for a span based on its protocol <see cref="HttpStatusCode"/>.
@ -112,6 +112,6 @@ namespace OpenTelemetry.Instrumentation.Dependencies.Implementation
private static string ConvertHttpStatusCodeToString(HttpStatusCode statusCode) => ((int)statusCode).ToString();
private static string ConvertProtcolVersionToString(Version protocolVersion) => protocolVersion.ToString();
private static string ConvertProtocolVersionToString(Version protocolVersion) => protocolVersion.ToString();
}
}

8
src/OpenTelemetry.Instrumentation.Dependencies/Implementation/HttpWebRequestActivitySource.netfx.cs
View File

@ -18,6 +18,7 @@ using System;
using System.Collections;
using System.Collections.Generic;
using System.Diagnostics;
using System.Linq;
using System.Net;
using System.Reflection;
using System.Reflection.Emit;
@ -39,6 +40,9 @@ namespace OpenTelemetry.Instrumentation.Dependencies.Implementation
internal const string ActivitySourceName = "OpenTelemetry.HttpWebRequest";
internal const string ActivityName = ActivitySourceName + ".HttpRequestOut";
internal static readonly Func<HttpWebRequest, string, IEnumerable<string>> HttpWebRequestHeaderValuesGetter = (request, name) => request.Headers.GetValues(name);
internal static readonly Action<HttpWebRequest, string, string> HttpWebRequestHeaderValuesSetter = (request, name, value) => request.Headers.Add(name, value);
internal static HttpWebRequestInstrumentationOptions Options = new HttpWebRequestInstrumentationOptions();
private const string CorrelationContextHeaderName = "Correlation-Context";
@ -186,7 +190,7 @@ namespace OpenTelemetry.Instrumentation.Dependencies.Implementation
[MethodImpl(MethodImplOptions.AggressiveInlining)]
private static void InstrumentRequest(HttpWebRequest request, Activity activity)
{
Options.TextFormat.Inject(activity.Context, request, (r, k, v) => r.Headers.Add(k, v));
Options.TextFormat.Inject(activity.Context, request, HttpWebRequestHeaderValuesSetter);
if (request.Headers.Get(CorrelationContextHeaderName) == null)
{
@ -210,7 +214,7 @@ namespace OpenTelemetry.Instrumentation.Dependencies.Implementation
[MethodImpl(MethodImplOptions.AggressiveInlining)]
private static bool IsRequestInstrumented(HttpWebRequest request)
=> Options.TextFormat.IsInjected(request, (r, h) => r.Headers.GetValues(h));
=> Options.TextFormat.IsInjected(request, HttpWebRequestHeaderValuesGetter);
private static void ProcessRequest(HttpWebRequest request)
{

28
test/OpenTelemetry.Instrumentation.Dependencies.Tests/HttpClientTests.Basic.netcore31.cs
View File

@ -15,6 +15,7 @@
// </copyright>
#if NETCOREAPP3_1
using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.Linq;
using System.Net.Http;
@ -23,7 +24,6 @@ using System.Threading.Tasks;
using Moq;
using OpenTelemetry.Context.Propagation;
using OpenTelemetry.Internal.Test;
using OpenTelemetry.Trace;
using OpenTelemetry.Trace.Configuration;
using OpenTelemetry.Trace.Export;
using Xunit;
@ -72,8 +72,27 @@ namespace OpenTelemetry.Instrumentation.Dependencies.Tests
parent.TraceStateString = "k1=v1,k2=v2";
parent.ActivityTraceFlags = ActivityTraceFlags.Recorded;
// Ensure that the header value func does not throw if the header key can't be found
var mockTextFormat = new Mock<ITextFormatActivity>();
var isInjectedHeaderValueGetterThrows = false;
mockTextFormat
.Setup(x => x.IsInjected(It.IsAny<HttpRequestMessage>(), It.IsAny<Func<HttpRequestMessage, string, IEnumerable<string>>>()))
.Callback<HttpRequestMessage, Func<HttpRequestMessage, string, IEnumerable<string>>>(
(carrier, getter) =>
{
try
{
// traceparent doesn't exist
getter(carrier, "traceparent");
}
catch
{
isInjectedHeaderValueGetterThrows = true;
}
});
using (OpenTelemetrySdk.EnableOpenTelemetry(
(builder) => builder.AddHttpClientDependencyInstrumentation()
(builder) => builder.AddHttpClientDependencyInstrumentation(o => o.TextFormat = mockTextFormat.Object)
.AddProcessorPipeline(p => p.AddProcessor(n => spanProcessor.Object))))
{
using var c = new HttpClient();
@ -94,7 +113,10 @@ namespace OpenTelemetry.Instrumentation.Dependencies.Tests
Assert.Single(tracestates);
Assert.Equal($"00-{span.Context.TraceId}-{span.Context.SpanId}-01", traceparents.Single());
Assert.Equal("k1=v1,k2=v2", tracestates.Single());
Assert.Equal("k1=v1,k2=v2", tracestates.Single());
mockTextFormat.Verify(x => x.IsInjected(It.IsAny<HttpRequestMessage>(), It.IsAny<Func<HttpRequestMessage, string, IEnumerable<string>>>()), Times.Once);
Assert.False(isInjectedHeaderValueGetterThrows);
}
[Fact]