Added options to HttpWebRequest instrumentation. (#741)

* Added options to HttpWebRequest instrumentation.

* Cleaned up some usings.

* Added IsInjected to ITextFormatActivity.

* Switched the Http instrumentation filter back to internal so we can move ahead with the other stuff while the API is reviewed.

* Updated HttpClient instrumentation to check IsInjected instead of traceparent header directly.

* FilterFunc is back to public. Removed ctors from http options.

* Removed ctor that isn't needed.

Co-authored-by: Cijo Thomas <cithomas@microsoft.com>
This commit is contained in:
Mikel Blanchard 2020-07-19 18:51:14 -07:00 committed by GitHub
parent dfd965a374
commit c294cd7f76
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
14 changed files with 218 additions and 115 deletions

View File

@ -49,5 +49,14 @@ namespace OpenTelemetry.Context.Propagation
/// <param name="getter">Function that will return string value of a key with the specified name.</param>
/// <returns>Activity context from it's text representation.</returns>
ActivityContext Extract<T>(T carrier, Func<T, string, IEnumerable<string>> getter);
/// <summary>
/// Tests if an activity context has been injected into a carrier.
/// </summary>
/// <typeparam name="T">Type of object to extract context from. Typically HttpRequest or similar.</typeparam>
/// <param name="carrier">Object to extract context from. Instance of this object will be passed to the getter.</param>
/// <param name="getter">Function that will return string value of a key with the specified name.</param>
/// <returns><see langword="true" /> if the carrier has been injected with an activity context.</returns>
bool IsInjected<T>(T carrier, Func<T, string, IEnumerable<string>> getter);
}
}

View File

@ -42,9 +42,52 @@ namespace OpenTelemetry.Context.Propagation
/// <inheritdoc/>
public ISet<string> Fields => new HashSet<string> { TraceState, TraceParent };
/// <inheritdoc/>
public bool IsInjected<T>(T carrier, Func<T, string, IEnumerable<string>> getter)
{
if (carrier == null)
{
OpenTelemetryApiEventSource.Log.FailedToInjectActivityContext("null carrier");
return false;
}
if (getter == null)
{
OpenTelemetryApiEventSource.Log.FailedToExtractContext("null getter");
return false;
}
try
{
var traceparentCollection = getter(carrier, TraceParent);
// There must be a single traceparent
return traceparentCollection != null && traceparentCollection.Count() == 1;
}
catch (Exception ex)
{
OpenTelemetryApiEventSource.Log.ActivityContextExtractException(ex);
}
// in case of exception indicate to upstream that there is no parseable context from the top
return false;
}
/// <inheritdoc/>
public ActivityContext Extract<T>(T carrier, Func<T, string, IEnumerable<string>> getter)
{
if (carrier == null)
{
OpenTelemetryApiEventSource.Log.FailedToInjectActivityContext("null carrier");
return default;
}
if (getter == null)
{
OpenTelemetryApiEventSource.Log.FailedToExtractContext("null getter");
return default;
}
try
{
var traceparentCollection = getter(carrier, TraceParent);

View File

@ -128,6 +128,12 @@ namespace OpenTelemetry.Internal
this.WriteEvent(9, error);
}
[Event(10, Message = "Failed to extract span context: '{0}'", Level = EventLevel.Warning)]
public void FailedToExtractContext(string error)
{
this.WriteEvent(10, error);
}
/// <summary>
/// Returns a culture-independent string representation of the given <paramref name="exception"/> object,
/// appropriate for diagnostics tracing.

View File

@ -26,15 +26,6 @@ namespace OpenTelemetry.Instrumentation.Dependencies
{
private readonly DiagnosticSourceSubscriber diagnosticSourceSubscriber;
/// <summary>
/// Initializes a new instance of the <see cref="HttpClientInstrumentation"/> class.
/// </summary>
/// <param name="activitySource">ActivitySource adapter instance.</param>
public HttpClientInstrumentation(ActivitySourceAdapter activitySource)
: this(activitySource, new HttpClientInstrumentationOptions())
{
}
/// <summary>
/// Initializes a new instance of the <see cref="HttpClientInstrumentation"/> class.
/// </summary>
@ -42,7 +33,7 @@ namespace OpenTelemetry.Instrumentation.Dependencies
/// <param name="options">Configuration options for dependencies instrumentation.</param>
public HttpClientInstrumentation(ActivitySourceAdapter activitySource, HttpClientInstrumentationOptions options)
{
this.diagnosticSourceSubscriber = new DiagnosticSourceSubscriber(new HttpHandlerDiagnosticListener(options, activitySource), options.EventFilter);
this.diagnosticSourceSubscriber = new DiagnosticSourceSubscriber(new HttpHandlerDiagnosticListener(options, activitySource), (activitySource, arg1, arg2) => options?.EventFilter(activitySource, arg1) ?? true);
this.diagnosticSourceSubscriber.Subscribe();
}

View File

@ -24,25 +24,6 @@ namespace OpenTelemetry.Instrumentation.Dependencies
/// </summary>
public class HttpClientInstrumentationOptions
{
/// <summary>
/// Initializes a new instance of the <see cref="HttpClientInstrumentationOptions"/> class.
/// </summary>
public HttpClientInstrumentationOptions()
{
this.EventFilter = DefaultFilter;
}
/// <summary>
/// Initializes a new instance of the <see cref="HttpClientInstrumentationOptions"/> class.
/// </summary>
/// <param name="eventFilter">Custom filtering predicate for DiagnosticSource events, if any.</param>
internal HttpClientInstrumentationOptions(Func<string, object, object, bool> eventFilter)
{
// TODO This API is unusable and likely to change, let's not expose it for now.
this.EventFilter = eventFilter;
}
/// <summary>
/// Gets or sets a value indicating whether add HTTP version to a trace.
/// </summary>
@ -54,54 +35,55 @@ namespace OpenTelemetry.Instrumentation.Dependencies
public ITextFormatActivity TextFormat { get; set; } = new TraceContextFormatActivity();
/// <summary>
/// Gets or sets a hook to exclude calls based on domain or other per-request criterion.
/// Gets or sets an optional callback method for filtering HttpClient requests that are sent through the instrumentation.
/// </summary>
internal Func<string, object, object, bool> EventFilter { get; set; }
public Func<HttpRequestMessage, bool> FilterFunc { get; set; }
private static bool DefaultFilter(string activityName, object arg1, object unused)
internal static bool IsInternalUrl(Uri requestUri)
{
// TODO: there is some preliminary consensus that we should introduce 'terminal' spans or context.
// exporters should ensure they set it
var originalString = requestUri.OriginalString;
if (IsHttpOutgoingPostRequest(activityName, arg1, out Uri requestUri))
// zipkin
if (originalString.Contains(":9411/api/v2/spans"))
{
var originalString = requestUri.OriginalString;
return true;
}
// zipkin
if (originalString.Contains(":9411/api/v2/spans"))
// applicationinsights
if (originalString.StartsWith("https://dc.services.visualstudio") ||
originalString.StartsWith("https://rt.services.visualstudio") ||
originalString.StartsWith("https://dc.applicationinsights") ||
originalString.StartsWith("https://live.applicationinsights") ||
originalString.StartsWith("https://quickpulse.applicationinsights"))
{
return true;
}
return false;
}
internal bool EventFilter(string activityName, object arg1)
{
if (TryParseHttpRequestMessage(activityName, arg1, out HttpRequestMessage requestMessage))
{
Uri requestUri;
if (requestMessage.Method == HttpMethod.Post && (requestUri = requestMessage.RequestUri) != null)
{
return false;
if (IsInternalUrl(requestUri))
{
return false;
}
}
// applicationinsights
if (originalString.StartsWith("https://dc.services.visualstudio") ||
originalString.StartsWith("https://rt.services.visualstudio") ||
originalString.StartsWith("https://dc.applicationinsights") ||
originalString.StartsWith("https://live.applicationinsights") ||
originalString.StartsWith("https://quickpulse.applicationinsights"))
{
return false;
}
return this.FilterFunc?.Invoke(requestMessage) ?? true;
}
return true;
}
private static bool IsHttpOutgoingPostRequest(string activityName, object arg1, out Uri requestUri)
private static bool TryParseHttpRequestMessage(string activityName, object arg1, out HttpRequestMessage requestMessage)
{
if (activityName == "System.Net.Http.HttpRequestOut")
{
if (arg1 is HttpRequestMessage request &&
request.RequestUri != null &&
request.Method == HttpMethod.Post)
{
requestUri = request.RequestUri;
return true;
}
}
requestUri = null;
return false;
return (requestMessage = arg1 as HttpRequestMessage) != null && activityName == "System.Net.Http.HttpRequestOut";
}
}
}

View File

@ -0,0 +1,57 @@
// <copyright file="HttpWebRequestInstrumentationOptions.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.Net;
using OpenTelemetry.Context.Propagation;
namespace OpenTelemetry.Instrumentation.Dependencies
{
/// <summary>
/// Options for dependencies instrumentation.
/// </summary>
public class HttpWebRequestInstrumentationOptions
{
/// <summary>
/// Gets or sets a value indicating whether add HTTP version to a trace.
/// </summary>
public bool SetHttpFlavor { get; set; } = false;
/// <summary>
/// Gets or sets <see cref="ITextFormat"/> for context propagation.
/// </summary>
public ITextFormatActivity TextFormat { get; set; } = new TraceContextFormatActivity();
/// <summary>
/// Gets or sets an optional callback method for filtering HttpClient requests that are sent through the instrumentation.
/// </summary>
public Func<HttpWebRequest, bool> FilterFunc { get; set; }
internal bool EventFilter(HttpWebRequest request)
{
Uri requestUri;
if (request.Method == "POST"
&& (requestUri = request.RequestUri) != null
&& HttpClientInstrumentationOptions.IsInternalUrl(requestUri))
{
return false;
}
return this.FilterFunc?.Invoke(request) ?? true;
}
}
}
#endif

View File

@ -68,7 +68,7 @@ namespace OpenTelemetry.Instrumentation.Dependencies.Implementation
return;
}
if (request.Headers.Contains("traceparent"))
if (this.options.TextFormat.IsInjected(request, (r, h) => r.Headers.GetValues(h)))
{
// this request is already instrumented, we should back off
activity.IsAllDataRequested = false;

View File

@ -34,16 +34,14 @@ namespace OpenTelemetry.Instrumentation.Dependencies.Implementation
/// Inspired from the System.Diagnostics.DiagnosticSource.HttpHandlerDiagnosticListener class which has some bugs and feature gaps.
/// See https://github.com/dotnet/runtime/pull/33732 for details.
/// </remarks>
internal sealed class HttpWebRequestActivitySource
internal static class HttpWebRequestActivitySource
{
internal const string ActivitySourceName = "OpenTelemetry.HttpWebRequest";
internal const string ActivityName = ActivitySourceName + ".HttpRequestOut";
internal static readonly HttpWebRequestActivitySource Instance = new HttpWebRequestActivitySource();
internal static HttpWebRequestInstrumentationOptions Options = new HttpWebRequestInstrumentationOptions();
private const string CorrelationContextHeaderName = "Correlation-Context";
private const string TraceParentHeaderName = "traceparent";
private const string TraceStateHeaderName = "tracestate";
private static readonly Version Version = typeof(HttpWebRequestActivitySource).Assembly.GetName().Version;
private static readonly ActivitySource WebRequestActivitySource = new ActivitySource(ActivitySourceName, Version.ToString());
@ -76,7 +74,7 @@ namespace OpenTelemetry.Instrumentation.Dependencies.Implementation
private static Func<HttpWebResponse, bool> isWebSocketResponseAccessor;
private static Func<HttpWebResponse, string> connectionGroupNameAccessor;
internal HttpWebRequestActivitySource()
static HttpWebRequestActivitySource()
{
try
{
@ -104,7 +102,10 @@ namespace OpenTelemetry.Instrumentation.Dependencies.Implementation
activity.AddTag(SemanticConventions.AttributeHTTPMethod, request.Method);
activity.AddTag(SemanticConventions.AttributeHTTPHost, HttpTagHelper.GetHostTagValueFromRequestUri(request.RequestUri));
activity.AddTag(SemanticConventions.AttributeHTTPURL, request.RequestUri.OriginalString);
activity.AddTag(SemanticConventions.AttributeHTTPFlavor, HttpTagHelper.GetFlavorTagValueFromProtocolVersion(request.ProtocolVersion));
if (Options.SetHttpFlavor)
{
activity.AddTag(SemanticConventions.AttributeHTTPFlavor, HttpTagHelper.GetFlavorTagValueFromProtocolVersion(request.ProtocolVersion));
}
}
}
@ -185,18 +186,7 @@ namespace OpenTelemetry.Instrumentation.Dependencies.Implementation
[MethodImpl(MethodImplOptions.AggressiveInlining)]
private static void InstrumentRequest(HttpWebRequest request, Activity activity)
{
// do not inject header if it was injected already
// perhaps tracing systems wants to override it
if (request.Headers.Get(TraceParentHeaderName) == null)
{
request.Headers.Add(TraceParentHeaderName, activity.Id);
string traceState = activity.TraceStateString;
if (traceState != null)
{
request.Headers.Add(TraceStateHeaderName, traceState);
}
}
Options.TextFormat.Inject(activity.Context, request, (r, k, v) => r.Headers.Add(k, v));
if (request.Headers.Get(CorrelationContextHeaderName) == null)
{
@ -220,11 +210,11 @@ namespace OpenTelemetry.Instrumentation.Dependencies.Implementation
[MethodImpl(MethodImplOptions.AggressiveInlining)]
private static bool IsRequestInstrumented(HttpWebRequest request)
=> request.Headers.Get(TraceParentHeaderName) != null;
=> Options.TextFormat.IsInjected(request, (r, h) => r.Headers.GetValues(h));
private static void ProcessRequest(HttpWebRequest request)
{
if (!WebRequestActivitySource.HasListeners() || IsRequestInstrumented(request))
if (!WebRequestActivitySource.HasListeners() || IsRequestInstrumented(request) || !Options.EventFilter(request))
{
// No subscribers to the ActivitySource or this request was instrumented by previous
// ProcessRequest, such is the case with redirect responses where the same request is sent again.

View File

@ -119,15 +119,21 @@ namespace OpenTelemetry.Trace.Configuration
/// Enables the outgoing requests automatic data collection for .NET Framework HttpWebRequest activity source.
/// </summary>
/// <param name="builder"><see cref="OpenTelemetryBuilder"/> being configured.</param>
/// <param name="configureOptions">HttpWebRequest configuration options.</param>
/// <returns>The instance of <see cref="OpenTelemetryBuilder"/> to chain the calls.</returns>
public static OpenTelemetryBuilder AddHttpWebRequestDependencyInstrumentation(this OpenTelemetryBuilder builder)
public static OpenTelemetryBuilder AddHttpWebRequestDependencyInstrumentation(
this OpenTelemetryBuilder builder,
Action<HttpWebRequestInstrumentationOptions> configureOptions = null)
{
if (builder == null)
{
throw new ArgumentNullException(nameof(builder));
}
GC.KeepAlive(HttpWebRequestActivitySource.Instance);
HttpWebRequestInstrumentationOptions options = new HttpWebRequestInstrumentationOptions();
configureOptions?.Invoke(options);
HttpWebRequestActivitySource.Options = options;
builder.AddActivitySource(HttpWebRequestActivitySource.ActivitySourceName);

View File

@ -69,6 +69,43 @@ namespace OpenTelemetry.Context.Propagation
/// <inheritdoc/>
public ISet<string> Fields => AllFields;
/// <inheritdoc/>
public bool IsInjected<T>(T carrier, Func<T, string, IEnumerable<string>> getter)
{
if (carrier == null)
{
OpenTelemetrySdkEventSource.Log.FailedToExtractContext("null carrier");
return false;
}
if (getter == null)
{
OpenTelemetrySdkEventSource.Log.FailedToExtractContext("null getter");
return false;
}
try
{
if (this.singleHeader)
{
var header = getter(carrier, XB3Combined)?.FirstOrDefault();
return !string.IsNullOrWhiteSpace(header);
}
else
{
var traceIdStr = getter(carrier, XB3TraceId)?.FirstOrDefault();
var spanIdStr = getter(carrier, XB3SpanId)?.FirstOrDefault();
return traceIdStr != null && spanIdStr != null;
}
}
catch (Exception e)
{
OpenTelemetrySdkEventSource.Log.ContextExtractException(e);
return false;
}
}
/// <inheritdoc/>
public ActivityContext Extract<T>(T carrier, Func<T, string, IEnumerable<string>> getter)
{

View File

@ -215,9 +215,7 @@ namespace OpenTelemetry.Instrumentation.Dependencies.Tests
using (OpenTelemetrySdk.EnableOpenTelemetry(
(builder) =>
builder.AddHttpClientDependencyInstrumentation(
(opt) => opt.EventFilter = (activityName, arg1, _) => !(activityName == "System.Net.Http.HttpRequestOut" &&
arg1 is HttpRequestMessage request &&
request.RequestUri.OriginalString.Contains(this.url)))
(opt) => opt.FilterFunc = (req) => !req.RequestUri.OriginalString.Contains(this.url))
.AddProcessorPipeline(p => p.AddProcessor(n => spanProcessor.Object))))
{
using var c = new HttpClient();

View File

@ -40,7 +40,8 @@ namespace OpenTelemetry.Instrumentation.Dependencies.Tests
static HttpWebRequestActivitySourceTests()
{
GC.KeepAlive(HttpWebRequestActivitySource.Instance);
// Need to touch something in HttpWebRequestActivitySource to do the static injection.
GC.KeepAlive(HttpWebRequestActivitySource.Options);
}
public HttpWebRequestActivitySourceTests()
@ -893,7 +894,6 @@ namespace OpenTelemetry.Instrumentation.Dependencies.Tests
}
Assert.Equal(url, activity.Tags.FirstOrDefault(i => i.Key == SemanticConventions.AttributeHTTPURL).Value);
Assert.Equal("1.1", activity.Tags.FirstOrDefault(i => i.Key == SemanticConventions.AttributeHTTPFlavor).Value);
}
private static void VerifyActivityStopTags(string statusCode, string statusText, Activity activity)

View File

@ -98,13 +98,12 @@ namespace OpenTelemetry.Instrumentation.Dependencies.Tests
parent.Stop();
}
/* TBD: ActivitySource doesn't support custom format TraceIds.
[Fact]
public async Task HttpDependenciesInstrumentationInjectsHeadersAsync_CustomFormat()
{
var textFormat = new Mock<ITextFormat>();
textFormat.Setup(m => m.Inject<HttpWebRequest>(It.IsAny<SpanContext>(), It.IsAny<HttpWebRequest>(), It.IsAny<Action<HttpWebRequest, string, string>>()))
.Callback<SpanContext, HttpWebRequest, Action<HttpWebRequest, string, string>>((context, message, action) =>
var textFormat = new Mock<ITextFormatActivity>();
textFormat.Setup(m => m.Inject(It.IsAny<ActivityContext>(), It.IsAny<HttpWebRequest>(), It.IsAny<Action<HttpWebRequest, string, string>>()))
.Callback<ActivityContext, HttpWebRequest, Action<HttpWebRequest, string, string>>((context, message, action) =>
{
action(message, "custom_traceparent", $"00/{context.TraceId}/{context.SpanId}/01");
action(message, "custom_tracestate", Activity.Current.TraceStateString);
@ -114,7 +113,7 @@ namespace OpenTelemetry.Instrumentation.Dependencies.Tests
using var shutdownSignal = OpenTelemetrySdk.EnableOpenTelemetry(b =>
{
b.AddProcessorPipeline(c => c.AddProcessor(ap => activityProcessor.Object));
b.AddHttpWebRequestDependencyInstrumentation();
b.AddHttpWebRequestDependencyInstrumentation(options => options.TextFormat = textFormat.Object);
});
var request = (HttpWebRequest)WebRequest.Create(this.url);
@ -145,7 +144,7 @@ namespace OpenTelemetry.Instrumentation.Dependencies.Tests
Assert.Equal("k1=v1,k2=v2", tracestate);
parent.Stop();
}*/
}
[Fact]
public async Task HttpDependenciesInstrumentationBacksOffIfAlreadyInstrumented()
@ -171,21 +170,17 @@ namespace OpenTelemetry.Instrumentation.Dependencies.Tests
Assert.Equal(0, activityProcessor.Invocations.Count);
}
[Fact(Skip = "HttpWebRequest instrumentation has no way to pass options and no ability to filter now.")]
[Fact]
public async Task HttpDependenciesInstrumentationFiltersOutRequests()
{
var activityProcessor = new Mock<ActivityProcessor>();
using var shutdownSignal = OpenTelemetrySdk.EnableOpenTelemetry(b =>
{
b.AddProcessorPipeline(c => c.AddProcessor(ap => activityProcessor.Object));
b.AddHttpWebRequestDependencyInstrumentation();
b.AddHttpWebRequestDependencyInstrumentation(
c => c.FilterFunc = (req) => !req.RequestUri.OriginalString.Contains(this.url));
});
var options = new HttpClientInstrumentationOptions((activityName, arg1, _)
=> !(activityName == HttpWebRequestActivitySource.ActivityName &&
arg1 is HttpWebRequest request &&
request.RequestUri.OriginalString.Contains(this.url)));
using var c = new HttpClient();
await c.GetAsync(this.url);

View File

@ -53,7 +53,7 @@ namespace OpenTelemetry.Instrumentation.Dependencies.Tests
{
b.SetResource(expectedResource);
b.AddProcessorPipeline(c => c.AddProcessor(ap => activityProcessor.Object));
b.AddHttpWebRequestDependencyInstrumentation();
b.AddHttpWebRequestDependencyInstrumentation(options => options.SetHttpFlavor = tc.SetHttpFlavor);
});
tc.Url = HttpTestData.NormalizeValues(tc.Url, host, port);
@ -126,17 +126,6 @@ namespace OpenTelemetry.Instrumentation.Dependencies.Tests
{
if (!tc.SpanAttributes.TryGetValue(tag.Key, out string value))
{
if (tag.Key == "http.flavor")
{
// http.flavor is optional in .NET Core instrumentation but there is no way to pass that option to the new ActivitySource model so it always shows up here.
if (tc.SetHttpFlavor)
{
Assert.Equal(value, tag.Value);
}
continue;
}
if (tag.Key == SpanAttributeConstants.StatusCodeKey)
{
Assert.Equal(tc.SpanStatus, d[tag.Value]);