From dfc8e6d3f1c314391379d8a24447c9483d06b3d6 Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Thu, 12 Aug 2021 11:57:08 -0700 Subject: [PATCH 1/9] New design for TelemetryHttpModule using ActivitySource + OpenTelemetry.API. (#2249) --- .../.publicApi/net461/PublicAPI.Unshipped.txt | 15 +- .../ActivityExtensions.cs | 159 ---------- .../ActivityHelper.cs | 132 ++++---- .../AspNetTelemetryEventSource.cs | 81 ++--- .../Internal/BaseHeaderParser.cs | 81 ----- .../Internal/GenericHeaderParser.cs | 39 --- .../Internal/HeaderUtilities.cs | 59 ---- .../Internal/HttpHeaderParser.cs | 40 --- .../Internal/HttpParseResult.cs | 37 --- .../Internal/HttpRuleParser.cs | 281 ------------------ .../Internal/NameValueHeaderValue.cs | 134 --------- ...entation.AspNet.TelemetryHttpModule.csproj | 2 +- .../TelemetryHttpModule.cs | 52 +++- 13 files changed, 165 insertions(+), 947 deletions(-) delete mode 100644 src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/ActivityExtensions.cs delete mode 100644 src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/Internal/BaseHeaderParser.cs delete mode 100644 src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/Internal/GenericHeaderParser.cs delete mode 100644 src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/Internal/HeaderUtilities.cs delete mode 100644 src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/Internal/HttpHeaderParser.cs delete mode 100644 src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/Internal/HttpParseResult.cs delete mode 100644 src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/Internal/HttpRuleParser.cs delete mode 100644 src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/Internal/NameValueHeaderValue.cs diff --git a/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/.publicApi/net461/PublicAPI.Unshipped.txt b/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/.publicApi/net461/PublicAPI.Unshipped.txt index 7e00f288d20..04b7b15df9b 100644 --- a/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/.publicApi/net461/PublicAPI.Unshipped.txt +++ b/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/.publicApi/net461/PublicAPI.Unshipped.txt @@ -1,9 +1,14 @@ -OpenTelemetry.Instrumentation.AspNet.ActivityExtensions +const OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule.AspNetActivityName = "Microsoft.AspNet.HttpReqIn" -> string +const OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule.AspNetSourceName = "OpenTelemetry.Instrumentation.AspNet.Telemetry" -> string OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule.Dispose() -> void OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule.Init(System.Web.HttpApplication context) -> void -OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule.ParseHeaders.get -> bool -OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule.ParseHeaders.set -> void +OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule.OnExceptionCallback.get -> System.Action +OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule.OnExceptionCallback.set -> void +OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule.OnRequestStartedCallback.get -> System.Action +OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule.OnRequestStartedCallback.set -> void +OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule.OnRequestStoppedCallback.get -> System.Action +OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule.OnRequestStoppedCallback.set -> void OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule.TelemetryHttpModule() -> void -static OpenTelemetry.Instrumentation.AspNet.ActivityExtensions.Extract(this System.Diagnostics.Activity activity, System.Collections.Specialized.NameValueCollection requestHeaders) -> bool -static OpenTelemetry.Instrumentation.AspNet.ActivityExtensions.TryParse(this System.Diagnostics.Activity activity, System.Collections.Specialized.NameValueCollection requestHeaders) -> bool \ No newline at end of file +OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule.TextMapPropagator.get -> OpenTelemetry.Context.Propagation.TraceContextPropagator +OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule.TextMapPropagator.set -> void diff --git a/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/ActivityExtensions.cs b/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/ActivityExtensions.cs deleted file mode 100644 index e5a542c76fb..00000000000 --- a/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/ActivityExtensions.cs +++ /dev/null @@ -1,159 +0,0 @@ -// -// 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. -// - -using System; -using System.Collections.Specialized; -using System.ComponentModel; -using System.Diagnostics; - -namespace OpenTelemetry.Instrumentation.AspNet -{ - /// - /// Extensions of Activity class. - /// - [EditorBrowsable(EditorBrowsableState.Never)] - public static class ActivityExtensions - { - /// - /// Http header name to carry the Request Id: https://github.com/dotnet/corefx/blob/master/src/System.Diagnostics.DiagnosticSource/src/HttpCorrelationProtocol.md. - /// - internal const string RequestIdHeaderName = "Request-Id"; - - /// - /// Http header name to carry the traceparent: https://www.w3.org/TR/trace-context/. - /// - internal const string TraceparentHeaderName = "traceparent"; - - /// - /// Http header name to carry the tracestate: https://www.w3.org/TR/trace-context/. - /// - internal const string TracestateHeaderName = "tracestate"; - - /// - /// Http header name to carry the correlation context. - /// - internal const string CorrelationContextHeaderName = "Correlation-Context"; - - /// - /// Maximum length of Correlation-Context header value. - /// - internal const int MaxCorrelationContextLength = 1024; - - /// - /// Reads Request-Id and Correlation-Context headers and sets ParentId and Baggage on Activity. - /// - /// Instance of activity that has not been started yet. - /// Request headers collection. - /// true if request was parsed successfully, false - otherwise. - public static bool Extract(this Activity activity, NameValueCollection requestHeaders) - { - if (activity == null) - { - AspNetTelemetryEventSource.Log.ActvityExtractionError("activity is null"); - return false; - } - - if (activity.ParentId != null) - { - AspNetTelemetryEventSource.Log.ActvityExtractionError("ParentId is already set on activity"); - return false; - } - - if (activity.Id != null) - { - AspNetTelemetryEventSource.Log.ActvityExtractionError("Activity is already started"); - return false; - } - - var parents = requestHeaders.GetValues(TraceparentHeaderName); - if (parents == null || parents.Length == 0) - { - parents = requestHeaders.GetValues(RequestIdHeaderName); - } - - if (parents != null && parents.Length > 0 && !string.IsNullOrEmpty(parents[0])) - { - // there may be several Request-Id or traceparent headers, but we only read the first one - activity.SetParentId(parents[0]); - - var tracestates = requestHeaders.GetValues(TracestateHeaderName); - if (tracestates != null && tracestates.Length > 0) - { - if (tracestates.Length == 1 && !string.IsNullOrEmpty(tracestates[0])) - { - activity.TraceStateString = tracestates[0]; - } - else - { - activity.TraceStateString = string.Join(",", tracestates); - } - } - - // Header format - Correlation-Context: key1=value1, key2=value2 - var baggages = requestHeaders.GetValues(CorrelationContextHeaderName); - if (baggages != null) - { - int correlationContextLength = -1; - - // there may be several Correlation-Context header - foreach (var item in baggages) - { - if (correlationContextLength >= MaxCorrelationContextLength) - { - break; - } - - foreach (var pair in item.Split(',')) - { - correlationContextLength += pair.Length + 1; // pair and comma - - if (correlationContextLength >= MaxCorrelationContextLength) - { - break; - } - - if (NameValueHeaderValue.TryParse(pair, out NameValueHeaderValue baggageItem)) - { - activity.AddBaggage(baggageItem.Name, baggageItem.Value); - } - else - { - AspNetTelemetryEventSource.Log.HeaderParsingError(CorrelationContextHeaderName, pair); - } - } - } - } - - return true; - } - - return false; - } - - /// - /// Reads Request-Id and Correlation-Context headers and sets ParentId and Baggage on Activity. - /// - /// Instance of activity that has not been started yet. - /// Request headers collection. - /// true if request was parsed successfully, false - otherwise. - [Obsolete("Method is obsolete, use Extract method instead", true)] - [EditorBrowsable(EditorBrowsableState.Never)] - public static bool TryParse(this Activity activity, NameValueCollection requestHeaders) - { - return Extract(activity, requestHeaders); - } - } -} diff --git a/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/ActivityHelper.cs b/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/ActivityHelper.cs index 7826968996e..dd13ab6f353 100644 --- a/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/ActivityHelper.cs +++ b/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/ActivityHelper.cs @@ -16,8 +16,10 @@ using System; using System.Collections; +using System.Collections.Generic; using System.Diagnostics; using System.Web; +using OpenTelemetry.Context.Propagation; namespace OpenTelemetry.Instrumentation.AspNet { @@ -27,97 +29,105 @@ namespace OpenTelemetry.Instrumentation.AspNet internal static class ActivityHelper { /// - /// Listener name. + /// Key to store the activity in HttpContext. /// - public const string AspNetListenerName = "OpenTelemetry.Instrumentation.AspNet.Telemetry"; + public const string ActivityKey = "__AspnetActivity__"; - /// - /// Activity name for http request. - /// - public const string AspNetActivityName = "Microsoft.AspNet.HttpReqIn"; + private static readonly ActivitySource AspNetSource = new ActivitySource(TelemetryHttpModule.AspNetSourceName); + private static readonly Func> HttpRequestHeaderValuesGetter = (request, name) => request.Headers.GetValues(name); /// - /// Event name for the activity start event. + /// Creates root (first level) activity that describes incoming request. /// - public const string AspNetActivityStartName = "Microsoft.AspNet.HttpReqIn.Start"; + /// . + /// Current HttpContext. + /// Callback action. + /// New root activity. + public static Activity StartAspNetActivity(TextMapPropagator textMapPropagator, HttpContext context, Action onRequestStartedCallback) + { + PropagationContext propagationContext = textMapPropagator.Extract(default, context.Request, HttpRequestHeaderValuesGetter); - /// - /// Key to store the activity in HttpContext. - /// - public const string ActivityKey = "__AspnetActivity__"; + Activity activity = AspNetSource.CreateActivity(TelemetryHttpModule.AspNetActivityName, ActivityKind.Server, propagationContext.ActivityContext); - private static readonly DiagnosticListener AspNetListener = new DiagnosticListener(AspNetListenerName); + if (activity != null) + { + context.Items[ActivityKey] = activity; - private static readonly object EmptyPayload = new object(); + if (propagationContext.Baggage != default) + { + Baggage.Current = propagationContext.Baggage; + } + + try + { + onRequestStartedCallback?.Invoke(activity, context); + } + catch (Exception callbackEx) + { + AspNetTelemetryEventSource.Log.CallbackException(activity, "OnStarted", callbackEx); + } + + AspNetTelemetryEventSource.Log.ActivityStarted(activity); + } + + return activity; + } /// /// Stops the activity and notifies listeners about it. /// - /// HttpContext.Items. - public static void StopAspNetActivity(IDictionary contextItems) + /// Current HttpContext. + /// Callback action. + public static void StopAspNetActivity(HttpContext context, Action onRequestStoppedCallback) { + var contextItems = context.Items; var currentActivity = Activity.Current; Activity aspNetActivity = (Activity)contextItems[ActivityKey]; if (currentActivity != aspNetActivity) { Activity.Current = aspNetActivity; - currentActivity = aspNetActivity; } - if (currentActivity != null) + if (aspNetActivity != null) { - // stop Activity with Stop event - AspNetListener.StopActivity(currentActivity, EmptyPayload); + aspNetActivity.Stop(); contextItems[ActivityKey] = null; - } - - AspNetTelemetryEventSource.Log.ActivityStopped(currentActivity?.Id, currentActivity?.OperationName); - } - /// - /// Creates root (first level) activity that describes incoming request. - /// - /// Current HttpContext. - /// Determines if headers should be parsed get correlation ids. - /// New root activity. - public static Activity CreateRootActivity(HttpContext context, bool parseHeaders) - { - if (AspNetListener.IsEnabled() && AspNetListener.IsEnabled(AspNetActivityName)) - { - var rootActivity = new Activity(AspNetActivityName); - - if (parseHeaders) + try { - rootActivity.Extract(context.Request.Unvalidated.Headers); + onRequestStoppedCallback?.Invoke(aspNetActivity, context); } - - AspNetListener.OnActivityImport(rootActivity, null); - - if (StartAspNetActivity(rootActivity)) + catch (Exception callbackEx) { - context.Items[ActivityKey] = rootActivity; - AspNetTelemetryEventSource.Log.ActivityStarted(rootActivity.Id); - return rootActivity; + AspNetTelemetryEventSource.Log.CallbackException(aspNetActivity, "OnStopped", callbackEx); } + + AspNetTelemetryEventSource.Log.ActivityStopped(currentActivity); } - return null; + if (currentActivity != aspNetActivity) + { + Activity.Current = currentActivity; + } } - public static void WriteActivityException(IDictionary contextItems, Exception exception) + public static void WriteActivityException(IDictionary contextItems, Exception exception, Action onExceptionCallback) { Activity aspNetActivity = (Activity)contextItems[ActivityKey]; if (aspNetActivity != null) { - if (Activity.Current != aspNetActivity) + try { - Activity.Current = aspNetActivity; + onExceptionCallback?.Invoke(aspNetActivity, exception); + } + catch (Exception callbackEx) + { + AspNetTelemetryEventSource.Log.CallbackException(aspNetActivity, "OnException", callbackEx); } - AspNetListener.Write(aspNetActivity.OperationName + ".Exception", exception); - AspNetTelemetryEventSource.Log.ActivityException(aspNetActivity.Id, aspNetActivity.OperationName, exception); + AspNetTelemetryEventSource.Log.ActivityException(aspNetActivity, exception); } } @@ -136,27 +146,9 @@ internal static void RestoreActivityIfNeeded(IDictionary contextItems) if (aspNetActivity != null) { Activity.Current = aspNetActivity; + AspNetTelemetryEventSource.Log.ActivityRestored(aspNetActivity); } } } - - private static bool StartAspNetActivity(Activity activity) - { - if (AspNetListener.IsEnabled(AspNetActivityName, activity, EmptyPayload)) - { - if (AspNetListener.IsEnabled(AspNetActivityStartName)) - { - AspNetListener.StartActivity(activity, EmptyPayload); - } - else - { - activity.Start(); - } - - return true; - } - - return false; - } } } diff --git a/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/AspNetTelemetryEventSource.cs b/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/AspNetTelemetryEventSource.cs index 602c5df99e9..38c6cb452ab 100644 --- a/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/AspNetTelemetryEventSource.cs +++ b/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/AspNetTelemetryEventSource.cs @@ -15,6 +15,7 @@ // using System; +using System.Diagnostics; using System.Diagnostics.Tracing; namespace OpenTelemetry.Instrumentation.AspNet @@ -31,78 +32,90 @@ internal sealed class AspNetTelemetryEventSource : EventSource public static readonly AspNetTelemetryEventSource Log = new AspNetTelemetryEventSource(); [NonEvent] - public void ActivityException(string id, string eventName, Exception ex) + public void ActivityStarted(Activity activity) { - if (this.IsEnabled(EventLevel.Error, EventKeywords.All)) + if (this.IsEnabled(EventLevel.Verbose, EventKeywords.All)) { - this.ActivityException(id, eventName, ex.ToString()); + this.ActivityStarted(activity?.Id); } } - [Event(1, Message = "Callback='{0}'", Level = EventLevel.Verbose)] - public void TraceCallback(string callback) + [NonEvent] + public void ActivityStopped(Activity activity) { - this.WriteEvent(1, callback); + if (this.IsEnabled(EventLevel.Verbose, EventKeywords.All)) + { + this.ActivityStopped(activity?.Id); + } } - [Event(2, Message = "Activity started, Id='{0}'", Level = EventLevel.Verbose)] - public void ActivityStarted(string id) + [NonEvent] + public void ActivityRestored(Activity activity) { - this.WriteEvent(2, id); + if (this.IsEnabled(EventLevel.Informational, EventKeywords.All)) + { + this.ActivityRestored(activity?.Id); + } } - [Event(3, Message = "Activity stopped, Id='{0}', Name='{1}'", Level = EventLevel.Verbose)] - public void ActivityStopped(string id, string eventName) + [NonEvent] + public void ActivityException(Activity activity, Exception ex) { - this.WriteEvent(3, id, eventName); + if (this.IsEnabled(EventLevel.Error, EventKeywords.All)) + { + this.ActivityException(activity?.Id, ex.ToString()); + } } - [Event(4, Message = "Failed to parse header '{0}', value: '{1}'", Level = EventLevel.Informational)] - public void HeaderParsingError(string headerName, string headerValue) + [NonEvent] + public void CallbackException(Activity activity, string eventName, Exception ex) { - this.WriteEvent(4, headerName, headerValue); + if (this.IsEnabled(EventLevel.Error, EventKeywords.All)) + { + this.CallbackException(activity?.Id, eventName, ex.ToString()); + } } - [Event(5, Message = "Failed to extract activity, reason '{0}'", Level = EventLevel.Error)] - public void ActvityExtractionError(string reason) + [Event(1, Message = "Callback='{0}'", Level = EventLevel.Verbose)] + public void TraceCallback(string callback) { - this.WriteEvent(5, reason); + this.WriteEvent(1, callback); } - [Event(6, Message = "Finished Activity is detected on the stack, Id: '{0}', Name: '{1}'", Level = EventLevel.Error)] - public void FinishedActivityIsDetected(string id, string name) + [Event(2, Message = "Activity started, Id='{0}'", Level = EventLevel.Verbose)] + public void ActivityStarted(string id) { - this.WriteEvent(6, id, name); + this.WriteEvent(2, id); } - [Event(7, Message = "System.Diagnostics.Activity stack is too deep. This is a code authoring error, Activity will not be stopped.", Level = EventLevel.Error)] - public void ActivityStackIsTooDeepError() + [Event(3, Message = "Activity stopped, Id='{0}'", Level = EventLevel.Verbose)] + public void ActivityStopped(string id) { - this.WriteEvent(7); + this.WriteEvent(3, id); } - [Event(8, Message = "Activity restored, Id='{0}'", Level = EventLevel.Informational)] + [Event(4, Message = "Activity restored, Id='{0}'", Level = EventLevel.Informational)] public void ActivityRestored(string id) { - this.WriteEvent(8, id); + this.WriteEvent(4, id); } - [Event(9, Message = "Failed to invoke OnExecuteRequestStep, Error='{0}'", Level = EventLevel.Error)] + [Event(5, Message = "Failed to invoke OnExecuteRequestStep, Error='{0}'", Level = EventLevel.Error)] public void OnExecuteRequestStepInvokationError(string error) { - this.WriteEvent(9, error); + this.WriteEvent(5, error); } - [Event(10, Message = "System.Diagnostics.Activity stack is too deep. Current Id: '{0}', Name: '{1}'", Level = EventLevel.Warning)] - public void ActivityStackIsTooDeepDetails(string id, string name) + [Event(6, Message = "Activity exception, Id='{0}': {1}", Level = EventLevel.Error)] + public void ActivityException(string id, string ex) { - this.WriteEvent(10, id, name); + this.WriteEvent(6, id, ex); } - [Event(11, Message = "Activity exception, Id='{0}', Name='{1}': {2}", Level = EventLevel.Error)] - public void ActivityException(string id, string eventName, string ex) + [Event(7, Message = "Callback exception, Id='{0}', Name='{1}': {2}", Level = EventLevel.Error)] + public void CallbackException(string id, string eventName, string ex) { - this.WriteEvent(11, id, eventName, ex); + this.WriteEvent(7, id, eventName, ex); } } } diff --git a/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/Internal/BaseHeaderParser.cs b/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/Internal/BaseHeaderParser.cs deleted file mode 100644 index 4c429212023..00000000000 --- a/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/Internal/BaseHeaderParser.cs +++ /dev/null @@ -1,81 +0,0 @@ -// -// 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. -// - -namespace OpenTelemetry.Instrumentation.AspNet -{ - // Adoptation of code from https://github.com/aspnet/HttpAbstractions/blob/07d115400e4f8c7a66ba239f230805f03a14ee3d/src/Microsoft.Net.Http.Headers/BaseHeaderParser.cs - internal abstract class BaseHeaderParser : HttpHeaderParser - { - protected BaseHeaderParser(bool supportsMultipleValues) - : base(supportsMultipleValues) - { - } - - public sealed override bool TryParseValue(string value, ref int index, out T parsedValue) - { - parsedValue = default; - - // If multiple values are supported (i.e. list of values), then accept an empty string: The header may - // be added multiple times to the request/response message. E.g. - // Accept: text/xml; q=1 - // Accept: - // Accept: text/plain; q=0.2 - if (string.IsNullOrEmpty(value) || (index == value.Length)) - { - return this.SupportsMultipleValues; - } - - var current = HeaderUtilities.GetNextNonEmptyOrWhitespaceIndex(value, index, this.SupportsMultipleValues, out var separatorFound); - - if (separatorFound && !this.SupportsMultipleValues) - { - return false; // leading separators not allowed if we don't support multiple values. - } - - if (current == value.Length) - { - if (this.SupportsMultipleValues) - { - index = current; - } - - return this.SupportsMultipleValues; - } - - var length = this.GetParsedValueLength(value, current, out var result); - - if (length == 0) - { - return false; - } - - current += length; - current = HeaderUtilities.GetNextNonEmptyOrWhitespaceIndex(value, current, this.SupportsMultipleValues, out separatorFound); - - // If we support multiple values and we've not reached the end of the string, then we must have a separator. - if ((separatorFound && !this.SupportsMultipleValues) || (!separatorFound && (current < value.Length))) - { - return false; - } - - index = current; - parsedValue = result; - return true; - } - - protected abstract int GetParsedValueLength(string value, int startIndex, out T parsedValue); - } -} diff --git a/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/Internal/GenericHeaderParser.cs b/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/Internal/GenericHeaderParser.cs deleted file mode 100644 index 37824a0d021..00000000000 --- a/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/Internal/GenericHeaderParser.cs +++ /dev/null @@ -1,39 +0,0 @@ -// -// 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. -// - -using System; - -namespace OpenTelemetry.Instrumentation.AspNet -{ - // Adoptation of code from https://github.com/aspnet/HttpAbstractions/blob/07d115400e4f8c7a66ba239f230805f03a14ee3d/src/Microsoft.Net.Http.Headers/GenericHeaderParser.cs - internal sealed class GenericHeaderParser : BaseHeaderParser - { - private readonly GetParsedValueLengthDelegate getParsedValueLength; - - internal GenericHeaderParser(bool supportsMultipleValues, GetParsedValueLengthDelegate getParsedValueLength) - : base(supportsMultipleValues) - { - this.getParsedValueLength = getParsedValueLength ?? throw new ArgumentNullException(nameof(getParsedValueLength)); - } - - internal delegate int GetParsedValueLengthDelegate(string value, int startIndex, out T parsedValue); - - protected override int GetParsedValueLength(string value, int startIndex, out T parsedValue) - { - return this.getParsedValueLength(value, startIndex, out parsedValue); - } - } -} diff --git a/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/Internal/HeaderUtilities.cs b/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/Internal/HeaderUtilities.cs deleted file mode 100644 index 05644b3a8bc..00000000000 --- a/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/Internal/HeaderUtilities.cs +++ /dev/null @@ -1,59 +0,0 @@ -// -// 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. -// - -using System.Diagnostics.Contracts; - -namespace OpenTelemetry.Instrumentation.AspNet -{ - // Adoption of the code from https://github.com/aspnet/HttpAbstractions/blob/07d115400e4f8c7a66ba239f230805f03a14ee3d/src/Microsoft.Net.Http.Headers/HeaderUtilities.cs - internal static class HeaderUtilities - { - internal static int GetNextNonEmptyOrWhitespaceIndex( - string input, - int startIndex, - bool skipEmptyValues, - out bool separatorFound) - { - Contract.Requires(input != null); - Contract.Requires(startIndex <= input.Length); // it's OK if index == value.Length. - - separatorFound = false; - var current = startIndex + HttpRuleParser.GetWhitespaceLength(input, startIndex); - - if ((current == input.Length) || (input[current] != ',')) - { - return current; - } - - // If we have a separator, skip the separator and all following whitespaces. If we support - // empty values, continue until the current character is neither a separator nor a whitespace. - separatorFound = true; - current++; // skip delimiter. - current += HttpRuleParser.GetWhitespaceLength(input, current); - - if (skipEmptyValues) - { - while ((current < input.Length) && (input[current] == ',')) - { - current++; // skip delimiter. - current += HttpRuleParser.GetWhitespaceLength(input, current); - } - } - - return current; - } - } -} diff --git a/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/Internal/HttpHeaderParser.cs b/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/Internal/HttpHeaderParser.cs deleted file mode 100644 index 05584edcd8d..00000000000 --- a/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/Internal/HttpHeaderParser.cs +++ /dev/null @@ -1,40 +0,0 @@ -// -// 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. -// - -namespace OpenTelemetry.Instrumentation.AspNet -{ - // Adoptation of code from https://github.com/aspnet/HttpAbstractions/blob/07d115400e4f8c7a66ba239f230805f03a14ee3d/src/Microsoft.Net.Http.Headers/HttpHeaderParser.cs - internal abstract class HttpHeaderParser - { - private bool supportsMultipleValues; - - protected HttpHeaderParser(bool supportsMultipleValues) - { - this.supportsMultipleValues = supportsMultipleValues; - } - - public bool SupportsMultipleValues - { - get { return this.supportsMultipleValues; } - } - - // If a parser supports multiple values, a call to ParseValue/TryParseValue should return a value for 'index' - // pointing to the next non-whitespace character after a delimiter. E.g. if called with a start index of 0 - // for string "value , second_value", then after the call completes, 'index' must point to 's', i.e. the first - // non-whitespace after the separator ','. - public abstract bool TryParseValue(string value, ref int index, out T parsedValue); - } -} diff --git a/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/Internal/HttpParseResult.cs b/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/Internal/HttpParseResult.cs deleted file mode 100644 index 288e1faa070..00000000000 --- a/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/Internal/HttpParseResult.cs +++ /dev/null @@ -1,37 +0,0 @@ -// -// 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. -// - -namespace OpenTelemetry.Instrumentation.AspNet -{ - // Adoptation of code from https://github.com/aspnet/HttpAbstractions/blob/07d115400e4f8c7a66ba239f230805f03a14ee3d/src/Microsoft.Net.Http.Headers/HttpParseResult.cs - internal enum HttpParseResult - { - /// - /// Parsed successfully. - /// - Parsed, - - /// - /// Was not parsed. - /// - NotParsed, - - /// - /// Invalid format. - /// - InvalidFormat, - } -} diff --git a/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/Internal/HttpRuleParser.cs b/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/Internal/HttpRuleParser.cs deleted file mode 100644 index bd3b86b30f6..00000000000 --- a/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/Internal/HttpRuleParser.cs +++ /dev/null @@ -1,281 +0,0 @@ -// -// 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. -// - -using System.Diagnostics.Contracts; - -namespace OpenTelemetry.Instrumentation.AspNet -{ - // Adoptation of code from https://github.com/aspnet/HttpAbstractions/blob/07d115400e4f8c7a66ba239f230805f03a14ee3d/src/Microsoft.Net.Http.Headers/HttpRuleParser.cs - internal static class HttpRuleParser - { - internal const char CR = '\r'; - internal const char LF = '\n'; - internal const char SP = ' '; - internal const char Tab = '\t'; - internal const int MaxInt64Digits = 19; - internal const int MaxInt32Digits = 10; - - private const int MaxNestedCount = 5; - private static readonly bool[] TokenChars = CreateTokenChars(); - - internal static bool IsTokenChar(char character) - { - // Must be between 'space' (32) and 'DEL' (127) - if (character > 127) - { - return false; - } - - return TokenChars[character]; - } - - internal static int GetTokenLength(string input, int startIndex) - { - Contract.Requires(input != null); - Contract.Ensures((Contract.Result() >= 0) && (Contract.Result() <= (input.Length - startIndex))); - - if (startIndex >= input.Length) - { - return 0; - } - - var current = startIndex; - - while (current < input.Length) - { - if (!IsTokenChar(input[current])) - { - return current - startIndex; - } - - current++; - } - - return input.Length - startIndex; - } - - internal static int GetWhitespaceLength(string input, int startIndex) - { - Contract.Requires(input != null); - Contract.Ensures((Contract.Result() >= 0) && (Contract.Result() <= (input.Length - startIndex))); - - if (startIndex >= input.Length) - { - return 0; - } - - var current = startIndex; - - char c; - while (current < input.Length) - { - c = input[current]; - - if ((c == SP) || (c == Tab)) - { - current++; - continue; - } - - if (c == CR) - { - // If we have a #13 char, it must be followed by #10 and then at least one SP or HT. - if ((current + 2 < input.Length) && (input[current + 1] == LF)) - { - char spaceOrTab = input[current + 2]; - if ((spaceOrTab == SP) || (spaceOrTab == Tab)) - { - current += 3; - continue; - } - } - } - - return current - startIndex; - } - - // All characters between startIndex and the end of the string are LWS characters. - return input.Length - startIndex; - } - - internal static HttpParseResult GetQuotedStringLength(string input, int startIndex, out int length) - { - var nestedCount = 0; - return GetExpressionLength(input, startIndex, '"', '"', false, ref nestedCount, out length); - } - - // quoted-pair = "\" CHAR - // CHAR = - internal static HttpParseResult GetQuotedPairLength(string input, int startIndex, out int length) - { - Contract.Requires(input != null); - Contract.Requires((startIndex >= 0) && (startIndex < input.Length)); - Contract.Ensures((Contract.ValueAtReturn(out _) >= 0) && - (Contract.ValueAtReturn(out _) <= (input.Length - startIndex))); - - length = 0; - - if (input[startIndex] != '\\') - { - return HttpParseResult.NotParsed; - } - - // Quoted-char has 2 characters. Check whether there are 2 chars left ('\' + char) - // If so, check whether the character is in the range 0-127. If not, it's an invalid value. - if ((startIndex + 2 > input.Length) || (input[startIndex + 1] > 127)) - { - return HttpParseResult.InvalidFormat; - } - - // We don't care what the char next to '\' is. - length = 2; - return HttpParseResult.Parsed; - } - - private static bool[] CreateTokenChars() - { - // token = 1* - // CTL = - var tokenChars = new bool[128]; // everything is false - - for (int i = 33; i < 127; i++) - { - // skip Space (32) & DEL (127) - tokenChars[i] = true; - } - - // remove separators: these are not valid token characters - tokenChars[(byte)'('] = false; - tokenChars[(byte)')'] = false; - tokenChars[(byte)'<'] = false; - tokenChars[(byte)'>'] = false; - tokenChars[(byte)'@'] = false; - tokenChars[(byte)','] = false; - tokenChars[(byte)';'] = false; - tokenChars[(byte)':'] = false; - tokenChars[(byte)'\\'] = false; - tokenChars[(byte)'"'] = false; - tokenChars[(byte)'/'] = false; - tokenChars[(byte)'['] = false; - tokenChars[(byte)']'] = false; - tokenChars[(byte)'?'] = false; - tokenChars[(byte)'='] = false; - tokenChars[(byte)'{'] = false; - tokenChars[(byte)'}'] = false; - - return tokenChars; - } - - // TEXT = - // LWS = [CRLF] 1*( SP | HT ) - // CTL = - // - // Since we don't really care about the content of a quoted string or comment, we're more tolerant and - // allow these characters. We only want to find the delimiters ('"' for quoted string and '(', ')' for comment). - // - // 'nestedCount': Comments can be nested. We allow a depth of up to 5 nested comments, i.e. something like - // "(((((comment)))))". If we wouldn't define a limit an attacker could send a comment with hundreds of nested - // comments, resulting in a stack overflow exception. In addition having more than 1 nested comment (if any) - // is unusual. - private static HttpParseResult GetExpressionLength( - string input, - int startIndex, - char openChar, - char closeChar, - bool supportsNesting, - ref int nestedCount, - out int length) - { - Contract.Requires(input != null); - Contract.Requires((startIndex >= 0) && (startIndex < input.Length)); - Contract.Ensures((Contract.Result() != HttpParseResult.Parsed) || - (Contract.ValueAtReturn(out _) > 0)); - - length = 0; - - if (input[startIndex] != openChar) - { - return HttpParseResult.NotParsed; - } - - var current = startIndex + 1; // Start parsing with the character next to the first open-char - while (current < input.Length) - { - // Only check whether we have a quoted char, if we have at least 3 characters left to read (i.e. - // quoted char + closing char). Otherwise the closing char may be considered part of the quoted char. - if ((current + 2 < input.Length) && - (GetQuotedPairLength(input, current, out var quotedPairLength) == HttpParseResult.Parsed)) - { - // We ignore invalid quoted-pairs. Invalid quoted-pairs may mean that it looked like a quoted pair, - // but we actually have a quoted-string: e.g. '\' followed by a char >127 - quoted-pair only - // allows ASCII chars after '\'; qdtext allows both '\' and >127 chars. - current += quotedPairLength; - continue; - } - - // If we support nested expressions and we find an open-char, then parse the nested expressions. - if (supportsNesting && (input[current] == openChar)) - { - nestedCount++; - try - { - // Check if we exceeded the number of nested calls. - if (nestedCount > MaxNestedCount) - { - return HttpParseResult.InvalidFormat; - } - - HttpParseResult nestedResult = GetExpressionLength(input, current, openChar, closeChar, supportsNesting, ref nestedCount, out var nestedLength); - - switch (nestedResult) - { - case HttpParseResult.Parsed: - current += nestedLength; // add the length of the nested expression and continue. - break; - - case HttpParseResult.NotParsed: - Contract.Assert(false, "'NotParsed' is unexpected: We started nested expression parsing, because we found the open-char. So either it's a valid nested expression or it has invalid format."); - break; - - case HttpParseResult.InvalidFormat: - // If the nested expression is invalid, we can't continue, so we fail with invalid format. - return HttpParseResult.InvalidFormat; - - default: - Contract.Assert(false, "Unknown enum result: " + nestedResult); - break; - } - } - finally - { - nestedCount--; - } - } - - if (input[current] == closeChar) - { - length = current - startIndex + 1; - return HttpParseResult.Parsed; - } - - current++; - } - - // We didn't see the final quote, therefore we have an invalid expression string. - return HttpParseResult.InvalidFormat; - } - } -} diff --git a/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/Internal/NameValueHeaderValue.cs b/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/Internal/NameValueHeaderValue.cs deleted file mode 100644 index 5add297c6f4..00000000000 --- a/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/Internal/NameValueHeaderValue.cs +++ /dev/null @@ -1,134 +0,0 @@ -// -// 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. -// - -using System.Diagnostics.Contracts; - -namespace OpenTelemetry.Instrumentation.AspNet -{ - // Adoptation of code from https://github.com/aspnet/HttpAbstractions/blob/07d115400e4f8c7a66ba239f230805f03a14ee3d/src/Microsoft.Net.Http.Headers/NameValueHeaderValue.cs - - // According to the RFC, in places where a "parameter" is required, the value is mandatory - // (e.g. Media-Type, Accept). However, we don't introduce a dedicated type for it. So NameValueHeaderValue supports - // name-only values in addition to name/value pairs. - internal class NameValueHeaderValue - { - private static readonly HttpHeaderParser SingleValueParser - = new GenericHeaderParser(false, GetNameValueLength); - - private string name; - private string value; - - private NameValueHeaderValue() - { - // Used by the parser to create a new instance of this type. - } - - public string Name - { - get { return this.name; } - } - - public string Value - { - get { return this.value; } - } - - public static bool TryParse(string input, out NameValueHeaderValue parsedValue) - { - var index = 0; - return SingleValueParser.TryParseValue(input, ref index, out parsedValue); - } - - internal static int GetValueLength(string input, int startIndex) - { - Contract.Requires(input != null); - - if (startIndex >= input.Length) - { - return 0; - } - - var valueLength = HttpRuleParser.GetTokenLength(input, startIndex); - - if (valueLength == 0) - { - // A value can either be a token or a quoted string. Check if it is a quoted string. - if (HttpRuleParser.GetQuotedStringLength(input, startIndex, out valueLength) != HttpParseResult.Parsed) - { - // We have an invalid value. Reset the name and return. - return 0; - } - } - - return valueLength; - } - - private static int GetNameValueLength(string input, int startIndex, out NameValueHeaderValue parsedValue) - { - Contract.Requires(input != null); - Contract.Requires(startIndex >= 0); - - parsedValue = null; - - if (string.IsNullOrEmpty(input) || (startIndex >= input.Length)) - { - return 0; - } - - // Parse the name, i.e. in name/value string "=". Caller must remove - // leading whitespaces. - var nameLength = HttpRuleParser.GetTokenLength(input, startIndex); - - if (nameLength == 0) - { - return 0; - } - - var name = input.Substring(startIndex, nameLength); - var current = startIndex + nameLength; - current += HttpRuleParser.GetWhitespaceLength(input, current); - - // Parse the separator between name and value - if ((current == input.Length) || (input[current] != '=')) - { - // We only have a name and that's OK. Return. - parsedValue = new NameValueHeaderValue - { - name = name, - }; - current += HttpRuleParser.GetWhitespaceLength(input, current); // skip whitespaces - return current - startIndex; - } - - current++; // skip delimiter. - current += HttpRuleParser.GetWhitespaceLength(input, current); - - // Parse the value, i.e. in name/value string "=" - int valueLength = GetValueLength(input, current); - - // Value after the '=' may be empty - // Use parameterless ctor to avoid double-parsing of name and value, i.e. skip public ctor validation. - parsedValue = new NameValueHeaderValue - { - name = name, - value = input.Substring(current, valueLength), - }; - current += valueLength; - current += HttpRuleParser.GetWhitespaceLength(input, current); // skip whitespaces - return current - startIndex; - } - } -} diff --git a/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule.csproj b/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule.csproj index c9f90a4892d..9f9c307f674 100644 --- a/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule.csproj +++ b/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule.csproj @@ -17,7 +17,7 @@ - + diff --git a/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/TelemetryHttpModule.cs b/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/TelemetryHttpModule.cs index 953f047773a..6801472ec53 100644 --- a/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/TelemetryHttpModule.cs +++ b/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/TelemetryHttpModule.cs @@ -16,8 +16,10 @@ using System; using System.ComponentModel; +using System.Diagnostics; using System.Reflection; using System.Web; +using OpenTelemetry.Context.Propagation; namespace OpenTelemetry.Instrumentation.AspNet { @@ -26,6 +28,16 @@ namespace OpenTelemetry.Instrumentation.AspNet /// public class TelemetryHttpModule : IHttpModule { + /// + /// OpenTelemetry.Instrumentation.AspNet name. + /// + public const string AspNetSourceName = "OpenTelemetry.Instrumentation.AspNet.Telemetry"; + + /// + /// for OpenTelemetry.Instrumentation.AspNet created objects. + /// + public const string AspNetActivityName = "Microsoft.AspNet.HttpReqIn"; + private const string BeginCalledFlag = "OpenTelemetry.Instrumentation.AspNet.BeginCalled"; // ServerVariable set only on rewritten HttpContext by URL Rewrite module. @@ -36,11 +48,37 @@ public class TelemetryHttpModule : IHttpModule private static readonly MethodInfo OnStepMethodInfo = typeof(HttpApplication).GetMethod("OnExecuteRequestStep"); + private TraceContextPropagator traceContextPropagator = new TraceContextPropagator(); + + /// + /// Gets or sets the to use to + /// extract from incoming requests. + /// + [EditorBrowsable(EditorBrowsableState.Never)] + public TraceContextPropagator TextMapPropagator + { + get => this.traceContextPropagator; + set => this.traceContextPropagator = value ?? throw new ArgumentNullException(nameof(value)); + } + + /// + /// Gets or sets a callback action to be fired when a request is started. + /// + [EditorBrowsable(EditorBrowsableState.Never)] + public Action OnRequestStartedCallback { get; set; } + + /// + /// Gets or sets a callback action to be fired when a request is stopped. + /// + [EditorBrowsable(EditorBrowsableState.Never)] + public Action OnRequestStoppedCallback { get; set; } + /// - /// Gets or sets a value indicating whether TelemetryHttpModule should parse headers to get correlation ids. + /// Gets or sets a callback action to be fired when an unhandled + /// exception is thrown processing a request. /// [EditorBrowsable(EditorBrowsableState.Never)] - public bool ParseHeaders { get; set; } = true; + public Action OnExceptionCallback { get; set; } /// public void Dispose() @@ -100,7 +138,7 @@ private void Application_BeginRequest(object sender, EventArgs e) { var context = ((HttpApplication)sender).Context; AspNetTelemetryEventSource.Log.TraceCallback("Application_BeginRequest"); - ActivityHelper.CreateRootActivity(context, this.ParseHeaders); + ActivityHelper.StartAspNetActivity(this.TextMapPropagator, context, this.OnRequestStartedCallback); context.Items[BeginCalledFlag] = true; } @@ -135,13 +173,13 @@ private void Application_EndRequest(object sender, EventArgs e) else { // Activity has never been started - ActivityHelper.CreateRootActivity(context, this.ParseHeaders); + ActivityHelper.StartAspNetActivity(this.TextMapPropagator, context, this.OnRequestStartedCallback); } } if (trackActivity) { - ActivityHelper.StopAspNetActivity(context.Items); + ActivityHelper.StopAspNetActivity(context, this.OnRequestStoppedCallback); } } @@ -156,10 +194,10 @@ private void Application_Error(object sender, EventArgs e) { if (!context.Items.Contains(BeginCalledFlag)) { - ActivityHelper.CreateRootActivity(context, this.ParseHeaders); + ActivityHelper.StartAspNetActivity(this.TextMapPropagator, context, this.OnRequestStartedCallback); } - ActivityHelper.WriteActivityException(context.Items, exception); + ActivityHelper.WriteActivityException(context.Items, exception, this.OnExceptionCallback); } } } From a012493764d7e0ad7d2e6ec7c4ace551e4139249 Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Fri, 13 Aug 2021 09:12:48 -0700 Subject: [PATCH 2/9] New design for TelemetryHttpModule using ActivitySource + OpenTelemetry.API part 2 (#2254) * Use a single context.Items key for state management to make things more efficient. * Added a comment for clarity. * Code review. --- .../ActivityHelper.cs | 92 ++++++++++++------- .../TelemetryHttpModule.cs | 17 ++-- 2 files changed, 67 insertions(+), 42 deletions(-) diff --git a/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/ActivityHelper.cs b/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/ActivityHelper.cs index dd13ab6f353..7675b8e4516 100644 --- a/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/ActivityHelper.cs +++ b/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/ActivityHelper.cs @@ -18,6 +18,7 @@ using System.Collections; using System.Collections.Generic; using System.Diagnostics; +using System.Runtime.CompilerServices; using System.Web; using OpenTelemetry.Context.Propagation; @@ -31,10 +32,33 @@ internal static class ActivityHelper /// /// Key to store the activity in HttpContext. /// - public const string ActivityKey = "__AspnetActivity__"; + private const string ActivityKey = "__AspnetActivity__"; private static readonly ActivitySource AspNetSource = new ActivitySource(TelemetryHttpModule.AspNetSourceName); private static readonly Func> HttpRequestHeaderValuesGetter = (request, name) => request.Headers.GetValues(name); + private static readonly object StartedButNotSampledObj = new object(); + + /// + /// Try to get the started for the running . + /// + /// . + /// Started or if 1) start has not been called or 2) start was + /// called but sampling decided not to create an instance. + /// if start has been called. + public static bool HasStarted(HttpContext httpContext, out Activity aspNetActivity) + { + object itemValue = httpContext.Items[ActivityKey]; + if (itemValue is Activity activity) + { + aspNetActivity = activity; + return true; + } + + aspNetActivity = null; + return itemValue == StartedButNotSampledObj; + } /// /// Creates root (first level) activity that describes incoming request. @@ -55,6 +79,12 @@ public static Activity StartAspNetActivity(TextMapPropagator textMapPropagator, if (propagationContext.Baggage != default) { + // todo: RestoreActivityIfNeeded below compensates for + // AsyncLocal Activity.Current being lost. Baggage + // potentially will suffer from the same issue, but we can’t + // simply add it to context.Items because any change results + // in a new instance. Probably need to save it at the end of + // each OnExecuteRequestStep. Baggage.Current = propagationContext.Baggage; } @@ -69,6 +99,10 @@ public static Activity StartAspNetActivity(TextMapPropagator textMapPropagator, AspNetTelemetryEventSource.Log.ActivityStarted(activity); } + else + { + context.Items[ActivityKey] = StartedButNotSampledObj; + } return activity; } @@ -76,46 +110,45 @@ public static Activity StartAspNetActivity(TextMapPropagator textMapPropagator, /// /// Stops the activity and notifies listeners about it. /// + /// . /// Current HttpContext. /// Callback action. - public static void StopAspNetActivity(HttpContext context, Action onRequestStoppedCallback) + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public static void StopAspNetActivity(Activity aspNetActivity, HttpContext context, Action onRequestStoppedCallback) { - var contextItems = context.Items; - var currentActivity = Activity.Current; - Activity aspNetActivity = (Activity)contextItems[ActivityKey]; - - if (currentActivity != aspNetActivity) + if (aspNetActivity == null) { - Activity.Current = aspNetActivity; + // This is the case where a start was called but no activity was + // created due to a sampling decision. + context.Items[ActivityKey] = null; + return; } - if (aspNetActivity != null) - { - aspNetActivity.Stop(); - contextItems[ActivityKey] = null; + var currentActivity = Activity.Current; - try - { - onRequestStoppedCallback?.Invoke(aspNetActivity, context); - } - catch (Exception callbackEx) - { - AspNetTelemetryEventSource.Log.CallbackException(aspNetActivity, "OnStopped", callbackEx); - } + aspNetActivity.Stop(); + context.Items[ActivityKey] = null; - AspNetTelemetryEventSource.Log.ActivityStopped(currentActivity); + try + { + onRequestStoppedCallback?.Invoke(aspNetActivity, context); + } + catch (Exception callbackEx) + { + AspNetTelemetryEventSource.Log.CallbackException(aspNetActivity, "OnStopped", callbackEx); } + AspNetTelemetryEventSource.Log.ActivityStopped(currentActivity); + if (currentActivity != aspNetActivity) { Activity.Current = currentActivity; } } - public static void WriteActivityException(IDictionary contextItems, Exception exception, Action onExceptionCallback) + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public static void WriteActivityException(Activity aspNetActivity, Exception exception, Action onExceptionCallback) { - Activity aspNetActivity = (Activity)contextItems[ActivityKey]; - if (aspNetActivity != null) { try @@ -138,16 +171,13 @@ public static void WriteActivityException(IDictionary contextItems, Exception ex /// activities with the root activity of the request. /// /// HttpContext.Items dictionary. + [MethodImpl(MethodImplOptions.AggressiveInlining)] internal static void RestoreActivityIfNeeded(IDictionary contextItems) { - if (Activity.Current == null) + if (Activity.Current == null && contextItems[ActivityKey] is Activity aspNetActivity) { - Activity aspNetActivity = (Activity)contextItems[ActivityKey]; - if (aspNetActivity != null) - { - Activity.Current = aspNetActivity; - AspNetTelemetryEventSource.Log.ActivityRestored(aspNetActivity); - } + Activity.Current = aspNetActivity; + AspNetTelemetryEventSource.Log.ActivityRestored(aspNetActivity); } } } diff --git a/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/TelemetryHttpModule.cs b/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/TelemetryHttpModule.cs index 6801472ec53..f6cb354290d 100644 --- a/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/TelemetryHttpModule.cs +++ b/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/TelemetryHttpModule.cs @@ -38,8 +38,6 @@ public class TelemetryHttpModule : IHttpModule /// public const string AspNetActivityName = "Microsoft.AspNet.HttpReqIn"; - private const string BeginCalledFlag = "OpenTelemetry.Instrumentation.AspNet.BeginCalled"; - // ServerVariable set only on rewritten HttpContext by URL Rewrite module. private const string URLRewriteRewrittenRequest = "IIS_WasUrlRewritten"; @@ -139,7 +137,6 @@ private void Application_BeginRequest(object sender, EventArgs e) var context = ((HttpApplication)sender).Context; AspNetTelemetryEventSource.Log.TraceCallback("Application_BeginRequest"); ActivityHelper.StartAspNetActivity(this.TextMapPropagator, context, this.OnRequestStartedCallback); - context.Items[BeginCalledFlag] = true; } private void Application_PreRequestHandlerExecute(object sender, EventArgs e) @@ -155,9 +152,7 @@ private void Application_EndRequest(object sender, EventArgs e) var context = ((HttpApplication)sender).Context; - // EndRequest does it's best effort to notify that request has ended - // BeginRequest has never been called - if (!context.Items.Contains(BeginCalledFlag)) + if (!ActivityHelper.HasStarted(context, out Activity aspNetActivity)) { // Rewrite: In case of rewrite, a new request context is created, called the child request, and it goes through the entire IIS/ASP.NET integrated pipeline. // The child request can be mapped to any of the handlers configured in IIS, and it's execution is no different than it would be if it was received via the HTTP stack. @@ -173,13 +168,13 @@ private void Application_EndRequest(object sender, EventArgs e) else { // Activity has never been started - ActivityHelper.StartAspNetActivity(this.TextMapPropagator, context, this.OnRequestStartedCallback); + aspNetActivity = ActivityHelper.StartAspNetActivity(this.TextMapPropagator, context, this.OnRequestStartedCallback); } } if (trackActivity) { - ActivityHelper.StopAspNetActivity(context, this.OnRequestStoppedCallback); + ActivityHelper.StopAspNetActivity(aspNetActivity, context, this.OnRequestStoppedCallback); } } @@ -192,12 +187,12 @@ private void Application_Error(object sender, EventArgs e) var exception = context.Error; if (exception != null) { - if (!context.Items.Contains(BeginCalledFlag)) + if (!ActivityHelper.HasStarted(context, out Activity aspNetActivity)) { - ActivityHelper.StartAspNetActivity(this.TextMapPropagator, context, this.OnRequestStartedCallback); + aspNetActivity = ActivityHelper.StartAspNetActivity(this.TextMapPropagator, context, this.OnRequestStartedCallback); } - ActivityHelper.WriteActivityException(context.Items, exception, this.OnExceptionCallback); + ActivityHelper.WriteActivityException(aspNetActivity, exception, this.OnExceptionCallback); } } } From 8b1fd836e75bcaa9fc2dca5e38f3b063309b89a3 Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Mon, 16 Aug 2021 09:44:46 -0700 Subject: [PATCH 3/9] New design for TelemetryHttpModule using ActivitySource + OpenTelemetry.API part 3 (#2256) * Update ASP.NET instrumentation to use the new TelemetryHttpModule. * Fixed TelemetryHttpModule not starting its Activity objects. Added an example of request suppression. * Tweaks an logging improvements. * Sealed AspNetInstrumentationEventSource. * Code review. --- examples/AspNet/Examples.AspNet.csproj | 3 +- .../SuppressInstrumentationHttpModule.cs | 58 +++++ examples/AspNet/Web.config | 13 +- .../.publicApi/net461/PublicAPI.Unshipped.txt | 18 +- .../ActivityHelper.cs | 9 +- .../AspNetTelemetryEventSource.cs | 5 +- ...entation.AspNet.TelemetryHttpModule.csproj | 4 + .../TelemetryHttpModule.cs | 43 +--- .../TelemetryHttpModuleOptions.cs | 61 ++++++ .../.publicApi/net461/PublicAPI.Unshipped.txt | 2 + .../AspNetInstrumentation.cs | 15 +- .../AspNetInstrumentationOptions.cs | 24 ++- .../AspNetInstrumentationEventSource.cs | 42 ++-- .../Implementation/HttpInListener.cs | 203 ++++++------------ ...penTelemetry.Instrumentation.AspNet.csproj | 8 +- .../TracerProviderBuilderExtensions.cs | 5 +- 16 files changed, 268 insertions(+), 245 deletions(-) create mode 100644 examples/AspNet/SuppressInstrumentationHttpModule.cs create mode 100644 src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/TelemetryHttpModuleOptions.cs diff --git a/examples/AspNet/Examples.AspNet.csproj b/examples/AspNet/Examples.AspNet.csproj index b67f0c72313..fef00f55b5e 100644 --- a/examples/AspNet/Examples.AspNet.csproj +++ b/examples/AspNet/Examples.AspNet.csproj @@ -61,6 +61,7 @@ + @@ -149,4 +150,4 @@ - + \ No newline at end of file diff --git a/examples/AspNet/SuppressInstrumentationHttpModule.cs b/examples/AspNet/SuppressInstrumentationHttpModule.cs new file mode 100644 index 00000000000..fb832109182 --- /dev/null +++ b/examples/AspNet/SuppressInstrumentationHttpModule.cs @@ -0,0 +1,58 @@ +// +// 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. +// + +using System; +using System.Web; +using OpenTelemetry; + +namespace Examples.AspNet +{ + /// + /// A demo which will suppress ASP.NET + /// instrumentation if a request contains "suppress=true" on the query + /// string. Suppressed spans will not be processed/exported by the + /// OpenTelemetry SDK. + /// + public class SuppressInstrumentationHttpModule : IHttpModule + { + private IDisposable suppressionScope; + + public void Init(HttpApplication context) + { + context.BeginRequest += this.Application_BeginRequest; + context.EndRequest += this.Application_EndRequest; + } + + public void Dispose() + { + } + + private void Application_BeginRequest(object sender, EventArgs e) + { + var context = ((HttpApplication)sender).Context; + + if (context.Request.QueryString["suppress"] == "true") + { + this.suppressionScope = SuppressInstrumentationScope.Begin(); + } + } + + private void Application_EndRequest(object sender, EventArgs e) + { + this.suppressionScope?.Dispose(); + } + } +} diff --git a/examples/AspNet/Web.config b/examples/AspNet/Web.config index fcf320ae33e..0ee991a134e 100644 --- a/examples/AspNet/Web.config +++ b/examples/AspNet/Web.config @@ -23,6 +23,7 @@ + @@ -37,20 +38,16 @@ - - + + - - - - - - + + diff --git a/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/.publicApi/net461/PublicAPI.Unshipped.txt b/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/.publicApi/net461/PublicAPI.Unshipped.txt index 04b7b15df9b..367f7a1e6e6 100644 --- a/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/.publicApi/net461/PublicAPI.Unshipped.txt +++ b/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/.publicApi/net461/PublicAPI.Unshipped.txt @@ -3,12 +3,14 @@ const OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule.AspNetSourceName OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule.Dispose() -> void OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule.Init(System.Web.HttpApplication context) -> void -OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule.OnExceptionCallback.get -> System.Action -OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule.OnExceptionCallback.set -> void -OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule.OnRequestStartedCallback.get -> System.Action -OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule.OnRequestStartedCallback.set -> void -OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule.OnRequestStoppedCallback.get -> System.Action -OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule.OnRequestStoppedCallback.set -> void OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule.TelemetryHttpModule() -> void -OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule.TextMapPropagator.get -> OpenTelemetry.Context.Propagation.TraceContextPropagator -OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule.TextMapPropagator.set -> void +OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModuleOptions +OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModuleOptions.OnExceptionCallback.get -> System.Action +OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModuleOptions.OnExceptionCallback.set -> void +OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModuleOptions.OnRequestStartedCallback.get -> System.Action +OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModuleOptions.OnRequestStartedCallback.set -> void +OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModuleOptions.OnRequestStoppedCallback.get -> System.Action +OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModuleOptions.OnRequestStoppedCallback.set -> void +OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModuleOptions.TextMapPropagator.get -> OpenTelemetry.Context.Propagation.TextMapPropagator +OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModuleOptions.TextMapPropagator.set -> void +static OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule.Options.get -> OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModuleOptions diff --git a/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/ActivityHelper.cs b/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/ActivityHelper.cs index 7675b8e4516..29711309204 100644 --- a/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/ActivityHelper.cs +++ b/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/ActivityHelper.cs @@ -34,7 +34,8 @@ internal static class ActivityHelper /// private const string ActivityKey = "__AspnetActivity__"; - private static readonly ActivitySource AspNetSource = new ActivitySource(TelemetryHttpModule.AspNetSourceName); + private static readonly Version Version = typeof(ActivityHelper).Assembly.GetName().Version; + private static readonly ActivitySource AspNetSource = new ActivitySource(TelemetryHttpModule.AspNetSourceName, Version.ToString()); private static readonly Func> HttpRequestHeaderValuesGetter = (request, name) => request.Headers.GetValues(name); private static readonly object StartedButNotSampledObj = new object(); @@ -71,7 +72,7 @@ public static Activity StartAspNetActivity(TextMapPropagator textMapPropagator, { PropagationContext propagationContext = textMapPropagator.Extract(default, context.Request, HttpRequestHeaderValuesGetter); - Activity activity = AspNetSource.CreateActivity(TelemetryHttpModule.AspNetActivityName, ActivityKind.Server, propagationContext.ActivityContext); + Activity activity = AspNetSource.StartActivity(TelemetryHttpModule.AspNetActivityName, ActivityKind.Server, propagationContext.ActivityContext); if (activity != null) { @@ -147,13 +148,13 @@ public static void StopAspNetActivity(Activity aspNetActivity, HttpContext conte } [MethodImpl(MethodImplOptions.AggressiveInlining)] - public static void WriteActivityException(Activity aspNetActivity, Exception exception, Action onExceptionCallback) + public static void WriteActivityException(Activity aspNetActivity, HttpContext context, Exception exception, Action onExceptionCallback) { if (aspNetActivity != null) { try { - onExceptionCallback?.Invoke(aspNetActivity, exception); + onExceptionCallback?.Invoke(aspNetActivity, context, exception); } catch (Exception callbackEx) { diff --git a/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/AspNetTelemetryEventSource.cs b/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/AspNetTelemetryEventSource.cs index 38c6cb452ab..9fa3a5ef871 100644 --- a/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/AspNetTelemetryEventSource.cs +++ b/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/AspNetTelemetryEventSource.cs @@ -17,6 +17,7 @@ using System; using System.Diagnostics; using System.Diagnostics.Tracing; +using OpenTelemetry.Internal; namespace OpenTelemetry.Instrumentation.AspNet { @@ -63,7 +64,7 @@ public void ActivityException(Activity activity, Exception ex) { if (this.IsEnabled(EventLevel.Error, EventKeywords.All)) { - this.ActivityException(activity?.Id, ex.ToString()); + this.ActivityException(activity?.Id, ex.ToInvariantString()); } } @@ -72,7 +73,7 @@ public void CallbackException(Activity activity, string eventName, Exception ex) { if (this.IsEnabled(EventLevel.Error, EventKeywords.All)) { - this.CallbackException(activity?.Id, eventName, ex.ToString()); + this.CallbackException(activity?.Id, eventName, ex.ToInvariantString()); } } diff --git a/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule.csproj b/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule.csproj index 9f9c307f674..9ac9a472c1f 100644 --- a/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule.csproj +++ b/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule.csproj @@ -12,6 +12,10 @@ false + + + + diff --git a/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/TelemetryHttpModule.cs b/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/TelemetryHttpModule.cs index f6cb354290d..337b956a648 100644 --- a/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/TelemetryHttpModule.cs +++ b/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/TelemetryHttpModule.cs @@ -15,11 +15,9 @@ // using System; -using System.ComponentModel; using System.Diagnostics; using System.Reflection; using System.Web; -using OpenTelemetry.Context.Propagation; namespace OpenTelemetry.Instrumentation.AspNet { @@ -46,37 +44,10 @@ public class TelemetryHttpModule : IHttpModule private static readonly MethodInfo OnStepMethodInfo = typeof(HttpApplication).GetMethod("OnExecuteRequestStep"); - private TraceContextPropagator traceContextPropagator = new TraceContextPropagator(); - - /// - /// Gets or sets the to use to - /// extract from incoming requests. - /// - [EditorBrowsable(EditorBrowsableState.Never)] - public TraceContextPropagator TextMapPropagator - { - get => this.traceContextPropagator; - set => this.traceContextPropagator = value ?? throw new ArgumentNullException(nameof(value)); - } - - /// - /// Gets or sets a callback action to be fired when a request is started. - /// - [EditorBrowsable(EditorBrowsableState.Never)] - public Action OnRequestStartedCallback { get; set; } - - /// - /// Gets or sets a callback action to be fired when a request is stopped. - /// - [EditorBrowsable(EditorBrowsableState.Never)] - public Action OnRequestStoppedCallback { get; set; } - /// - /// Gets or sets a callback action to be fired when an unhandled - /// exception is thrown processing a request. + /// Gets the applied to requests processed by the handler. /// - [EditorBrowsable(EditorBrowsableState.Never)] - public Action OnExceptionCallback { get; set; } + public static TelemetryHttpModuleOptions Options { get; } = new TelemetryHttpModuleOptions(); /// public void Dispose() @@ -136,7 +107,7 @@ private void Application_BeginRequest(object sender, EventArgs e) { var context = ((HttpApplication)sender).Context; AspNetTelemetryEventSource.Log.TraceCallback("Application_BeginRequest"); - ActivityHelper.StartAspNetActivity(this.TextMapPropagator, context, this.OnRequestStartedCallback); + ActivityHelper.StartAspNetActivity(Options.TextMapPropagator, context, Options.OnRequestStartedCallback); } private void Application_PreRequestHandlerExecute(object sender, EventArgs e) @@ -168,13 +139,13 @@ private void Application_EndRequest(object sender, EventArgs e) else { // Activity has never been started - aspNetActivity = ActivityHelper.StartAspNetActivity(this.TextMapPropagator, context, this.OnRequestStartedCallback); + aspNetActivity = ActivityHelper.StartAspNetActivity(Options.TextMapPropagator, context, Options.OnRequestStartedCallback); } } if (trackActivity) { - ActivityHelper.StopAspNetActivity(aspNetActivity, context, this.OnRequestStoppedCallback); + ActivityHelper.StopAspNetActivity(aspNetActivity, context, Options.OnRequestStoppedCallback); } } @@ -189,10 +160,10 @@ private void Application_Error(object sender, EventArgs e) { if (!ActivityHelper.HasStarted(context, out Activity aspNetActivity)) { - aspNetActivity = ActivityHelper.StartAspNetActivity(this.TextMapPropagator, context, this.OnRequestStartedCallback); + aspNetActivity = ActivityHelper.StartAspNetActivity(Options.TextMapPropagator, context, Options.OnRequestStartedCallback); } - ActivityHelper.WriteActivityException(aspNetActivity, exception, this.OnExceptionCallback); + ActivityHelper.WriteActivityException(aspNetActivity, context, exception, Options.OnExceptionCallback); } } } diff --git a/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/TelemetryHttpModuleOptions.cs b/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/TelemetryHttpModuleOptions.cs new file mode 100644 index 00000000000..5110bfeb769 --- /dev/null +++ b/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/TelemetryHttpModuleOptions.cs @@ -0,0 +1,61 @@ +// +// 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. +// + +using System; +using System.Diagnostics; +using System.Web; +using OpenTelemetry.Context.Propagation; + +namespace OpenTelemetry.Instrumentation.AspNet +{ + /// + /// Stores options for the . + /// + public class TelemetryHttpModuleOptions + { + private TextMapPropagator textMapPropagator = new TraceContextPropagator(); + + internal TelemetryHttpModuleOptions() + { + } + + /// + /// Gets or sets the to use to + /// extract from incoming requests. + /// + public TextMapPropagator TextMapPropagator + { + get => this.textMapPropagator; + set => this.textMapPropagator = value ?? throw new ArgumentNullException(nameof(value)); + } + + /// + /// Gets or sets a callback action to be fired when a request is started. + /// + public Action OnRequestStartedCallback { get; set; } + + /// + /// Gets or sets a callback action to be fired when a request is stopped. + /// + public Action OnRequestStoppedCallback { get; set; } + + /// + /// Gets or sets a callback action to be fired when an unhandled + /// exception is thrown processing a request. + /// + public Action OnExceptionCallback { get; set; } + } +} diff --git a/src/OpenTelemetry.Instrumentation.AspNet/.publicApi/net461/PublicAPI.Unshipped.txt b/src/OpenTelemetry.Instrumentation.AspNet/.publicApi/net461/PublicAPI.Unshipped.txt index 569931e171a..35b5f87638f 100644 --- a/src/OpenTelemetry.Instrumentation.AspNet/.publicApi/net461/PublicAPI.Unshipped.txt +++ b/src/OpenTelemetry.Instrumentation.AspNet/.publicApi/net461/PublicAPI.Unshipped.txt @@ -4,5 +4,7 @@ OpenTelemetry.Instrumentation.AspNet.AspNetInstrumentationOptions.Enrich.get -> OpenTelemetry.Instrumentation.AspNet.AspNetInstrumentationOptions.Enrich.set -> void OpenTelemetry.Instrumentation.AspNet.AspNetInstrumentationOptions.Filter.get -> System.Func OpenTelemetry.Instrumentation.AspNet.AspNetInstrumentationOptions.Filter.set -> void +OpenTelemetry.Instrumentation.AspNet.AspNetInstrumentationOptions.RecordException.get -> bool +OpenTelemetry.Instrumentation.AspNet.AspNetInstrumentationOptions.RecordException.set -> void OpenTelemetry.Trace.TracerProviderBuilderExtensions static OpenTelemetry.Trace.TracerProviderBuilderExtensions.AddAspNetInstrumentation(this OpenTelemetry.Trace.TracerProviderBuilder builder, System.Action configureAspNetInstrumentationOptions = null) -> OpenTelemetry.Trace.TracerProviderBuilder diff --git a/src/OpenTelemetry.Instrumentation.AspNet/AspNetInstrumentation.cs b/src/OpenTelemetry.Instrumentation.AspNet/AspNetInstrumentation.cs index 3360b08abea..2bd2a988be7 100644 --- a/src/OpenTelemetry.Instrumentation.AspNet/AspNetInstrumentation.cs +++ b/src/OpenTelemetry.Instrumentation.AspNet/AspNetInstrumentation.cs @@ -13,6 +13,7 @@ // See the License for the specific language governing permissions and // limitations under the License. // + using System; using OpenTelemetry.Instrumentation.AspNet.Implementation; @@ -21,11 +22,9 @@ namespace OpenTelemetry.Instrumentation.AspNet /// /// Asp.Net Requests instrumentation. /// - internal class AspNetInstrumentation : IDisposable + internal sealed class AspNetInstrumentation : IDisposable { - internal const string AspNetDiagnosticListenerName = "OpenTelemetry.Instrumentation.AspNet.Telemetry"; - - private readonly DiagnosticSourceSubscriber diagnosticSourceSubscriber; + private readonly HttpInListener httpInListener; /// /// Initializes a new instance of the class. @@ -33,17 +32,13 @@ internal class AspNetInstrumentation : IDisposable /// Configuration options for ASP.NET instrumentation. public AspNetInstrumentation(AspNetInstrumentationOptions options) { - this.diagnosticSourceSubscriber = new DiagnosticSourceSubscriber( - name => new HttpInListener(name, options), - listener => listener.Name == AspNetDiagnosticListenerName, - null); - this.diagnosticSourceSubscriber.Subscribe(); + this.httpInListener = new HttpInListener(options); } /// public void Dispose() { - this.diagnosticSourceSubscriber?.Dispose(); + this.httpInListener?.Dispose(); } } } diff --git a/src/OpenTelemetry.Instrumentation.AspNet/AspNetInstrumentationOptions.cs b/src/OpenTelemetry.Instrumentation.AspNet/AspNetInstrumentationOptions.cs index 613b3f922f7..4209ce23e05 100644 --- a/src/OpenTelemetry.Instrumentation.AspNet/AspNetInstrumentationOptions.cs +++ b/src/OpenTelemetry.Instrumentation.AspNet/AspNetInstrumentationOptions.cs @@ -26,11 +26,19 @@ namespace OpenTelemetry.Instrumentation.AspNet public class AspNetInstrumentationOptions { /// - /// Gets or sets a Filter function that determines whether or not to collect telemetry about requests on a per request basis. - /// The Filter gets the HttpContext, and should return a boolean. - /// If Filter returns true, the request is collected. - /// If Filter returns false or throw exception, the request is filtered out. + /// Gets or sets a filter callback function that determines on a per + /// request basis whether or not to collect telemetry. /// + /// + /// The filter callback receives the for the + /// current request and should return a boolean. + /// + /// If filter returns the request is + /// collected. + /// If filter returns or throws an + /// exception the request is filtered out (NOT collected). + /// + /// public Func Filter { get; set; } /// @@ -43,5 +51,13 @@ public class AspNetInstrumentationOptions /// The type of this object depends on the event, which is given by the above parameter. /// public Action Enrich { get; set; } + + /// + /// Gets or sets a value indicating whether the exception will be recorded as ActivityEvent or not. + /// + /// + /// See: . + /// + public bool RecordException { get; set; } } } diff --git a/src/OpenTelemetry.Instrumentation.AspNet/Implementation/AspNetInstrumentationEventSource.cs b/src/OpenTelemetry.Instrumentation.AspNet/Implementation/AspNetInstrumentationEventSource.cs index c66228b74df..e7184746b22 100644 --- a/src/OpenTelemetry.Instrumentation.AspNet/Implementation/AspNetInstrumentationEventSource.cs +++ b/src/OpenTelemetry.Instrumentation.AspNet/Implementation/AspNetInstrumentationEventSource.cs @@ -24,50 +24,44 @@ namespace OpenTelemetry.Instrumentation.AspNet.Implementation /// EventSource events emitted from the project. /// [EventSource(Name = "OpenTelemetry-Instrumentation-AspNet")] - internal class AspNetInstrumentationEventSource : EventSource + internal sealed class AspNetInstrumentationEventSource : EventSource { public static AspNetInstrumentationEventSource Log = new AspNetInstrumentationEventSource(); [NonEvent] - public void RequestFilterException(Exception ex) + public void RequestFilterException(string operationName, Exception ex) { if (this.IsEnabled(EventLevel.Error, EventKeywords.All)) { - this.RequestFilterException(ex.ToInvariantString()); + this.RequestFilterException(operationName, ex.ToInvariantString()); } } - [Event(1, Message = "Payload is NULL in event '{1}' from handler '{0}', span will not be recorded.", Level = EventLevel.Warning)] - public void NullPayload(string handlerName, string eventName) - { - this.WriteEvent(1, handlerName, eventName); - } - - [Event(2, Message = "Request is filtered out.", Level = EventLevel.Verbose)] - public void RequestIsFilteredOut(string eventName) + [NonEvent] + public void EnrichmentException(string eventName, Exception ex) { - this.WriteEvent(2, eventName); + if (this.IsEnabled(EventLevel.Error, EventKeywords.All)) + { + this.EnrichmentException(eventName, ex.ToInvariantString()); + } } - [Event(3, Message = "InstrumentationFilter threw exception. Request will not be collected. Exception {0}.", Level = EventLevel.Error)] - public void RequestFilterException(string exception) + [Event(1, Message = "Request is filtered out and will not be collected. Operation='{0}'", Level = EventLevel.Verbose)] + public void RequestIsFilteredOut(string operationName) { - this.WriteEvent(3, exception); + this.WriteEvent(1, operationName); } - [NonEvent] - public void EnrichmentException(Exception ex) + [Event(2, Message = "Filter callback threw an exception. Request will not be collected. Operation='{0}': {1}", Level = EventLevel.Error)] + public void RequestFilterException(string operationName, string exception) { - if (this.IsEnabled(EventLevel.Error, EventKeywords.All)) - { - this.EnrichmentException(ex.ToInvariantString()); - } + this.WriteEvent(2, operationName, exception); } - [Event(4, Message = "Enrichment threw exception. Exception {0}.", Level = EventLevel.Error)] - public void EnrichmentException(string exception) + [Event(3, Message = "Enrich callback threw an exception. Event='{0}': {1}", Level = EventLevel.Error)] + public void EnrichmentException(string eventName, string exception) { - this.WriteEvent(4, exception); + this.WriteEvent(3, eventName, exception); } } } diff --git a/src/OpenTelemetry.Instrumentation.AspNet/Implementation/HttpInListener.cs b/src/OpenTelemetry.Instrumentation.AspNet/Implementation/HttpInListener.cs index 07867a036c1..3253f3c8f10 100644 --- a/src/OpenTelemetry.Instrumentation.AspNet/Implementation/HttpInListener.cs +++ b/src/OpenTelemetry.Instrumentation.AspNet/Implementation/HttpInListener.cs @@ -15,9 +15,7 @@ // using System; -using System.Collections.Generic; using System.Diagnostics; -using System.Reflection; using System.Web; using System.Web.Routing; using OpenTelemetry.Context.Propagation; @@ -25,93 +23,57 @@ namespace OpenTelemetry.Instrumentation.AspNet.Implementation { - internal class HttpInListener : ListenerHandler + internal sealed class HttpInListener : IDisposable { - internal const string ActivityNameByHttpInListener = "ActivityCreatedByHttpInListener"; - internal const string ActivityOperationName = "Microsoft.AspNet.HttpReqIn"; - internal static readonly AssemblyName AssemblyName = typeof(HttpInListener).Assembly.GetName(); - internal static readonly string ActivitySourceName = AssemblyName.Name; - internal static readonly Version Version = AssemblyName.Version; - internal static readonly ActivitySource ActivitySource = new ActivitySource(ActivitySourceName, Version.ToString()); - private static readonly Func> 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; - public HttpInListener(string name, AspNetInstrumentationOptions options) - : base(name) + public HttpInListener(AspNetInstrumentationOptions options) { this.options = options ?? throw new ArgumentNullException(nameof(options)); - } - [System.Diagnostics.CodeAnalysis.SuppressMessage("Reliability", "CA2000:Dispose objects before losing scope", Justification = "Activity is retrieved from Activity.Current later and disposed.")] - public override void OnStartActivity(Activity activity, object payload) - { - // The overall flow of what AspNet library does is as below: - // Activity.Start() - // DiagnosticSource.WriteEvent("Start", payload) - // DiagnosticSource.WriteEvent("Stop", payload) - // Activity.Stop() + TelemetryHttpModule.Options.TextMapPropagator = Propagators.DefaultTextMapPropagator; - // This method is in the WriteEvent("Start", payload) path. - // By this time, samplers have already run and - // activity.IsAllDataRequested populated accordingly. + TelemetryHttpModule.Options.OnRequestStartedCallback += this.OnStartActivity; + TelemetryHttpModule.Options.OnRequestStoppedCallback += this.OnStopActivity; + TelemetryHttpModule.Options.OnExceptionCallback += this.OnException; + } - if (Sdk.SuppressInstrumentation) - { - return; - } + public void Dispose() + { + TelemetryHttpModule.Options.OnRequestStartedCallback -= this.OnStartActivity; + TelemetryHttpModule.Options.OnRequestStoppedCallback -= this.OnStopActivity; + TelemetryHttpModule.Options.OnExceptionCallback -= this.OnException; + } - // Ensure context extraction irrespective of sampling decision - var context = HttpContext.Current; - if (context == null) + /// + /// Gets the OpenTelemetry standard uri tag value for a span based on its request . + /// + /// . + /// Span uri value. + private static string GetUriTagValueFromRequestUri(Uri uri) + { + if (string.IsNullOrEmpty(uri.UserInfo)) { - AspNetInstrumentationEventSource.Log.NullPayload(nameof(HttpInListener), nameof(this.OnStartActivity)); - return; + return uri.ToString(); } - var request = context.Request; - var requestValues = request.Unvalidated; - var textMapPropagator = Propagators.DefaultTextMapPropagator; - - if (!(textMapPropagator is TraceContextPropagator)) - { - var ctx = textMapPropagator.Extract(default, request, HttpRequestHeaderValuesGetter); - - if (ctx.ActivityContext.IsValid() - && ctx.ActivityContext != new ActivityContext(activity.TraceId, activity.ParentSpanId, activity.ActivityTraceFlags, activity.TraceStateString, true)) - { - // 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 - // ASP.NET. - Activity newOne = new Activity(ActivityNameByHttpInListener); - newOne.SetParentId(ctx.ActivityContext.TraceId, ctx.ActivityContext.SpanId, ctx.ActivityContext.TraceFlags); - newOne.TraceStateString = ctx.ActivityContext.TraceState; - - // Starting the new activity make it the Activity.Current one. - newOne.Start(); - - // Both new activity and old one store the other activity - // inside them. This is required in the Stop step to - // correctly stop and restore Activity.Current. - newOne.SetCustomProperty("OTel.ActivityByAspNet", activity); - activity.SetCustomProperty("OTel.ActivityByHttpInListener", newOne); - - // Set IsAllDataRequested to false for the activity created by the framework to only export the sibling activity and not the framework activity - activity.IsAllDataRequested = false; - activity = newOne; - } - - if (ctx.Baggage != default) - { - Baggage.Current = ctx.Baggage; - } - } + return string.Concat(uri.Scheme, Uri.SchemeDelimiter, uri.Authority, uri.PathAndQuery, uri.Fragment); + } + private void OnStartActivity(Activity activity, HttpContext context) + { if (activity.IsAllDataRequested) { try { + // todo: Ideally we would also check + // Sdk.SuppressInstrumentation here to prevent tagging a + // span that will not be collected but we can't do that + // without an SDK reference. Need the spec to come around on + // this. + if (this.options.Filter?.Invoke(context) == false) { AspNetInstrumentationEventSource.Log.RequestIsFilteredOut(activity.OperationName); @@ -122,14 +84,14 @@ public override void OnStartActivity(Activity activity, object payload) } catch (Exception ex) { - AspNetInstrumentationEventSource.Log.RequestFilterException(ex); + AspNetInstrumentationEventSource.Log.RequestFilterException(activity.OperationName, ex); activity.IsAllDataRequested = false; activity.ActivityTraceFlags &= ~ActivityTraceFlags.Recorded; return; } - ActivityInstrumentationHelper.SetActivitySourceProperty(activity, ActivitySource); - ActivityInstrumentationHelper.SetKindProperty(activity, ActivityKind.Server); + var request = context.Request; + var requestValues = request.Unvalidated; // see the spec https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/semantic_conventions/http.md var path = requestValues.Path; @@ -155,52 +117,22 @@ public override void OnStartActivity(Activity activity, object payload) } catch (Exception ex) { - AspNetInstrumentationEventSource.Log.EnrichmentException(ex); + AspNetInstrumentationEventSource.Log.EnrichmentException("OnStartActivity", ex); } } } - public override void OnStopActivity(Activity activity, object payload) + private void OnStopActivity(Activity activity, HttpContext context) { - Activity activityToEnrich = activity; - Activity createdActivity = null; - - var textMapPropagator = Propagators.DefaultTextMapPropagator; - bool isCustomPropagator = !(textMapPropagator is TraceContextPropagator); - - if (isCustomPropagator) - { - // If using custom context propagator, then the activity here - // could be either the one from Asp.Net, or the one - // this instrumentation created in Start. - // This is because Asp.Net, under certain circumstances, restores Activity.Current - // to its own activity. - if (activity.OperationName.Equals(ActivityOperationName, StringComparison.Ordinal)) - { - // This block is hit if Asp.Net did restore Current to its own activity, - // and we need to retrieve the one created by HttpInListener, - // or an additional activity was never created. - createdActivity = (Activity)activity.GetCustomProperty("OTel.ActivityByHttpInListener"); - activityToEnrich = createdActivity ?? activity; - } - } - - if (activityToEnrich.IsAllDataRequested) + if (activity.IsAllDataRequested) { - var context = HttpContext.Current; - if (context == null) - { - AspNetInstrumentationEventSource.Log.NullPayload(nameof(HttpInListener), nameof(this.OnStopActivity)); - return; - } - var response = context.Response; - activityToEnrich.SetTag(SemanticConventions.AttributeHttpStatusCode, response.StatusCode); + activity.SetTag(SemanticConventions.AttributeHttpStatusCode, response.StatusCode); - if (activityToEnrich.GetStatus().StatusCode == StatusCode.Unset) + if (activity.GetStatus().StatusCode == StatusCode.Unset) { - activityToEnrich.SetStatus(SpanHelper.ResolveSpanStatusForHttpStatusCode(response.StatusCode)); + activity.SetStatus(SpanHelper.ResolveSpanStatusForHttpStatusCode(response.StatusCode)); } var routeData = context.Request.RequestContext.RouteData; @@ -227,58 +159,45 @@ public override void OnStopActivity(Activity activity, object payload) if (!string.IsNullOrEmpty(template)) { // Override the name that was previously set to the path part of URL. - activityToEnrich.DisplayName = template; - activityToEnrich.SetTag(SemanticConventions.AttributeHttpRoute, template); + activity.DisplayName = template; + activity.SetTag(SemanticConventions.AttributeHttpRoute, template); } try { - this.options.Enrich?.Invoke(activityToEnrich, "OnStopActivity", response); + this.options.Enrich?.Invoke(activity, "OnStopActivity", response); } catch (Exception ex) { - AspNetInstrumentationEventSource.Log.EnrichmentException(ex); + AspNetInstrumentationEventSource.Log.EnrichmentException("OnStopActivity", ex); } } + } - if (isCustomPropagator) + private void OnException(Activity activity, HttpContext context, Exception exception) + { + if (activity.IsAllDataRequested) { - if (activity.OperationName.Equals(ActivityNameByHttpInListener, StringComparison.Ordinal)) + if (this.options.RecordException) { - // If instrumentation started a new Activity, it must - // be stopped here. - activity.Stop(); + activity.RecordException(exception); + } - // Restore the original activity as Current. - var activityByAspNet = (Activity)activity.GetCustomProperty("OTel.ActivityByAspNet"); - Activity.Current = activityByAspNet; + activity.SetStatus(Status.Error.WithDescription(exception.Message)); + + try + { + this.options.Enrich?.Invoke(activity, "OnException", exception); } - else if (createdActivity != null) + catch (Exception ex) { - // This block is hit if Asp.Net did restore Current to its own activity, - // then we need to retrieve the one created by HttpInListener - // and stop it. - createdActivity.Stop(); - - // Restore current back to the one created by Asp.Net - Activity.Current = activity; + AspNetInstrumentationEventSource.Log.EnrichmentException("OnException", ex); } } - } - - /// - /// Gets the OpenTelemetry standard uri tag value for a span based on its request . - /// - /// . - /// Span uri value. - private static string GetUriTagValueFromRequestUri(Uri uri) - { - if (string.IsNullOrEmpty(uri.UserInfo)) + else { - return uri.ToString(); + activity.SetStatus(Status.Error); } - - return string.Concat(uri.Scheme, Uri.SchemeDelimiter, uri.Authority, uri.PathAndQuery, uri.Fragment); } } } diff --git a/src/OpenTelemetry.Instrumentation.AspNet/OpenTelemetry.Instrumentation.AspNet.csproj b/src/OpenTelemetry.Instrumentation.AspNet/OpenTelemetry.Instrumentation.AspNet.csproj index 46790741b95..353cf46280b 100644 --- a/src/OpenTelemetry.Instrumentation.AspNet/OpenTelemetry.Instrumentation.AspNet.csproj +++ b/src/OpenTelemetry.Instrumentation.AspNet/OpenTelemetry.Instrumentation.AspNet.csproj @@ -3,7 +3,7 @@ net461 ASP.NET instrumentation for OpenTelemetry .NET $(PackageTags);distributed-tracing;AspNet;MVC;WebAPI - true + true @@ -11,7 +11,11 @@ - + + + + + diff --git a/src/OpenTelemetry.Instrumentation.AspNet/TracerProviderBuilderExtensions.cs b/src/OpenTelemetry.Instrumentation.AspNet/TracerProviderBuilderExtensions.cs index 372c7c38731..035b6c9e8da 100644 --- a/src/OpenTelemetry.Instrumentation.AspNet/TracerProviderBuilderExtensions.cs +++ b/src/OpenTelemetry.Instrumentation.AspNet/TracerProviderBuilderExtensions.cs @@ -16,7 +16,6 @@ using System; using OpenTelemetry.Instrumentation.AspNet; -using OpenTelemetry.Instrumentation.AspNet.Implementation; namespace OpenTelemetry.Trace { @@ -44,9 +43,7 @@ public static TracerProviderBuilder AddAspNetInstrumentation( configureAspNetInstrumentationOptions?.Invoke(aspnetOptions); builder.AddInstrumentation(() => new AspNetInstrumentation(aspnetOptions)); - builder.AddSource(HttpInListener.ActivitySourceName); - builder.AddLegacySource("Microsoft.AspNet.HttpReqIn"); // for the activities created by AspNetCore - builder.AddLegacySource("ActivityCreatedByHttpInListener"); // for the sibling activities created by the instrumentation library + builder.AddSource(TelemetryHttpModule.AspNetSourceName); return builder; } From 12b0303f74d10b9e5f7cad6fa71bfd0a1ba4725e Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Tue, 17 Aug 2021 20:36:44 -0700 Subject: [PATCH 4/9] New design for TelemetryHttpModule using ActivitySource + OpenTelemetry.API part 4 (#2258) * Fixed up TelemetryHttpModule unit tests. * Added tests for the new HasStarted helper and added checks for StartedButNotSampledObj when not sampled. * Code review. --- .../ActivityHelper.cs | 36 +- .../ActivityExtensionsTest.cs | 322 --------------- .../ActivityHelperTest.cs | 371 ++++++++---------- .../PropertyExtensions.cs | 28 -- .../TestDiagnosticListener.cs | 44 --- 5 files changed, 186 insertions(+), 615 deletions(-) delete mode 100644 test/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule.Tests/ActivityExtensionsTest.cs delete mode 100644 test/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule.Tests/PropertyExtensions.cs delete mode 100644 test/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule.Tests/TestDiagnosticListener.cs diff --git a/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/ActivityHelper.cs b/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/ActivityHelper.cs index 29711309204..da7cd234384 100644 --- a/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/ActivityHelper.cs +++ b/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/ActivityHelper.cs @@ -32,25 +32,27 @@ internal static class ActivityHelper /// /// Key to store the activity in HttpContext. /// - private const string ActivityKey = "__AspnetActivity__"; + internal const string ActivityKey = "__AspnetActivity__"; + internal static readonly object StartedButNotSampledObj = new object(); private static readonly Version Version = typeof(ActivityHelper).Assembly.GetName().Version; private static readonly ActivitySource AspNetSource = new ActivitySource(TelemetryHttpModule.AspNetSourceName, Version.ToString()); private static readonly Func> HttpRequestHeaderValuesGetter = (request, name) => request.Headers.GetValues(name); - private static readonly object StartedButNotSampledObj = new object(); /// /// Try to get the started for the running . /// - /// . + /// . /// Started or if 1) start has not been called or 2) start was /// called but sampling decided not to create an instance. /// if start has been called. - public static bool HasStarted(HttpContext httpContext, out Activity aspNetActivity) + public static bool HasStarted(HttpContext context, out Activity aspNetActivity) { - object itemValue = httpContext.Items[ActivityKey]; + Debug.Assert(context != null, "Context is null."); + + object itemValue = context.Items[ActivityKey]; if (itemValue is Activity activity) { aspNetActivity = activity; @@ -65,11 +67,13 @@ public static bool HasStarted(HttpContext httpContext, out Activity aspNetActivi /// Creates root (first level) activity that describes incoming request. /// /// . - /// Current HttpContext. + /// . /// Callback action. /// New root activity. public static Activity StartAspNetActivity(TextMapPropagator textMapPropagator, HttpContext context, Action onRequestStartedCallback) { + Debug.Assert(context != null, "Context is null."); + PropagationContext propagationContext = textMapPropagator.Extract(default, context.Request, HttpRequestHeaderValuesGetter); Activity activity = AspNetSource.StartActivity(TelemetryHttpModule.AspNetActivityName, ActivityKind.Server, propagationContext.ActivityContext); @@ -112,19 +116,25 @@ public static Activity StartAspNetActivity(TextMapPropagator textMapPropagator, /// Stops the activity and notifies listeners about it. /// /// . - /// Current HttpContext. + /// . /// Callback action. [MethodImpl(MethodImplOptions.AggressiveInlining)] public static void StopAspNetActivity(Activity aspNetActivity, HttpContext context, Action onRequestStoppedCallback) { + Debug.Assert(context != null, "Context is null."); + if (aspNetActivity == null) { + Debug.Assert(context.Items[ActivityKey] == StartedButNotSampledObj, "Context item is not StartedButNotSampledObj."); + // This is the case where a start was called but no activity was // created due to a sampling decision. context.Items[ActivityKey] = null; return; } + Debug.Assert(context.Items[ActivityKey] is Activity, "Context item is not an Activity instance."); + var currentActivity = Activity.Current; aspNetActivity.Stop(); @@ -147,9 +157,19 @@ public static void StopAspNetActivity(Activity aspNetActivity, HttpContext conte } } + /// + /// Notifies listeners about an unhandled exception thrown on the . + /// + /// . + /// . + /// . + /// Callback action. [MethodImpl(MethodImplOptions.AggressiveInlining)] public static void WriteActivityException(Activity aspNetActivity, HttpContext context, Exception exception, Action onExceptionCallback) { + Debug.Assert(context != null, "Context is null."); + Debug.Assert(exception != null, "Exception is null."); + if (aspNetActivity != null) { try @@ -175,6 +195,8 @@ public static void WriteActivityException(Activity aspNetActivity, HttpContext c [MethodImpl(MethodImplOptions.AggressiveInlining)] internal static void RestoreActivityIfNeeded(IDictionary contextItems) { + Debug.Assert(contextItems != null, "Context Items is null."); + if (Activity.Current == null && contextItems[ActivityKey] is Activity aspNetActivity) { Activity.Current = aspNetActivity; diff --git a/test/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule.Tests/ActivityExtensionsTest.cs b/test/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule.Tests/ActivityExtensionsTest.cs deleted file mode 100644 index 8bb6505ca48..00000000000 --- a/test/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule.Tests/ActivityExtensionsTest.cs +++ /dev/null @@ -1,322 +0,0 @@ -// -// 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. -// - -namespace OpenTelemetry.Instrumentation.AspNet.Tests -{ - using System.Collections.Generic; - using System.Collections.Specialized; - using System.Diagnostics; - using System.Linq; - using Xunit; - - public class ActivityExtensionsTest - { - private const string TestActivityName = "Activity.Test"; - - [Fact] - public void Restore_Nothing_If_Header_Does_Not_Contain_RequestId() - { - var activity = new Activity(TestActivityName); - var requestHeaders = new NameValueCollection(); - - Assert.False(activity.Extract(requestHeaders)); - - Assert.True(string.IsNullOrEmpty(activity.ParentId)); - Assert.Null(activity.TraceStateString); - Assert.Empty(activity.Baggage); - } - - [Fact] - public void Can_Restore_First_RequestId_When_Multiple_RequestId_In_Headers() - { - var activity = new Activity(TestActivityName); - var requestHeaders = new NameValueCollection - { - { ActivityExtensions.RequestIdHeaderName, "|aba2f1e978b11111.1" }, - { ActivityExtensions.RequestIdHeaderName, "|aba2f1e978b22222.1" }, - }; - Assert.True(activity.Extract(requestHeaders)); - - Assert.Equal("|aba2f1e978b11111.1", activity.ParentId); - Assert.Empty(activity.Baggage); - } - - [Fact] - public void Extract_RequestId_Is_Ignored_When_Traceparent_Is_Present() - { - var activity = new Activity(TestActivityName); - var requestHeaders = new NameValueCollection - { - { ActivityExtensions.RequestIdHeaderName, "|aba2f1e978b11111.1" }, - { ActivityExtensions.TraceparentHeaderName, "00-0123456789abcdef0123456789abcdef-0123456789abcdef-00" }, - }; - Assert.True(activity.Extract(requestHeaders)); - - activity.Start(); - Assert.Equal(ActivityIdFormat.W3C, activity.IdFormat); - Assert.Equal("00-0123456789abcdef0123456789abcdef-0123456789abcdef-00", activity.ParentId); - Assert.Equal("0123456789abcdef0123456789abcdef", activity.TraceId.ToHexString()); - Assert.Equal("0123456789abcdef", activity.ParentSpanId.ToHexString()); - Assert.False(activity.Recorded); - - Assert.Null(activity.TraceStateString); - Assert.Empty(activity.Baggage); - } - - [Fact] - public void Can_Extract_First_Traceparent_When_Multiple_Traceparents_In_Headers() - { - var activity = new Activity(TestActivityName); - var requestHeaders = new NameValueCollection - { - { ActivityExtensions.TraceparentHeaderName, "00-0123456789abcdef0123456789abcdef-0123456789abcdef-00" }, - { ActivityExtensions.TraceparentHeaderName, "00-fedcba09876543210fedcba09876543210-fedcba09876543210-01" }, - }; - Assert.True(activity.Extract(requestHeaders)); - - activity.Start(); - Assert.Equal(ActivityIdFormat.W3C, activity.IdFormat); - Assert.Equal("00-0123456789abcdef0123456789abcdef-0123456789abcdef-00", activity.ParentId); - Assert.Equal("0123456789abcdef0123456789abcdef", activity.TraceId.ToHexString()); - Assert.Equal("0123456789abcdef", activity.ParentSpanId.ToHexString()); - Assert.False(activity.Recorded); - - Assert.Null(activity.TraceStateString); - Assert.Empty(activity.Baggage); - } - - [Fact] - public void Can_Extract_RootActivity_From_W3C_Headers_And_CC() - { - var activity = new Activity(TestActivityName); - var requestHeaders = new NameValueCollection - { - { ActivityExtensions.TraceparentHeaderName, "00-0123456789abcdef0123456789abcdef-0123456789abcdef-01" }, - { ActivityExtensions.TracestateHeaderName, "ts1=v1,ts2=v2" }, - { ActivityExtensions.CorrelationContextHeaderName, "key1=123,key2=456,key3=789" }, - }; - - Assert.True(activity.Extract(requestHeaders)); - activity.Start(); - Assert.Equal(ActivityIdFormat.W3C, activity.IdFormat); - Assert.Equal("0123456789abcdef0123456789abcdef", activity.TraceId.ToHexString()); - Assert.Equal("0123456789abcdef", activity.ParentSpanId.ToHexString()); - Assert.True(activity.Recorded); - - Assert.Equal("ts1=v1,ts2=v2", activity.TraceStateString); - var baggageItems = new List> - { - new KeyValuePair("key1", "123"), - new KeyValuePair("key2", "456"), - new KeyValuePair("key3", "789"), - }; - var expectedBaggage = baggageItems.OrderBy(kvp => kvp.Key); - var actualBaggage = activity.Baggage.OrderBy(kvp => kvp.Key); - Assert.Equal(expectedBaggage, actualBaggage); - } - - [Fact] - public void Can_Extract_Empty_Traceparent() - { - var activity = new Activity(TestActivityName); - var requestHeaders = new NameValueCollection - { - { ActivityExtensions.TraceparentHeaderName, string.Empty }, - }; - - Assert.False(activity.Extract(requestHeaders)); - - Assert.Equal(default, activity.ParentSpanId); - Assert.Null(activity.ParentId); - } - - [Fact] - public void Can_Extract_Multi_Line_Tracestate() - { - var activity = new Activity(TestActivityName); - var requestHeaders = new NameValueCollection - { - { ActivityExtensions.TraceparentHeaderName, "00-0123456789abcdef0123456789abcdef-0123456789abcdef-01" }, - { ActivityExtensions.TracestateHeaderName, "ts1=v1" }, - { ActivityExtensions.TracestateHeaderName, "ts2=v2" }, - }; - - Assert.True(activity.Extract(requestHeaders)); - activity.Start(); - Assert.Equal(ActivityIdFormat.W3C, activity.IdFormat); - Assert.Equal("0123456789abcdef0123456789abcdef", activity.TraceId.ToHexString()); - Assert.Equal("0123456789abcdef", activity.ParentSpanId.ToHexString()); - Assert.True(activity.Recorded); - - Assert.Equal("ts1=v1,ts2=v2", activity.TraceStateString); - } - - [Fact] - public void Restore_Empty_RequestId_Should_Not_Throw_Exception() - { - var activity = new Activity(TestActivityName); - var requestHeaders = new NameValueCollection - { - { ActivityExtensions.RequestIdHeaderName, string.Empty }, - }; - Assert.False(activity.Extract(requestHeaders)); - - Assert.Null(activity.ParentId); - Assert.Empty(activity.Baggage); - } - - [Fact] - public void Restore_Empty_Traceparent_Should_Not_Throw_Exception() - { - var activity = new Activity(TestActivityName); - var requestHeaders = new NameValueCollection - { - { ActivityExtensions.TraceparentHeaderName, string.Empty }, - }; - Assert.False(activity.Extract(requestHeaders)); - - Assert.Null(activity.ParentId); - Assert.Null(activity.TraceStateString); - Assert.Empty(activity.Baggage); - } - - [Fact] - public void Can_Restore_Baggages_When_CorrelationContext_In_Headers() - { - var activity = new Activity(TestActivityName); - var requestHeaders = new NameValueCollection - { - { ActivityExtensions.RequestIdHeaderName, "|aba2f1e978b11111.1" }, - { ActivityExtensions.CorrelationContextHeaderName, "key1=123,key2=456,key3=789" }, - }; - Assert.True(activity.Extract(requestHeaders)); - - Assert.Equal("|aba2f1e978b11111.1", activity.ParentId); - var baggageItems = new List> - { - new KeyValuePair("key1", "123"), - new KeyValuePair("key2", "456"), - new KeyValuePair("key3", "789"), - }; - var expectedBaggage = baggageItems.OrderBy(kvp => kvp.Key); - var actualBaggage = activity.Baggage.OrderBy(kvp => kvp.Key); - Assert.Equal(expectedBaggage, actualBaggage); - } - - [Fact] - public void Can_Restore_Baggages_When_Multiple_CorrelationContext_In_Headers() - { - var activity = new Activity(TestActivityName); - var requestHeaders = new NameValueCollection - { - { ActivityExtensions.RequestIdHeaderName, "|aba2f1e978b11111.1" }, - { ActivityExtensions.CorrelationContextHeaderName, "key1=123,key2=456,key3=789" }, - { ActivityExtensions.CorrelationContextHeaderName, "key4=abc,key5=def" }, - { ActivityExtensions.CorrelationContextHeaderName, "key6=xyz" }, - }; - Assert.True(activity.Extract(requestHeaders)); - - Assert.Equal("|aba2f1e978b11111.1", activity.ParentId); - var baggageItems = new List> - { - new KeyValuePair("key1", "123"), - new KeyValuePair("key2", "456"), - new KeyValuePair("key3", "789"), - new KeyValuePair("key4", "abc"), - new KeyValuePair("key5", "def"), - new KeyValuePair("key6", "xyz"), - }; - var expectedBaggage = baggageItems.OrderBy(kvp => kvp.Key); - var actualBaggage = activity.Baggage.OrderBy(kvp => kvp.Key); - Assert.Equal(expectedBaggage, actualBaggage); - } - - [Fact] - public void Can_Restore_Baggages_When_Some_MalFormat_CorrelationContext_In_Headers() - { - var activity = new Activity(TestActivityName); - var requestHeaders = new NameValueCollection - { - { ActivityExtensions.RequestIdHeaderName, "|aba2f1e978b11111.1" }, - { ActivityExtensions.CorrelationContextHeaderName, "key1=123,key2=456,key3=789" }, - { ActivityExtensions.CorrelationContextHeaderName, "key4=abc;key5=def" }, - { ActivityExtensions.CorrelationContextHeaderName, "key6????xyz" }, - { ActivityExtensions.CorrelationContextHeaderName, "key7=123=456" }, - }; - Assert.True(activity.Extract(requestHeaders)); - - Assert.Equal("|aba2f1e978b11111.1", activity.ParentId); - var baggageItems = new List> - { - new KeyValuePair("key1", "123"), - new KeyValuePair("key2", "456"), - new KeyValuePair("key3", "789"), - }; - var expectedBaggage = baggageItems.OrderBy(kvp => kvp.Key); - var actualBaggage = activity.Baggage.OrderBy(kvp => kvp.Key); - Assert.Equal(expectedBaggage, actualBaggage); - } - - [Theory] - [InlineData( - "key0=value0,key1=value1,key2=value2,key3=value3,key4=value4,key5=value5,key6=value6,key7=value7,key8=value8,key9=value9," + - "key10=value10,key11=value11,key12=value12,key13=value13,key14=value14,key15=value15,key16=value16,key17=value17,key18=value18,key19=value19," + - "key20=value20,key21=value21,key22=value22,key23=value23,key24=value24,key25=value25,key26=value26,key27=value27,key28=value28,key29=value29," + - "key30=value30,key31=value31,key32=value32,key33=value33,key34=value34,key35=value35,key36=value36,key37=value37,key38=value38,key39=value39," + - "key40=value40,key41=value41,key42=value42,key43=value43,key44=value44,key45=value45,key46=value46,key47=value47,key48=value48,key49=value49," + - "key50=value50,key51=value51,key52=value52,key53=value53,key54=value54,key55=value55,key56=value56,key57=value57,key58=value58,key59=value59," + - "key60=value60,key61=value61,key62=value62,key63=value63,key64=value64,key65=value65,key66=value66,key67=value67,key68=value68,key69=value69," + - "key70=value70,key71=value71,key72=value72,key73=value73,k100=vx", 1023)] // 1023 chars - [InlineData( - "key0=value0,key1=value1,key2=value2,key3=value3,key4=value4,key5=value5,key6=value6,key7=value7,key8=value8,key9=value9," + - "key10=value10,key11=value11,key12=value12,key13=value13,key14=value14,key15=value15,key16=value16,key17=value17,key18=value18,key19=value19," + - "key20=value20,key21=value21,key22=value22,key23=value23,key24=value24,key25=value25,key26=value26,key27=value27,key28=value28,key29=value29," + - "key30=value30,key31=value31,key32=value32,key33=value33,key34=value34,key35=value35,key36=value36,key37=value37,key38=value38,key39=value39," + - "key40=value40,key41=value41,key42=value42,key43=value43,key44=value44,key45=value45,key46=value46,key47=value47,key48=value48,key49=value49," + - "key50=value50,key51=value51,key52=value52,key53=value53,key54=value54,key55=value55,key56=value56,key57=value57,key58=value58,key59=value59," + - "key60=value60,key61=value61,key62=value62,key63=value63,key64=value64,key65=value65,key66=value66,key67=value67,key68=value68,key69=value69," + - "key70=value70,key71=value71,key72=value72,key73=value73,k100=vx1", 1024)] // 1024 chars - [InlineData( - "key0=value0,key1=value1,key2=value2,key3=value3,key4=value4,key5=value5,key6=value6,key7=value7,key8=value8,key9=value9," + - "key10=value10,key11=value11,key12=value12,key13=value13,key14=value14,key15=value15,key16=value16,key17=value17,key18=value18,key19=value19," + - "key20=value20,key21=value21,key22=value22,key23=value23,key24=value24,key25=value25,key26=value26,key27=value27,key28=value28,key29=value29," + - "key30=value30,key31=value31,key32=value32,key33=value33,key34=value34,key35=value35,key36=value36,key37=value37,key38=value38,key39=value39," + - "key40=value40,key41=value41,key42=value42,key43=value43,key44=value44,key45=value45,key46=value46,key47=value47,key48=value48,key49=value49," + - "key50=value50,key51=value51,key52=value52,key53=value53,key54=value54,key55=value55,key56=value56,key57=value57,key58=value58,key59=value59," + - "key60=value60,key61=value61,key62=value62,key63=value63,key64=value64,key65=value65,key66=value66,key67=value67,key68=value68,key69=value69," + - "key70=value70,key71=value71,key72=value72,key73=value73,key74=value74", 1029)] // more than 1024 chars - public void Validates_Correlation_Context_Length(string correlationContext, int expectedLength) - { - var activity = new Activity(TestActivityName); - var requestHeaders = new NameValueCollection - { - { ActivityExtensions.RequestIdHeaderName, "|abc.1" }, - { ActivityExtensions.CorrelationContextHeaderName, correlationContext }, - }; - Assert.True(activity.Extract(requestHeaders)); - - var baggageItems = Enumerable.Range(0, 74).Select(i => new KeyValuePair("key" + i, "value" + i)).ToList(); - if (expectedLength < 1024) - { - baggageItems.Add(new KeyValuePair("k100", "vx")); - } - - var expectedBaggage = baggageItems.OrderBy(kvp => kvp.Key); - var actualBaggage = activity.Baggage.OrderBy(kvp => kvp.Key); - Assert.Equal(expectedBaggage, actualBaggage); - } - } -} diff --git a/test/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule.Tests/ActivityHelperTest.cs b/test/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule.Tests/ActivityHelperTest.cs index 8cfc2b76919..a256a1c474f 100644 --- a/test/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule.Tests/ActivityHelperTest.cs +++ b/test/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule.Tests/ActivityHelperTest.cs @@ -21,102 +21,92 @@ namespace OpenTelemetry.Instrumentation.AspNet.Tests using System.Collections.Generic; using System.Collections.Specialized; using System.Diagnostics; - using System.Linq; - using System.Reflection; using System.Threading; using System.Threading.Tasks; using System.Web; + using OpenTelemetry.Context.Propagation; using Xunit; public class ActivityHelperTest : IDisposable { + private const string TraceParentHeaderName = "traceparent"; + private const string TraceStateHeaderName = "tracestate"; + private const string BaggageHeaderName = "baggage"; private const string TestActivityName = "Activity.Test"; - private readonly List> baggageItems; private readonly string baggageInHeader; - private IDisposable subscriptionAllListeners; - private IDisposable subscriptionAspNetListener; + private ActivityListener activitySourceListener; public ActivityHelperTest() { - this.baggageItems = new List> - { - new KeyValuePair("TestKey1", "123"), - new KeyValuePair("TestKey2", "456"), - new KeyValuePair("TestKey1", "789"), - }; - this.baggageInHeader = "TestKey1=123,TestKey2=456,TestKey1=789"; - - // reset static fields - var allListenerField = typeof(DiagnosticListener). - GetField("s_allListenerObservable", BindingFlags.Static | BindingFlags.NonPublic); - allListenerField.SetValue(null, null); - var aspnetListenerField = typeof(ActivityHelper). - GetField("AspNetListener", BindingFlags.Static | BindingFlags.NonPublic); - aspnetListenerField.SetValue(null, new DiagnosticListener(ActivityHelper.AspNetListenerName)); } public void Dispose() { - this.subscriptionAspNetListener?.Dispose(); - this.subscriptionAllListeners?.Dispose(); + this.activitySourceListener?.Dispose(); } [Fact] - public void Can_Restore_Activity() + public void Has_Started_Returns_Correctly() { - this.EnableAll(); var context = HttpContextHelper.GetFakeHttpContext(); - var rootActivity = ActivityHelper.CreateRootActivity(context, false); - rootActivity.AddTag("k1", "v1"); - rootActivity.AddTag("k2", "v2"); - Activity.Current = null; + bool result = ActivityHelper.HasStarted(context, out Activity aspNetActivity); - ActivityHelper.RestoreActivityIfNeeded(context.Items); + Assert.False(result); + Assert.Null(aspNetActivity); - Assert.Same(Activity.Current, rootActivity); + context.Items[ActivityHelper.ActivityKey] = ActivityHelper.StartedButNotSampledObj; + + result = ActivityHelper.HasStarted(context, out aspNetActivity); + + Assert.True(result); + Assert.Null(aspNetActivity); + + Activity activity = new Activity(TestActivityName); + context.Items[ActivityHelper.ActivityKey] = activity; + + result = ActivityHelper.HasStarted(context, out aspNetActivity); + + Assert.True(result); + Assert.NotNull(aspNetActivity); + Assert.Equal(activity, aspNetActivity); } [Fact] - public void Can_Stop_Lost_Activity() + public void Can_Restore_Activity() { - this.EnableAll(pair => - { - Assert.NotNull(Activity.Current); - Assert.Equal(ActivityHelper.AspNetActivityName, Activity.Current.OperationName); - }); + this.EnableListener(); var context = HttpContextHelper.GetFakeHttpContext(); - var rootActivity = ActivityHelper.CreateRootActivity(context, false); + var rootActivity = ActivityHelper.StartAspNetActivity(new NoopTextMapPropagator(), context, null); rootActivity.AddTag("k1", "v1"); rootActivity.AddTag("k2", "v2"); Activity.Current = null; - ActivityHelper.StopAspNetActivity(context.Items); - Assert.True(rootActivity.Duration != TimeSpan.Zero); - Assert.Null(Activity.Current); - Assert.Null(context.Items[ActivityHelper.ActivityKey]); + ActivityHelper.RestoreActivityIfNeeded(context.Items); + + Assert.Same(Activity.Current, rootActivity); } [Fact] - public void Can_Not_Stop_Lost_Activity_If_Not_In_Context() + public void Can_Stop_Lost_Activity() { - this.EnableAll(pair => + this.EnableListener(a => { Assert.NotNull(Activity.Current); - Assert.Equal(ActivityHelper.AspNetActivityName, Activity.Current.OperationName); + Assert.Equal(Activity.Current, a); + Assert.Equal(TelemetryHttpModule.AspNetActivityName, Activity.Current.OperationName); }); var context = HttpContextHelper.GetFakeHttpContext(); - var rootActivity = ActivityHelper.CreateRootActivity(context, false); - context.Items.Remove(ActivityHelper.ActivityKey); + var rootActivity = ActivityHelper.StartAspNetActivity(new NoopTextMapPropagator(), context, null); rootActivity.AddTag("k1", "v1"); rootActivity.AddTag("k2", "v2"); Activity.Current = null; - ActivityHelper.StopAspNetActivity(context.Items); - Assert.True(rootActivity.Duration == TimeSpan.Zero); + ActivityHelper.StopAspNetActivity(rootActivity, context, null); + Assert.True(rootActivity.Duration != TimeSpan.Zero); Assert.Null(Activity.Current); Assert.Null(context.Items[ActivityHelper.ActivityKey]); } @@ -124,7 +114,7 @@ public void Can_Not_Stop_Lost_Activity_If_Not_In_Context() [Fact] public void Do_Not_Restore_Activity_When_There_Is_No_Activity_In_Context() { - this.EnableAll(); + this.EnableListener(); ActivityHelper.RestoreActivityIfNeeded(HttpContextHelper.GetFakeHttpContext().Items); Assert.Null(Activity.Current); @@ -133,14 +123,12 @@ public void Do_Not_Restore_Activity_When_There_Is_No_Activity_In_Context() [Fact] public void Do_Not_Restore_Activity_When_It_Is_Not_Lost() { - this.EnableAll(); + this.EnableListener(); var root = new Activity("root").Start(); var context = HttpContextHelper.GetFakeHttpContext(); context.Items[ActivityHelper.ActivityKey] = root; - var module = new TelemetryHttpModule(); - ActivityHelper.RestoreActivityIfNeeded(context.Items); Assert.Equal(root, Activity.Current); @@ -150,11 +138,11 @@ public void Do_Not_Restore_Activity_When_It_Is_Not_Lost() public void Can_Stop_Activity_Without_AspNetListener_Enabled() { var context = HttpContextHelper.GetFakeHttpContext(); - var rootActivity = this.CreateActivity(); + var rootActivity = new Activity(TestActivityName); rootActivity.Start(); context.Items[ActivityHelper.ActivityKey] = rootActivity; Thread.Sleep(100); - ActivityHelper.StopAspNetActivity(context.Items); + ActivityHelper.StopAspNetActivity(rootActivity, context, null); Assert.True(rootActivity.Duration != TimeSpan.Zero); Assert.Null(rootActivity.Parent); @@ -165,12 +153,12 @@ public void Can_Stop_Activity_Without_AspNetListener_Enabled() public void Can_Stop_Activity_With_AspNetListener_Enabled() { var context = HttpContextHelper.GetFakeHttpContext(); - var rootActivity = this.CreateActivity(); + var rootActivity = new Activity(TestActivityName); rootActivity.Start(); context.Items[ActivityHelper.ActivityKey] = rootActivity; Thread.Sleep(100); - this.EnableAspNetListenerOnly(); - ActivityHelper.StopAspNetActivity(context.Items); + this.EnableListener(); + ActivityHelper.StopAspNetActivity(rootActivity, context, null); Assert.True(rootActivity.Duration != TimeSpan.Zero); Assert.Null(rootActivity.Parent); @@ -180,14 +168,14 @@ public void Can_Stop_Activity_With_AspNetListener_Enabled() [Fact] public void Can_Stop_Root_Activity_With_All_Children() { - this.EnableAll(); + this.EnableListener(); var context = HttpContextHelper.GetFakeHttpContext(); - var rootActivity = ActivityHelper.CreateRootActivity(context, false); + var rootActivity = ActivityHelper.StartAspNetActivity(new NoopTextMapPropagator(), context, null); var child = new Activity("child").Start(); new Activity("grandchild").Start(); - ActivityHelper.StopAspNetActivity(context.Items); + ActivityHelper.StopAspNetActivity(rootActivity, context, null); Assert.True(rootActivity.Duration != TimeSpan.Zero); Assert.True(child.Duration == TimeSpan.Zero); @@ -198,59 +186,25 @@ public void Can_Stop_Root_Activity_With_All_Children() [Fact] public void Can_Stop_Root_While_Child_Is_Current() { - this.EnableAll(); + this.EnableListener(); var context = HttpContextHelper.GetFakeHttpContext(); - var rootActivity = ActivityHelper.CreateRootActivity(context, false); + var rootActivity = ActivityHelper.StartAspNetActivity(new NoopTextMapPropagator(), context, null); var child = new Activity("child").Start(); - ActivityHelper.StopAspNetActivity(context.Items); + ActivityHelper.StopAspNetActivity(rootActivity, context, null); Assert.True(child.Duration == TimeSpan.Zero); - Assert.Null(Activity.Current); + Assert.NotNull(Activity.Current); + Assert.Equal(Activity.Current, child); Assert.Null(context.Items[ActivityHelper.ActivityKey]); } - [Fact] - public void OnImportActivity_Is_Called() - { - bool onImportIsCalled = false; - Activity importedActivity = null; - this.EnableAll(onImport: (activity, _) => - { - onImportIsCalled = true; - importedActivity = activity; - Assert.Null(Activity.Current); - }); - - var context = HttpContextHelper.GetFakeHttpContext(); - var rootActivity = ActivityHelper.CreateRootActivity(context, false); - Assert.True(onImportIsCalled); - Assert.NotNull(importedActivity); - Assert.Equal(importedActivity, Activity.Current); - Assert.Equal(importedActivity, rootActivity); - } - - [Fact] - public void OnImportActivity_Can_Set_Parent() - { - this.EnableAll(onImport: (activity, _) => - { - Assert.Null(activity.ParentId); - activity.SetParentId("|guid.123."); - }); - - var context = HttpContextHelper.GetFakeHttpContext(); - var rootActivity = ActivityHelper.CreateRootActivity(context, false); - - Assert.Equal("|guid.123.", Activity.Current.ParentId); - } - [Fact] public async Task Can_Stop_Root_Activity_If_It_Is_Broken() { - this.EnableAll(); + this.EnableListener(); var context = HttpContextHelper.GetFakeHttpContext(); - var root = ActivityHelper.CreateRootActivity(context, false); + var root = ActivityHelper.StartAspNetActivity(new NoopTextMapPropagator(), context, null); new Activity("child").Start(); for (int i = 0; i < 2; i++) @@ -269,7 +223,7 @@ await Task.Run(() => // do not affect 'parent' context in which Task.Run is called. // But 'child' Activity is stopped, thus consequent calls to Stop will // not update Current - ActivityHelper.StopAspNetActivity(context.Items); + ActivityHelper.StopAspNetActivity(root, context, null); Assert.True(root.Duration != TimeSpan.Zero); Assert.Null(context.Items[ActivityHelper.ActivityKey]); Assert.Null(Activity.Current); @@ -278,9 +232,9 @@ await Task.Run(() => [Fact] public void Stop_Root_Activity_With_129_Nesting_Depth() { - this.EnableAll(); + this.EnableListener(); var context = HttpContextHelper.GetFakeHttpContext(); - var root = ActivityHelper.CreateRootActivity(context, false); + var root = ActivityHelper.StartAspNetActivity(new NoopTextMapPropagator(), context, null); for (int i = 0; i < 129; i++) { @@ -288,125 +242,130 @@ public void Stop_Root_Activity_With_129_Nesting_Depth() } // can stop any activity regardless of the stack depth - ActivityHelper.StopAspNetActivity(context.Items); + ActivityHelper.StopAspNetActivity(root, context, null); Assert.True(root.Duration != TimeSpan.Zero); Assert.Null(context.Items[ActivityHelper.ActivityKey]); - Assert.Null(Activity.Current); + Assert.NotNull(Activity.Current); } [Fact] public void Should_Not_Create_RootActivity_If_AspNetListener_Not_Enabled() { var context = HttpContextHelper.GetFakeHttpContext(); - var rootActivity = ActivityHelper.CreateRootActivity(context, true); + var rootActivity = ActivityHelper.StartAspNetActivity(new NoopTextMapPropagator(), context, null); Assert.Null(rootActivity); + Assert.Equal(ActivityHelper.StartedButNotSampledObj, context.Items[ActivityHelper.ActivityKey]); + + ActivityHelper.StopAspNetActivity(rootActivity, context, null); + Assert.Null(context.Items[ActivityHelper.ActivityKey]); } [Fact] public void Should_Not_Create_RootActivity_If_AspNetActivity_Not_Enabled() { var context = HttpContextHelper.GetFakeHttpContext(); - this.EnableAspNetListenerOnly(); - var rootActivity = ActivityHelper.CreateRootActivity(context, true); + this.EnableListener(onSample: (context) => ActivitySamplingResult.None); + var rootActivity = ActivityHelper.StartAspNetActivity(new NoopTextMapPropagator(), context, null); Assert.Null(rootActivity); - } + Assert.Equal(ActivityHelper.StartedButNotSampledObj, context.Items[ActivityHelper.ActivityKey]); - [Fact] - public void Should_Not_Create_RootActivity_If_AspNetActivity_Not_Enabled_With_Arguments() - { - var context = HttpContextHelper.GetFakeHttpContext(); - this.EnableAspNetListenerAndDisableActivity(); - var rootActivity = ActivityHelper.CreateRootActivity(context, true); - - Assert.Null(rootActivity); + ActivityHelper.StopAspNetActivity(rootActivity, context, null); + Assert.Null(context.Items[ActivityHelper.ActivityKey]); } [Fact] - public void Can_Create_RootActivity_And_Restore_Info_From_Request_Header() + public void Can_Create_RootActivity_From_W3C_Traceparent() { - this.EnableAll(); + this.EnableListener(); var requestHeaders = new Dictionary { - { ActivityExtensions.RequestIdHeaderName, "|aba2f1e978b2cab6.1." }, - { ActivityExtensions.CorrelationContextHeaderName, this.baggageInHeader }, + { TraceParentHeaderName, "00-0123456789abcdef0123456789abcdef-0123456789abcdef-00" }, }; var context = HttpContextHelper.GetFakeHttpContext(headers: requestHeaders); - this.EnableAspNetListenerAndActivity(); - var rootActivity = ActivityHelper.CreateRootActivity(context, true); + var rootActivity = ActivityHelper.StartAspNetActivity(new TraceContextPropagator(), context, null); Assert.NotNull(rootActivity); - Assert.True(rootActivity.ParentId == "|aba2f1e978b2cab6.1."); - var expectedBaggage = this.baggageItems.OrderBy(item => item.Value); - var actualBaggage = rootActivity.Baggage.OrderBy(item => item.Value); - Assert.Equal(expectedBaggage, actualBaggage); + Assert.Equal(ActivityIdFormat.W3C, rootActivity.IdFormat); + Assert.Equal("00-0123456789abcdef0123456789abcdef-0123456789abcdef-00", rootActivity.ParentId); + Assert.Equal("0123456789abcdef0123456789abcdef", rootActivity.TraceId.ToHexString()); + Assert.Equal("0123456789abcdef", rootActivity.ParentSpanId.ToHexString()); + Assert.True(rootActivity.Recorded); // note: We're not using a parent-based sampler in this test so the recorded flag of traceparent is ignored. + + Assert.Null(rootActivity.TraceStateString); + Assert.Empty(rootActivity.Baggage); + + Assert.Equal(0, Baggage.Current.Count); } [Fact] - public void Can_Create_RootActivity_From_W3C_Traceparent() + public void Can_Create_RootActivityWithTraceState_From_W3C_TraceContext() { - this.EnableAll(); + this.EnableListener(); var requestHeaders = new Dictionary { - { ActivityExtensions.TraceparentHeaderName, "00-0123456789abcdef0123456789abcdef-0123456789abcdef-00" }, + { TraceParentHeaderName, "00-0123456789abcdef0123456789abcdef-0123456789abcdef-01" }, + { TraceStateHeaderName, "ts1=v1,ts2=v2" }, }; var context = HttpContextHelper.GetFakeHttpContext(headers: requestHeaders); - this.EnableAspNetListenerAndActivity(); - var rootActivity = ActivityHelper.CreateRootActivity(context, true); + var rootActivity = ActivityHelper.StartAspNetActivity(new TraceContextPropagator(), context, null); Assert.NotNull(rootActivity); Assert.Equal(ActivityIdFormat.W3C, rootActivity.IdFormat); - Assert.Equal("00-0123456789abcdef0123456789abcdef-0123456789abcdef-00", rootActivity.ParentId); + Assert.Equal("00-0123456789abcdef0123456789abcdef-0123456789abcdef-01", rootActivity.ParentId); Assert.Equal("0123456789abcdef0123456789abcdef", rootActivity.TraceId.ToHexString()); Assert.Equal("0123456789abcdef", rootActivity.ParentSpanId.ToHexString()); - Assert.False(rootActivity.Recorded); + Assert.True(rootActivity.Recorded); - Assert.Null(rootActivity.TraceStateString); + Assert.Equal("ts1=v1,ts2=v2", rootActivity.TraceStateString); Assert.Empty(rootActivity.Baggage); + + Assert.Equal(0, Baggage.Current.Count); } [Fact] - public void Can_Create_RootActivityWithTraceState_From_W3C_TraceContext() + public void Can_Create_RootActivity_From_W3C_Traceparent_With_Baggage() { - this.EnableAll(); + this.EnableListener(); var requestHeaders = new Dictionary { - { ActivityExtensions.TraceparentHeaderName, "00-0123456789abcdef0123456789abcdef-0123456789abcdef-01" }, - { ActivityExtensions.TracestateHeaderName, "ts1=v1,ts2=v2" }, + { TraceParentHeaderName, "00-0123456789abcdef0123456789abcdef-0123456789abcdef-00" }, + { BaggageHeaderName, this.baggageInHeader }, }; var context = HttpContextHelper.GetFakeHttpContext(headers: requestHeaders); - this.EnableAspNetListenerAndActivity(); - var rootActivity = ActivityHelper.CreateRootActivity(context, true); + var rootActivity = ActivityHelper.StartAspNetActivity(new CompositeTextMapPropagator(new TextMapPropagator[] { new TraceContextPropagator(), new BaggagePropagator() }), context, null); Assert.NotNull(rootActivity); Assert.Equal(ActivityIdFormat.W3C, rootActivity.IdFormat); - Assert.Equal("00-0123456789abcdef0123456789abcdef-0123456789abcdef-01", rootActivity.ParentId); + Assert.Equal("00-0123456789abcdef0123456789abcdef-0123456789abcdef-00", rootActivity.ParentId); Assert.Equal("0123456789abcdef0123456789abcdef", rootActivity.TraceId.ToHexString()); Assert.Equal("0123456789abcdef", rootActivity.ParentSpanId.ToHexString()); - Assert.True(rootActivity.Recorded); + Assert.True(rootActivity.Recorded); // note: We're not using a parent-based sampler in this test so the recorded flag of traceparent is ignored. - Assert.Equal("ts1=v1,ts2=v2", rootActivity.TraceStateString); + Assert.Null(rootActivity.TraceStateString); Assert.Empty(rootActivity.Baggage); + + Assert.Equal(2, Baggage.Current.Count); + Assert.Equal("789", Baggage.Current.GetBaggage("TestKey1")); + Assert.Equal("456", Baggage.Current.GetBaggage("TestKey2")); } [Fact] public void Can_Create_RootActivity_And_Ignore_Info_From_Request_Header_If_ParseHeaders_Is_False() { - this.EnableAll(); + this.EnableListener(); var requestHeaders = new Dictionary { - { ActivityExtensions.RequestIdHeaderName, "|aba2f1e978b2cab6.1." }, - { ActivityExtensions.CorrelationContextHeaderName, this.baggageInHeader }, + { TraceParentHeaderName, "00-0123456789abcdef0123456789abcdef-0123456789abcdef-01" }, }; var context = HttpContextHelper.GetFakeHttpContext(headers: requestHeaders); - this.EnableAspNetListenerAndActivity(); - var rootActivity = ActivityHelper.CreateRootActivity(context, parseHeaders: false); + var rootActivity = ActivityHelper.StartAspNetActivity(new NoopTextMapPropagator(), context, null); Assert.NotNull(rootActivity); Assert.Null(rootActivity.ParentId); @@ -416,10 +375,9 @@ public void Can_Create_RootActivity_And_Ignore_Info_From_Request_Header_If_Parse [Fact] public void Can_Create_RootActivity_And_Start_Activity() { - this.EnableAll(); + this.EnableListener(); var context = HttpContextHelper.GetFakeHttpContext(); - this.EnableAspNetListenerAndActivity(); - var rootActivity = ActivityHelper.CreateRootActivity(context, true); + var rootActivity = ActivityHelper.StartAspNetActivity(new NoopTextMapPropagator(), context, null); Assert.NotNull(rootActivity); Assert.True(!string.IsNullOrEmpty(rootActivity.Id)); @@ -428,83 +386,52 @@ public void Can_Create_RootActivity_And_Start_Activity() [Fact] public void Can_Create_RootActivity_And_Saved_In_HttContext() { - this.EnableAll(); + this.EnableListener(); var context = HttpContextHelper.GetFakeHttpContext(); - this.EnableAspNetListenerAndActivity(); - var rootActivity = ActivityHelper.CreateRootActivity(context, true); + var rootActivity = ActivityHelper.StartAspNetActivity(new NoopTextMapPropagator(), context, null); Assert.NotNull(rootActivity); Assert.Same(rootActivity, context.Items[ActivityHelper.ActivityKey]); } - private Activity CreateActivity() + [Fact] + public void Fire_Exception_Events() { - var activity = new Activity(TestActivityName); - this.baggageItems.ForEach(kv => activity.AddBaggage(kv.Key, kv.Value)); + int callbacksFired = 0; - return activity; - } + var context = HttpContextHelper.GetFakeHttpContext(); - private void EnableAll(Action> onNext = null, Action onImport = null) - { - this.subscriptionAllListeners = DiagnosticListener.AllListeners.Subscribe(listener => - { - // if AspNetListener has subscription, then it is enabled - if (listener.Name == ActivityHelper.AspNetListenerName) - { - this.subscriptionAspNetListener = listener.Subscribe( - new TestDiagnosticListener(onNext), - (name, a1, a2) => true, - (a, o) => onImport?.Invoke(a, o), - (a, o) => { }); - } - }); - } + Activity activity = new Activity(TestActivityName); - private void EnableAspNetListenerAndDisableActivity( - Action> onNext = null, - string activityName = ActivityHelper.AspNetActivityName) - { - this.subscriptionAllListeners = DiagnosticListener.AllListeners.Subscribe(listener => - { - // if AspNetListener has subscription, then it is enabled - if (listener.Name == ActivityHelper.AspNetListenerName) - { - this.subscriptionAspNetListener = listener.Subscribe( - new TestDiagnosticListener(onNext), - (name, arg1, arg2) => name == activityName && arg1 == null); - } - }); - } + ActivityHelper.WriteActivityException(activity, context, new InvalidOperationException(), (a, c, e) => { callbacksFired++; }); - private void EnableAspNetListenerAndActivity( - Action> onNext = null, - string activityName = ActivityHelper.AspNetActivityName) - { - this.subscriptionAllListeners = DiagnosticListener.AllListeners.Subscribe(listener => - { - // if AspNetListener has subscription, then it is enabled - if (listener.Name == ActivityHelper.AspNetListenerName) - { - this.subscriptionAspNetListener = listener.Subscribe( - new TestDiagnosticListener(onNext), - (name, arg1, arg2) => name == activityName); - } - }); + ActivityHelper.WriteActivityException(null, context, new InvalidOperationException(), (a, c, e) => { callbacksFired++; }); + + // Callback should fire only for non-null activity + Assert.Equal(1, callbacksFired); } - private void EnableAspNetListenerOnly(Action> onNext = null) + private void EnableListener(Action onStarted = null, Action onStopped = null, Func onSample = null) { - this.subscriptionAllListeners = DiagnosticListener.AllListeners.Subscribe(listener => + Debug.Assert(this.activitySourceListener == null, "Cannot attach multiple listeners in tests."); + + this.activitySourceListener = new ActivityListener { - // if AspNetListener has subscription, then it is enabled - if (listener.Name == ActivityHelper.AspNetListenerName) + ShouldListenTo = (activitySource) => activitySource.Name == TelemetryHttpModule.AspNetSourceName, + ActivityStarted = (a) => onStarted?.Invoke(a), + ActivityStopped = (a) => onStopped?.Invoke(a), + Sample = (ref ActivityCreationOptions options) => { - this.subscriptionAspNetListener = listener.Subscribe( - new TestDiagnosticListener(onNext), - activityName => false); - } - }); + if (onSample != null) + { + return onSample(options.Parent); + } + + return ActivitySamplingResult.AllDataAndRecorded; + }, + }; + + ActivitySource.AddActivityListener(this.activitySourceListener); } private class TestHttpRequest : HttpRequestBase @@ -565,5 +492,21 @@ public TestHttpContext(Exception error = null) public override HttpServerUtilityBase Server { get; } } + + private class NoopTextMapPropagator : TextMapPropagator + { + private static readonly PropagationContext DefaultPropagationContext = default; + + public override ISet Fields => null; + + public override PropagationContext Extract(PropagationContext context, T carrier, Func> getter) + { + return DefaultPropagationContext; + } + + public override void Inject(PropagationContext context, T carrier, Action setter) + { + } + } } } diff --git a/test/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule.Tests/PropertyExtensions.cs b/test/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule.Tests/PropertyExtensions.cs deleted file mode 100644 index 135a4a97236..00000000000 --- a/test/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule.Tests/PropertyExtensions.cs +++ /dev/null @@ -1,28 +0,0 @@ -// -// 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. -// - -namespace OpenTelemetry.Instrumentation.AspNet.Tests -{ - using System.Reflection; - - internal static class PropertyExtensions - { - public static object GetProperty(this object obj, string propertyName) - { - return obj.GetType().GetTypeInfo().GetDeclaredProperty(propertyName)?.GetValue(obj); - } - } -} diff --git a/test/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule.Tests/TestDiagnosticListener.cs b/test/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule.Tests/TestDiagnosticListener.cs deleted file mode 100644 index 4b29e2ab9f2..00000000000 --- a/test/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule.Tests/TestDiagnosticListener.cs +++ /dev/null @@ -1,44 +0,0 @@ -// -// 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. -// - -namespace OpenTelemetry.Instrumentation.AspNet.Tests -{ - using System; - using System.Collections.Generic; - - internal class TestDiagnosticListener : IObserver> - { - private readonly Action> onNextCallBack; - - public TestDiagnosticListener(Action> onNext) - { - this.onNextCallBack = onNext; - } - - public void OnCompleted() - { - } - - public void OnError(Exception error) - { - } - - public void OnNext(KeyValuePair value) - { - this.onNextCallBack?.Invoke(value); - } - } -} From 2752246e217ed238acb1c7d10af9421bbe7850f3 Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Thu, 19 Aug 2021 11:41:51 -0700 Subject: [PATCH 5/9] New design for TelemetryHttpModule using ActivitySource + OpenTelemetry.API part 5 (#2261) * Updated ASP.NET instrumentation tests for new TelemetryHttpModule. * Added a test for the new RecordException option. * Code review. --- .../ActivityHelper.cs | 5 +- .../AssemblyInfo.cs | 2 + .../Implementation/HttpInListener.cs | 4 - .../HttpInListenerTests.cs | 277 +++++------------- ...emetry.Instrumentation.AspNet.Tests.csproj | 11 +- 5 files changed, 85 insertions(+), 214 deletions(-) diff --git a/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/ActivityHelper.cs b/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/ActivityHelper.cs index da7cd234384..10faac6a94f 100644 --- a/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/ActivityHelper.cs +++ b/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/ActivityHelper.cs @@ -35,9 +35,10 @@ internal static class ActivityHelper internal const string ActivityKey = "__AspnetActivity__"; internal static readonly object StartedButNotSampledObj = new object(); - private static readonly Version Version = typeof(ActivityHelper).Assembly.GetName().Version; - private static readonly ActivitySource AspNetSource = new ActivitySource(TelemetryHttpModule.AspNetSourceName, Version.ToString()); private static readonly Func> HttpRequestHeaderValuesGetter = (request, name) => request.Headers.GetValues(name); + private static readonly ActivitySource AspNetSource = new ActivitySource( + TelemetryHttpModule.AspNetSourceName, + typeof(ActivityHelper).Assembly.GetName().Version.ToString()); /// /// Try to get the started for the running (); - propagator.Setup(m => m.Extract(It.IsAny(), It.IsAny(), It.IsAny>>())).Returns(new PropagationContext( - new ActivityContext( - expectedTraceId, - expectedSpanId, - ActivityTraceFlags.Recorded, - isRemote: true), - default)); - - var activity = new Activity(HttpInListener.ActivityOperationName); - if (carrierFormat == "TraceContext" || carrierFormat == "CustomContextMatchParent") - { - activity.SetParentId(expectedTraceId, expectedSpanId, ActivityTraceFlags.Recorded); - } + List exportedItems = new List(16); - var activityProcessor = new Mock>(); - Sdk.SetDefaultTextMapPropagator(propagator.Object); + Sdk.SetDefaultTextMapPropagator(new TraceContextPropagator()); using (openTelemetry = Sdk.CreateTracerProviderBuilder() - .AddAspNetInstrumentation( - (options) => + .AddAspNetInstrumentation((options) => { options.Filter = httpContext => { @@ -177,109 +145,43 @@ public void AspNetRequestsAreCollectedSuccessfully( { options.Enrich = GetEnrichmentAction(default); } + + options.RecordException = recordException; }) - .AddProcessor(activityProcessor.Object).Build()) + .AddInMemoryExporter(exportedItems) + .Build()) { - activity.Start(); + using var inMemoryEventListener = new InMemoryEventListener(AspNetInstrumentationEventSource.Log); - using (var inMemoryEventListener = new InMemoryEventListener(AspNetInstrumentationEventSource.Log)) - { - this.fakeAspNetDiagnosticSource.Write("Start", null); + var activity = ActivityHelper.StartAspNetActivity(Propagators.DefaultTextMapPropagator, HttpContext.Current, TelemetryHttpModule.Options.OnRequestStartedCallback); - if (filter == "{ThrowException}") - { - Assert.Single(inMemoryEventListener.Events.Where((e) => e.EventId == 3)); - } + if (filter == "{ThrowException}") + { + Assert.Single(inMemoryEventListener.Events.Where((e) => e.EventId == 2)); } - if (restoreCurrentActivity) + Assert.Equal(TelemetryHttpModule.AspNetActivityName, Activity.Current.OperationName); + + if (recordException) { - Activity.Current = activity; + ActivityHelper.WriteActivityException(activity, HttpContext.Current, new InvalidOperationException(), TelemetryHttpModule.Options.OnExceptionCallback); } - this.fakeAspNetDiagnosticSource.Write("Stop", null); - - // The above line fires DS event which is listened by Instrumentation. - // Validate that Current activity is still the one created by Asp.Net - Assert.Equal(HttpInListener.ActivityOperationName, Activity.Current.OperationName); - activity.Stop(); + ActivityHelper.StopAspNetActivity(activity, HttpContext.Current, TelemetryHttpModule.Options.OnRequestStoppedCallback); } if (HttpContext.Current.Request.Path == filter || filter == "{ThrowException}") { - // only SetParentProvider/Shutdown/Dispose/OnStart are called because request was filtered. - Assert.Equal(4, activityProcessor.Invocations.Count); + Assert.Empty(exportedItems); return; } - // Validate that Activity.Current is always the one created by Asp.Net - var currentActivity = Activity.Current; - - Activity span; - if (carrierFormat == "CustomContextNonmatchParent") - { - Assert.Equal(6, activityProcessor.Invocations.Count); // SetParentProvider/OnStart(framework activity)/OnStart(sibling activity)/OnEnd(sibling activity)/OnShutdown/Dispose called. - - var startedActivities = activityProcessor.Invocations.Where(invo => invo.Method.Name == "OnStart"); - var stoppedActivities = activityProcessor.Invocations.Where(invo => invo.Method.Name == "OnEnd"); - Assert.Equal(2, startedActivities.Count()); - Assert.Single(stoppedActivities); - - // The activity created by the framework and the sibling activity are both sent to Processor.OnStart - Assert.Contains(startedActivities, item => - { - var startedActivity = item.Arguments[0] as Activity; - return startedActivity.OperationName == HttpInListener.ActivityOperationName; - }); - - Assert.Contains(startedActivities, item => - { - var startedActivity = item.Arguments[0] as Activity; - return startedActivity.OperationName == HttpInListener.ActivityNameByHttpInListener; - }); - - // Only the sibling activity is sent to Processor.OnEnd - Assert.Contains(stoppedActivities, item => - { - var stoppedActivity = item.Arguments[0] as Activity; - return stoppedActivity.OperationName == HttpInListener.ActivityNameByHttpInListener; - }); - } - else - { - Assert.Equal(5, activityProcessor.Invocations.Count); // SetParentProvider/OnStart/OnEnd/OnShutdown/Dispose called. - - var startedActivities = activityProcessor.Invocations.Where(invo => invo.Method.Name == "OnStart"); - var stoppedActivities = activityProcessor.Invocations.Where(invo => invo.Method.Name == "OnEnd"); - - // There is no sibling activity created - Assert.Single(startedActivities); - Assert.Single(stoppedActivities); - - Assert.Contains(startedActivities, item => - { - var startedActivity = item.Arguments[0] as Activity; - return startedActivity.OperationName == HttpInListener.ActivityOperationName; - }); - - // Only the sibling activity is sent to Processor.OnEnd - Assert.Contains(stoppedActivities, item => - { - var stoppedActivity = item.Arguments[0] as Activity; - return stoppedActivity.OperationName == HttpInListener.ActivityOperationName; - }); - } + Assert.Single(exportedItems); - span = (Activity)activityProcessor.Invocations[2].Arguments[0]; + Activity span = exportedItems[0]; - Assert.Equal( - carrierFormat == "TraceContext" || carrierFormat == "CustomContextMatchParent" - ? HttpInListener.ActivityOperationName - : HttpInListener.ActivityNameByHttpInListener, - span.OperationName); + Assert.Equal(TelemetryHttpModule.AspNetActivityName, span.OperationName); Assert.NotEqual(TimeSpan.Zero, span.Duration); - Assert.Equal(expectedTraceId, span.TraceId); - Assert.Equal(expectedSpanId, span.ParentSpanId); Assert.Equal(routeTemplate ?? HttpContext.Current.Request.Path, span.DisplayName); Assert.Equal(ActivityKind.Server, span.Kind); @@ -287,25 +189,6 @@ public void AspNetRequestsAreCollectedSuccessfully( Assert.Equal(200, span.GetTagValue(SemanticConventions.AttributeHttpStatusCode)); - if (setStatusToErrorInEnrich) - { - // This validates that users can override the - // status in Enrich. - Assert.Equal(Status.Error, span.GetStatus()); - - // Instrumentation is not expected to set status description - // as the reason can be inferred from SemanticConventions.AttributeHttpStatusCode - Assert.True(string.IsNullOrEmpty(span.GetStatus().Description)); - } - else - { - Assert.Equal(Status.Unset, span.GetStatus()); - - // Instrumentation is not expected to set status description - // as the reason can be inferred from SemanticConventions.AttributeHttpStatusCode - Assert.True(string.IsNullOrEmpty(span.GetStatus().Description)); - } - var expectedUri = new Uri(expectedUrl); var actualUrl = span.GetTagValue(SemanticConventions.AttributeHttpUrl); @@ -338,13 +221,37 @@ public void AspNetRequestsAreCollectedSuccessfully( Assert.Equal(HttpContext.Current.Request.HttpMethod, span.GetTagValue(SemanticConventions.AttributeHttpMethod) as string); Assert.Equal(HttpContext.Current.Request.Path, span.GetTagValue(SpanAttributeConstants.HttpPathKey) as string); Assert.Equal(HttpContext.Current.Request.UserAgent, span.GetTagValue(SemanticConventions.AttributeHttpUserAgent) as string); + + if (recordException) + { + var status = span.GetStatus(); + Assert.Equal(Status.Error.StatusCode, status.StatusCode); + Assert.Equal("Operation is not valid due to the current state of the object.", status.Description); + } + else if (setStatusToErrorInEnrich) + { + // This validates that users can override the + // status in Enrich. + Assert.Equal(Status.Error, span.GetStatus()); + + // Instrumentation is not expected to set status description + // as the reason can be inferred from SemanticConventions.AttributeHttpStatusCode + Assert.True(string.IsNullOrEmpty(span.GetStatus().Description)); + } + else + { + Assert.Equal(Status.Unset, span.GetStatus()); + + // Instrumentation is not expected to set status description + // as the reason can be inferred from SemanticConventions.AttributeHttpStatusCode + Assert.True(string.IsNullOrEmpty(span.GetStatus().Description)); + } } [Theory] [InlineData(SamplingDecision.Drop)] [InlineData(SamplingDecision.RecordOnly)] [InlineData(SamplingDecision.RecordAndSample)] - public void ExtractContextIrrespectiveOfSamplingDecision(SamplingDecision samplingDecision) { HttpContext.Current = new HttpContext( @@ -363,11 +270,9 @@ public void ExtractContextIrrespectiveOfSamplingDecision(SamplingDecision sampli .Returns(() => { isPropagatorCalled = true; - return default(PropagationContext); + return default; }); - var activity = new Activity(HttpInListener.ActivityOperationName); - var activityProcessor = new Mock>(); Sdk.SetDefaultTextMapPropagator(propagator.Object); using (var openTelemetry = Sdk.CreateTracerProviderBuilder() @@ -375,15 +280,8 @@ public void ExtractContextIrrespectiveOfSamplingDecision(SamplingDecision sampli .AddAspNetInstrumentation() .AddProcessor(activityProcessor.Object).Build()) { - activity.Start(); - - using (var inMemoryEventListener = new InMemoryEventListener(AspNetInstrumentationEventSource.Log)) - { - this.fakeAspNetDiagnosticSource.Write("Start", null); - } - - this.fakeAspNetDiagnosticSource.Write("Stop", null); - activity.Stop(); + var activity = ActivityHelper.StartAspNetActivity(Propagators.DefaultTextMapPropagator, HttpContext.Current, TelemetryHttpModule.Options.OnRequestStartedCallback); + ActivityHelper.StopAspNetActivity(activity, HttpContext.Current, TelemetryHttpModule.Options.OnRequestStoppedCallback); } Assert.True(isPropagatorCalled); @@ -408,11 +306,9 @@ public void ExtractContextIrrespectiveOfTheFilterApplied() .Returns(() => { isPropagatorCalled = true; - return default(PropagationContext); + return default; }); - var activity = new Activity(HttpInListener.ActivityOperationName); - bool isFilterCalled = false; var activityProcessor = new Mock>(); Sdk.SetDefaultTextMapPropagator(propagator.Object); @@ -427,15 +323,8 @@ public void ExtractContextIrrespectiveOfTheFilterApplied() }) .AddProcessor(activityProcessor.Object).Build()) { - activity.Start(); - - using (var inMemoryEventListener = new InMemoryEventListener(AspNetInstrumentationEventSource.Log)) - { - this.fakeAspNetDiagnosticSource.Write("Start", null); - } - - this.fakeAspNetDiagnosticSource.Write("Stop", null); - activity.Stop(); + var activity = ActivityHelper.StartAspNetActivity(Propagators.DefaultTextMapPropagator, HttpContext.Current, TelemetryHttpModule.Options.OnRequestStartedCallback); + ActivityHelper.StopAspNetActivity(activity, HttpContext.Current, TelemetryHttpModule.Options.OnRequestStoppedCallback); } Assert.True(isFilterCalled); @@ -444,9 +333,7 @@ public void ExtractContextIrrespectiveOfTheFilterApplied() private static Action GetEnrichmentAction(Status statusToBeSet) { - Action enrichAction; - - enrichAction = (activity, method, obj) => + void EnrichAction(Activity activity, string method, object obj) { Assert.True(activity.IsAllDataRequested); switch (method) @@ -467,34 +354,14 @@ private static Action GetEnrichmentAction(Status statu default: break; } - }; - - return enrichAction; - } - - private class FakeAspNetDiagnosticSource : IDisposable - { - private readonly DiagnosticListener listener; - - public FakeAspNetDiagnosticSource() - { - this.listener = new DiagnosticListener(AspNetInstrumentation.AspNetDiagnosticListenerName); - } - - public void Write(string name, object value) - { - this.listener.Write(name, value); } - public void Dispose() - { - this.listener.Dispose(); - } + return EnrichAction; } private class TestSampler : Sampler { - private SamplingDecision samplingDecision; + private readonly SamplingDecision samplingDecision; public TestSampler(SamplingDecision samplingDecision) { diff --git a/test/OpenTelemetry.Instrumentation.AspNet.Tests/OpenTelemetry.Instrumentation.AspNet.Tests.csproj b/test/OpenTelemetry.Instrumentation.AspNet.Tests/OpenTelemetry.Instrumentation.AspNet.Tests.csproj index 011b8c50816..f2b8fd8edc8 100644 --- a/test/OpenTelemetry.Instrumentation.AspNet.Tests/OpenTelemetry.Instrumentation.AspNet.Tests.csproj +++ b/test/OpenTelemetry.Instrumentation.AspNet.Tests/OpenTelemetry.Instrumentation.AspNet.Tests.csproj @@ -16,13 +16,18 @@ + - - - + + + + + + + From fc0492c9725282c044fb1015a7cb0138670d5f94 Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Sat, 21 Aug 2021 20:12:01 -0700 Subject: [PATCH 6/9] New design for TelemetryHttpModule using ActivitySource + OpenTelemetry.API part 6 (#2264) * CHANGELOG & README updates. * Apply suggestions from code review Co-authored-by: Reiley Yang Co-authored-by: Reiley Yang --- .../CHANGELOG.md | 14 +- .../README.md | 151 +++++++++++++----- .../CHANGELOG.md | 14 ++ .../README.md | 16 +- .../README.md | 7 +- 5 files changed, 151 insertions(+), 51 deletions(-) diff --git a/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/CHANGELOG.md b/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/CHANGELOG.md index 7f0685a7419..5edd83ddf29 100644 --- a/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/CHANGELOG.md +++ b/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/CHANGELOG.md @@ -2,6 +2,11 @@ ## Unreleased +* Adopted the donation + [Microsoft.AspNet.TelemetryCorrelation](https://github.com/aspnet/Microsoft.AspNet.TelemetryCorrelation) + from [.NET Foundation](https://dotnetfoundation.org/) + ([#2223](https://github.com/open-telemetry/opentelemetry-dotnet/pull/2223)) + * Renamed the module, refactored existing code ([#2224](https://github.com/open-telemetry/opentelemetry-dotnet/pull/2224) [#2225](https://github.com/open-telemetry/opentelemetry-dotnet/pull/2225) @@ -12,7 +17,8 @@ [#2238](https://github.com/open-telemetry/opentelemetry-dotnet/pull/2238) [#2240](https://github.com/open-telemetry/opentelemetry-dotnet/pull/2240)) -* Adopted the donation - [Microsoft.AspNet.TelemetryCorrelation](https://github.com/aspnet/Microsoft.AspNet.TelemetryCorrelation) - from [.NET Foundation](https://dotnetfoundation.org/) - ([#2223](https://github.com/open-telemetry/opentelemetry-dotnet/pull/2223)) +* Updated to use + [ActivitySource](https://docs.microsoft.com/en-us/dotnet/api/system.diagnostics.activitysource) + & OpenTelemetry.API + ([#2249](https://github.com/open-telemetry/opentelemetry-dotnet/pull/2249) & + follow-ups (linked to #2249)) diff --git a/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/README.md b/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/README.md index f9e7a90cb9c..ba8b31ad8f1 100644 --- a/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/README.md +++ b/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/README.md @@ -1,59 +1,134 @@ -# Telemetry correlation http module +# ASP.NET Telemetry HttpModule for OpenTelemetry [![NuGet](https://img.shields.io/nuget/v/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule.svg)](https://www.nuget.org/packages/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/) -Telemetry correlation http module enables cross tier telemetry tracking. +The ASP.NET Telemetry HttpModule enables distributed tracing of incoming ASP.NET +requests using the OpenTelemetry API. ## Usage -1. Install NuGet for your app. -2. Enable diagnostics source listener using code below. Note, some telemetry - vendors like Azure Application Insights will enable it automatically. - - ``` csharp - public class NoopDiagnosticsListener : IObserver> +### Step 1: Install NuGet package + +If you are using the traditional `packages.config` reference style, a `web.config` +transform should run automatically and configure the `TelemetryHttpModule` for +you. If you are using the more modern PackageReference style, this may be needed +to be done manually. For more information, see: [Migrate from packages.config to +PackageReference](https://docs.microsoft.com/en-us/nuget/consume-packages/migrate-packages-config-to-package-reference). + +To configure your `web.config` manually, add this: + +```xml + + + + + +``` + +### Step 2: Register a listener + +`TelemetryHttpModule` registers an +[ActivitySource](https://docs.microsoft.com/dotnet/api/system.diagnostics.activitysource) +with the name `OpenTelemetry.Instrumentation.AspNet.Telemetry`. By default, .NET +`ActivitySource` will not generate any `Activity` objects unless there is a +registered listener. + +To register a listener automatically using OpenTelemetry, please use the +[OpenTelemetry.Instrumentation.AspNet](https://www.nuget.org/packages/OpenTelemetry.Instrumentation.AspNet/) +NuGet package. + +To register a listener manually, use code such as the following: + +```csharp +using System.Diagnostics; +using System.Web; +using System.Web.Http; +using System.Web.Mvc; +using System.Web.Routing; +using OpenTelemetry.Instrumentation.AspNet; + +namespace Examples.AspNet +{ + public class WebApiApplication : HttpApplication { - public void OnCompleted() { } + private ActivityListener aspNetActivityListener; - public void OnError(Exception error) { } + protected void Application_Start() + { + this.aspNetActivityListener = new ActivityListener + { + ShouldListenTo = (activitySource) => + { + // Only listen to TelemetryHttpModule's ActivitySource. + return activitySource.Name == TelemetryHttpModule.AspNetSourceName; + }, + Sample = (ref ActivityCreationOptions options) => + { + // Sample everything created by TelemetryHttpModule's ActivitySource. + return ActivitySamplingResult.AllDataAndRecorded; + }, + }; + + ActivitySource.AddActivityListener(this.aspNetActivityListener); + + GlobalConfiguration.Configure(WebApiConfig.Register); + + AreaRegistration.RegisterAllAreas(); + RouteConfig.RegisterRoutes(RouteTable.Routes); + } - public void OnNext(KeyValuePair evnt) + protected void Application_End() { + this.aspNetActivityListener?.Dispose(); } } +} +``` - public class NoopSubscriber : IObserver - { - public void OnCompleted() { } +## Options - public void OnError(Exception error) { } +`TelemetryHttpModule` provides a static options property +(`TelemetryHttpModule.Options`) which can be used to configure the +`TelemetryHttpModule` and listen to events it fires. - public void OnNext(DiagnosticListener listener) - { - if (listener.Name == "OpenTelemetry.Instrumentation.AspNet.Telemetry") - { - listener.Subscribe(new NoopDiagnosticsListener()); - } - } - } - ``` +### TextMapPropagator + +`TextMapPropagator` controls how trace context will be extracted from incoming +Http request messages. By default, [W3C Trace +Context](https://www.w3.org/TR/trace-context/) is enabled. + +The OpenTelemetry API ships with a handful of [standard +implementations](https://github.com/open-telemetry/opentelemetry-dotnet/tree/main/src/OpenTelemetry.Api/Context/Propagation) +which may be used, or you can write your own by deriving from the +`TextMapPropagator` class. -3. Double check that http module was registered in `web.config` for your app. +To add support for +[Baggage](https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/baggage/api.md) +propagation in addition to W3C Trace Context, use: -Once enabled - this http module will: +```csharp +TelemetryHttpModuleOptions.TextMapPropagator = new CompositeTextMapPropagator(new TextMapPropagator[] +{ + new TraceContextPropagator(), + new BaggagePropagator(), +}); +``` -- Reads correlation http headers -- Start/Stops Activity for the http request -- Ensure the Activity ambient state is transferred thru the IIS callbacks +Note: When using the `OpenTelemetry.Instrumentation.AspNet` +`TelemetryHttpModuleOptions.TextMapPropagator` is automatically initialized to +the SDK default propagator (`Propagators.DefaultTextMapPropagator`) which by +default supports W3C Trace Context & Baggage. -See http protocol [specifications][http-protocol-specification] for details. +### Events -This http module is used by Application Insights. See -[documentation][usage-in-ai-docs] and [code][usage-in-ai-code]. +`OnRequestStartedCallback`, `OnRequestStoppedCallback`, & `OnExceptionCallback` +are provided on `TelemetryHttpModuleOptions` and will be fired by the +`TelemetryHttpModule` as requests are processed. -[http-protocol-specification]: -https://github.com/dotnet/corefx/blob/master/src/System.Diagnostics.DiagnosticSource/src/HttpCorrelationProtocol.md -[usage-in-ai-docs]: -https://docs.microsoft.com/azure/application-insights/application-insights-correlation -[usage-in-ai-code]: -https://github.com/Microsoft/ApplicationInsights-dotnet-server +A typical use case for these events is to add information (tags, events, and/or +links) to the created `Activity` based on the request, response, and/or +exception event being fired. diff --git a/src/OpenTelemetry.Instrumentation.AspNet/CHANGELOG.md b/src/OpenTelemetry.Instrumentation.AspNet/CHANGELOG.md index 26c381d70a1..f4c3468b0bc 100644 --- a/src/OpenTelemetry.Instrumentation.AspNet/CHANGELOG.md +++ b/src/OpenTelemetry.Instrumentation.AspNet/CHANGELOG.md @@ -5,6 +5,20 @@ * Removes .NET Framework 4.5.2, .NET 4.6 support. The minimum .NET Framework version supported is .NET 4.6.1. ([#2138](https://github.com/open-telemetry/opentelemetry-dotnet/issues/2138)) +* ASP.NET instrumentation now uses + [OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule](https://www.nuget.org/packages/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/) + instead of + [Microsoft.AspNet.TelemetryCorrelation](https://www.nuget.org/packages/Microsoft.AspNet.TelemetryCorrelation/) + to listen for incoming http requests to the process. Please see the (Step 2: + Modify + Web.config)[https://github.com/open-telemetry/opentelemetry-dotnet/tree/main/src/OpenTelemetry.Instrumentation.AspNet#step-2-modify-webconfig] + README section for details on the new HttpModule definition required. + ([#2222](https://github.com/open-telemetry/opentelemetry-dotnet/issues/2222)) + +* Added `RecordException` option. Specify `true` to have unhandled exception + details automatically captured on spans. + ([#2256](https://github.com/open-telemetry/opentelemetry-dotnet/pull/2256)) + ## 1.0.0-rc7 Released 2021-Jul-12 diff --git a/src/OpenTelemetry.Instrumentation.AspNet/README.md b/src/OpenTelemetry.Instrumentation.AspNet/README.md index 9a465608fbd..d9637ce611c 100644 --- a/src/OpenTelemetry.Instrumentation.AspNet/README.md +++ b/src/OpenTelemetry.Instrumentation.AspNet/README.md @@ -31,11 +31,11 @@ following shows changes required to your `Web.config` when using IIS web server. ```xml - + ``` @@ -143,6 +143,12 @@ general extensibility point to add additional properties to any activity. The `Enrich` option is specific to this instrumentation, and is provided to get access to `HttpRequest` and `HttpResponse`. +### RecordException + +This instrumentation automatically sets Activity Status to Error if an unhandled +exception is thrown. Additionally, `RecordException` feature may be turned on, +to store the exception to the Activity itself as ActivityEvent. + ## References * [ASP.NET](https://dotnet.microsoft.com/apps/aspnet) diff --git a/src/OpenTelemetry.Instrumentation.AspNetCore/README.md b/src/OpenTelemetry.Instrumentation.AspNetCore/README.md index d439ec06d16..bbbd43a0645 100644 --- a/src/OpenTelemetry.Instrumentation.AspNetCore/README.md +++ b/src/OpenTelemetry.Instrumentation.AspNetCore/README.md @@ -155,10 +155,9 @@ get access to `HttpRequest` and `HttpResponse`. ### RecordException -This instrumentation automatically sets Activity Status to Error if the -Http StatusCode is >= 400. -Additionally, `RecordException` feature may be turned on, to store the exception -to the Activity itself as ActivityEvent. +This instrumentation automatically sets Activity Status to Error if an unhandled +exception is thrown. Additionally, `RecordException` feature may be turned on, +to store the exception to the Activity itself as ActivityEvent. ## References From 00ea470868f3a321e9dfb4bb6fdd1210656962d1 Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Sat, 21 Aug 2021 21:35:35 -0700 Subject: [PATCH 7/9] Lint + sanity checks. --- examples/AspNet/Examples.AspNet.csproj | 2 +- .../ActivityHelper.cs | 2 +- .../README.md | 9 +++++---- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/examples/AspNet/Examples.AspNet.csproj b/examples/AspNet/Examples.AspNet.csproj index fef00f55b5e..50ed1730846 100644 --- a/examples/AspNet/Examples.AspNet.csproj +++ b/examples/AspNet/Examples.AspNet.csproj @@ -150,4 +150,4 @@ - \ No newline at end of file + diff --git a/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/ActivityHelper.cs b/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/ActivityHelper.cs index 10faac6a94f..e4b478babf7 100644 --- a/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/ActivityHelper.cs +++ b/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/ActivityHelper.cs @@ -87,7 +87,7 @@ public static Activity StartAspNetActivity(TextMapPropagator textMapPropagator, { // todo: RestoreActivityIfNeeded below compensates for // AsyncLocal Activity.Current being lost. Baggage - // potentially will suffer from the same issue, but we can’t + // potentially will suffer from the same issue, but we can't // simply add it to context.Items because any change results // in a new instance. Probably need to save it at the end of // each OnExecuteRequestStep. diff --git a/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/README.md b/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/README.md index ba8b31ad8f1..990f525c7d2 100644 --- a/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/README.md +++ b/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/README.md @@ -9,10 +9,11 @@ requests using the OpenTelemetry API. ### Step 1: Install NuGet package -If you are using the traditional `packages.config` reference style, a `web.config` -transform should run automatically and configure the `TelemetryHttpModule` for -you. If you are using the more modern PackageReference style, this may be needed -to be done manually. For more information, see: [Migrate from packages.config to +If you are using the traditional `packages.config` reference style, a +`web.config` transform should run automatically and configure the +`TelemetryHttpModule` for you. If you are using the more modern PackageReference +style, this may be needed to be done manually. For more information, see: +[Migrate from packages.config to PackageReference](https://docs.microsoft.com/en-us/nuget/consume-packages/migrate-packages-config-to-package-reference). To configure your `web.config` manually, add this: From 81673621b8da8d714009c956500a3bbb89da3e67 Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Sat, 21 Aug 2021 21:53:31 -0700 Subject: [PATCH 8/9] Lint attempt 2. --- .../README.md | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/README.md b/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/README.md index 990f525c7d2..731cd8c1fa6 100644 --- a/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/README.md +++ b/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/README.md @@ -112,11 +112,12 @@ To add support for propagation in addition to W3C Trace Context, use: ```csharp -TelemetryHttpModuleOptions.TextMapPropagator = new CompositeTextMapPropagator(new TextMapPropagator[] -{ - new TraceContextPropagator(), - new BaggagePropagator(), -}); +TelemetryHttpModuleOptions.TextMapPropagator = new CompositeTextMapPropagator( + new TextMapPropagator[] + { + new TraceContextPropagator(), + new BaggagePropagator(), + }); ``` Note: When using the `OpenTelemetry.Instrumentation.AspNet` From f697c52dc1535fdd267b283ddbc7e32eae4737fa Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Tue, 24 Aug 2021 10:28:53 -0700 Subject: [PATCH 9/9] Restored CHANGELOG changes lost in merge. --- .../CHANGELOG.md | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/OpenTelemetry.Instrumentation.AspNet/CHANGELOG.md b/src/OpenTelemetry.Instrumentation.AspNet/CHANGELOG.md index 517e097ca0a..d8f03a0a7e4 100644 --- a/src/OpenTelemetry.Instrumentation.AspNet/CHANGELOG.md +++ b/src/OpenTelemetry.Instrumentation.AspNet/CHANGELOG.md @@ -8,6 +8,20 @@ * Replaced `http.path` tag on activity with `http.target`. ([#2266](https://github.com/open-telemetry/opentelemetry-dotnet/pull/2266)) +* ASP.NET instrumentation now uses + [OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule](https://www.nuget.org/packages/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/) + instead of + [Microsoft.AspNet.TelemetryCorrelation](https://www.nuget.org/packages/Microsoft.AspNet.TelemetryCorrelation/) + to listen for incoming http requests to the process. Please see the (Step 2: + Modify + Web.config)[https://github.com/open-telemetry/opentelemetry-dotnet/tree/main/src/OpenTelemetry.Instrumentation.AspNet#step-2-modify-webconfig] + README section for details on the new HttpModule definition required. + ([#2222](https://github.com/open-telemetry/opentelemetry-dotnet/issues/2222)) + +* Added `RecordException` option. Specify `true` to have unhandled exception + details automatically captured on spans. + ([#2256](https://github.com/open-telemetry/opentelemetry-dotnet/pull/2256)) + ## 1.0.0-rc7 Released 2021-Jul-12