diff --git a/src/OpenTelemetry.Instrumentation.Http/CHANGELOG.md b/src/OpenTelemetry.Instrumentation.Http/CHANGELOG.md index a94fa9789ef..26a3971a0f9 100644 --- a/src/OpenTelemetry.Instrumentation.Http/CHANGELOG.md +++ b/src/OpenTelemetry.Instrumentation.Http/CHANGELOG.md @@ -2,6 +2,38 @@ ## Unreleased +* Introduced a new metric, `http.client.request.duration` measured in seconds. + The OTel SDK + [applies custom histogram buckets](https://github.com/open-telemetry/opentelemetry-dotnet/pull/4820) + for this metric to comply with the + [Semantic Convention for Http Metrics](https://github.com/open-telemetry/semantic-conventions/blob/2bad9afad58fbd6b33cc683d1ad1f006e35e4a5d/docs/http/http-metrics.md). + This new metric is only available for users who opt-in to the new + semantic convention by configuring the `OTEL_SEMCONV_STABILITY_OPT_IN` + environment variable to either `http` (to emit only the new metric) or + `http/dup` (to emit both the new and old metrics). + ([#4870](https://github.com/open-telemetry/opentelemetry-dotnet/pull/4870)) + + * New metric: `http.client.request.duration` + * Unit: `s` (seconds) + * Histogram Buckets: `0, 0.005, 0.01, 0.025, 0.05, 0.075, 0.1, 0.25, 0.5, + 0.75, 1, 2.5, 5, 7.5, 10` + * Old metric: `http.client.duration` + * Unit: `ms` (milliseconds) + * Histogram Buckets: `0, 5, 10, 25, 50, 75, 100, 250, 500, 750, 1000, 2500, + 5000, 7500, 10000` + + Note: The older `http.client.duration` metric and + `OTEL_SEMCONV_STABILITY_OPT_IN` environment variable will eventually be + removed after the HTTP semantic conventions are marked stable. At which time + this instrumentation can publish a stable release. Refer to the specification + for more information regarding the new HTTP semantic conventions: + * [http-spans](https://github.com/open-telemetry/semantic-conventions/blob/2bad9afad58fbd6b33cc683d1ad1f006e35e4a5d/docs/http/http-spans.md) + * [http-metrics](https://github.com/open-telemetry/semantic-conventions/blob/2bad9afad58fbd6b33cc683d1ad1f006e35e4a5d/docs/http/http-metrics.md) + +* Added support for publishing `http.client.duration` & + `http.client.request.duration` metrics on .NET Framework + ([#4870](https://github.com/open-telemetry/opentelemetry-dotnet/pull/4870)) + ## 1.5.1-beta.1 Released 2023-Jul-20 diff --git a/src/OpenTelemetry.Instrumentation.Http/HttpClientMetrics.cs b/src/OpenTelemetry.Instrumentation.Http/HttpClientMetrics.cs index 2ff85729892..2f4db9ec9b2 100644 --- a/src/OpenTelemetry.Instrumentation.Http/HttpClientMetrics.cs +++ b/src/OpenTelemetry.Instrumentation.Http/HttpClientMetrics.cs @@ -14,8 +14,6 @@ // limitations under the License. // -using System.Diagnostics.Metrics; -using System.Reflection; using OpenTelemetry.Instrumentation.Http.Implementation; namespace OpenTelemetry.Instrumentation.Http; @@ -25,10 +23,6 @@ namespace OpenTelemetry.Instrumentation.Http; /// internal sealed class HttpClientMetrics : IDisposable { - internal static readonly AssemblyName AssemblyName = typeof(HttpClientMetrics).Assembly.GetName(); - internal static readonly string InstrumentationName = AssemblyName.Name; - internal static readonly string InstrumentationVersion = AssemblyName.Version.ToString(); - private static readonly HashSet ExcludedDiagnosticSourceEvents = new() { "System.Net.Http.Request", @@ -36,7 +30,6 @@ internal sealed class HttpClientMetrics : IDisposable }; private readonly DiagnosticSourceSubscriber diagnosticSourceSubscriber; - private readonly Meter meter; private readonly Func isEnabled = (activityName, obj1, obj2) => !ExcludedDiagnosticSourceEvents.Contains(activityName); @@ -47,8 +40,10 @@ internal sealed class HttpClientMetrics : IDisposable /// HttpClient metric instrumentation options. public HttpClientMetrics(HttpClientMetricInstrumentationOptions options) { - this.meter = new Meter(InstrumentationName, InstrumentationVersion); - this.diagnosticSourceSubscriber = new DiagnosticSourceSubscriber(new HttpHandlerMetricsDiagnosticListener("HttpHandlerDiagnosticListener", this.meter, options), this.isEnabled, HttpInstrumentationEventSource.Log.UnknownErrorProcessingEvent); + this.diagnosticSourceSubscriber = new DiagnosticSourceSubscriber( + new HttpHandlerMetricsDiagnosticListener("HttpHandlerDiagnosticListener", options), + this.isEnabled, + HttpInstrumentationEventSource.Log.UnknownErrorProcessingEvent); this.diagnosticSourceSubscriber.Subscribe(); } @@ -56,6 +51,5 @@ public HttpClientMetrics(HttpClientMetricInstrumentationOptions options) public void Dispose() { this.diagnosticSourceSubscriber?.Dispose(); - this.meter?.Dispose(); } } diff --git a/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpHandlerMetricsDiagnosticListener.cs b/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpHandlerMetricsDiagnosticListener.cs index 3aac7b32160..f43e3c586de 100644 --- a/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpHandlerMetricsDiagnosticListener.cs +++ b/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpHandlerMetricsDiagnosticListener.cs @@ -22,6 +22,7 @@ #if NETFRAMEWORK using System.Net.Http; #endif +using System.Reflection; using OpenTelemetry.Trace; using static OpenTelemetry.Internal.HttpSemanticConventionHelper; @@ -31,21 +32,25 @@ internal sealed class HttpHandlerMetricsDiagnosticListener : ListenerHandler { internal const string OnStopEvent = "System.Net.Http.HttpRequestOut.Stop"; + internal static readonly AssemblyName AssemblyName = typeof(HttpClientMetrics).Assembly.GetName(); + internal static readonly string MeterName = AssemblyName.Name; + internal static readonly string MeterVersion = AssemblyName.Version.ToString(); + internal static readonly Meter Meter = new(MeterName, MeterVersion); + private static readonly Histogram HttpClientDuration = Meter.CreateHistogram("http.client.duration", "ms", "Measures the duration of outbound HTTP requests."); + private static readonly Histogram HttpClientRequestDuration = Meter.CreateHistogram("http.client.request.duration", "s", "Measures the duration of outbound HTTP requests."); + private static readonly PropertyFetcher StopRequestFetcher = new("Request"); private static readonly PropertyFetcher StopResponseFetcher = new("Response"); - private readonly Histogram httpClientDuration; private readonly HttpClientMetricInstrumentationOptions options; private readonly bool emitOldAttributes; private readonly bool emitNewAttributes; - public HttpHandlerMetricsDiagnosticListener(string name, Meter meter, HttpClientMetricInstrumentationOptions options) + public HttpHandlerMetricsDiagnosticListener(string name, HttpClientMetricInstrumentationOptions options) : base(name) { - this.httpClientDuration = meter.CreateHistogram("http.client.duration", "ms", "Measures the duration of outbound HTTP requests."); this.options = options; this.emitOldAttributes = this.options.HttpSemanticConvention.HasFlag(HttpSemanticConvention.Old); - this.emitNewAttributes = this.options.HttpSemanticConvention.HasFlag(HttpSemanticConvention.New); } @@ -61,11 +66,11 @@ public override void OnEventWritten(string name, object payload) var activity = Activity.Current; if (TryFetchRequest(payload, out HttpRequestMessage request)) { - TagList tags = default; - // see the spec https://github.com/open-telemetry/opentelemetry-specification/blob/v1.20.0/specification/trace/semantic_conventions/http.md if (this.emitOldAttributes) { + TagList tags = default; + tags.Add(new KeyValuePair(SemanticConventions.AttributeHttpMethod, HttpTagHelper.GetNameForHttpMethod(request.Method))); tags.Add(new KeyValuePair(SemanticConventions.AttributeHttpScheme, request.RequestUri.Scheme)); tags.Add(new KeyValuePair(SemanticConventions.AttributeHttpFlavor, HttpTagHelper.GetFlavorTagValueFromProtocolVersion(request.Version))); @@ -80,11 +85,18 @@ public override void OnEventWritten(string name, object payload) { tags.Add(new KeyValuePair(SemanticConventions.AttributeHttpStatusCode, TelemetryHelper.GetBoxedStatusCode(response.StatusCode))); } + + // We are relying here on HttpClient library to set duration before writing the stop event. + // https://github.com/dotnet/runtime/blob/90603686d314147017c8bbe1fa8965776ce607d0/src/libraries/System.Net.Http/src/System/Net/Http/DiagnosticsHandler.cs#L178 + // TODO: Follow up with .NET team if we can continue to rely on this behavior. + HttpClientDuration.Record(activity.Duration.TotalMilliseconds, tags); } // see the spec https://github.com/open-telemetry/semantic-conventions/blob/v1.21.0/docs/http/http-spans.md if (this.emitNewAttributes) { + TagList tags = default; + tags.Add(new KeyValuePair(SemanticConventions.AttributeHttpRequestMethod, HttpTagHelper.GetNameForHttpMethod(request.Method))); tags.Add(new KeyValuePair(SemanticConventions.AttributeNetworkProtocolVersion, HttpTagHelper.GetFlavorTagValueFromProtocolVersion(request.Version))); tags.Add(new KeyValuePair(SemanticConventions.AttributeServerAddress, request.RequestUri.Host)); @@ -98,12 +110,12 @@ public override void OnEventWritten(string name, object payload) { tags.Add(new KeyValuePair(SemanticConventions.AttributeHttpResponseStatusCode, TelemetryHelper.GetBoxedStatusCode(response.StatusCode))); } - } - // We are relying here on HttpClient library to set duration before writing the stop event. - // https://github.com/dotnet/runtime/blob/90603686d314147017c8bbe1fa8965776ce607d0/src/libraries/System.Net.Http/src/System/Net/Http/DiagnosticsHandler.cs#L178 - // TODO: Follow up with .NET team if we can continue to rely on this behavior. - this.httpClientDuration.Record(activity.Duration.TotalMilliseconds, tags); + // We are relying here on HttpClient library to set duration before writing the stop event. + // https://github.com/dotnet/runtime/blob/90603686d314147017c8bbe1fa8965776ce607d0/src/libraries/System.Net.Http/src/System/Net/Http/DiagnosticsHandler.cs#L178 + // TODO: Follow up with .NET team if we can continue to rely on this behavior. + HttpClientRequestDuration.Record(activity.Duration.TotalSeconds, tags); + } } } diff --git a/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpWebRequestActivitySource.netfx.cs b/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpWebRequestActivitySource.netfx.cs index 670ce8f3613..d341ebbcf14 100644 --- a/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpWebRequestActivitySource.netfx.cs +++ b/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpWebRequestActivitySource.netfx.cs @@ -17,6 +17,7 @@ #if NETFRAMEWORK using System.Collections; using System.Diagnostics; +using System.Diagnostics.Metrics; using System.Net; using System.Reflection; using System.Reflection.Emit; @@ -39,17 +40,24 @@ internal static class HttpWebRequestActivitySource internal static readonly AssemblyName AssemblyName = typeof(HttpWebRequestActivitySource).Assembly.GetName(); internal static readonly string ActivitySourceName = AssemblyName.Name + ".HttpWebRequest"; internal static readonly string ActivityName = ActivitySourceName + ".HttpRequestOut"; + internal static readonly string MeterName = AssemblyName.Name; internal static readonly Func> HttpWebRequestHeaderValuesGetter = (request, name) => request.Headers.GetValues(name); internal static readonly Action HttpWebRequestHeaderValuesSetter = (request, name, value) => request.Headers.Add(name, value); - private static readonly Version Version = AssemblyName.Version; - private static readonly ActivitySource WebRequestActivitySource = new ActivitySource(ActivitySourceName, Version.ToString()); + private static readonly string Version = AssemblyName.Version.ToString(); + private static readonly ActivitySource WebRequestActivitySource = new(ActivitySourceName, Version); + private static readonly Meter WebRequestMeter = new(MeterName, Version); + private static readonly Histogram HttpClientDuration = WebRequestMeter.CreateHistogram("http.client.duration", "ms", "Measures the duration of outbound HTTP requests."); + private static readonly Histogram HttpClientRequestDuration = WebRequestMeter.CreateHistogram("http.client.request.duration", "s", "Measures the duration of outbound HTTP requests."); - private static HttpClientInstrumentationOptions options; + private static HttpClientInstrumentationOptions tracingOptions; + private static HttpClientMetricInstrumentationOptions metricsOptions; - private static bool emitOldAttributes; - private static bool emitNewAttributes; + private static bool tracingEmitOldAttributes; + private static bool tracingEmitNewAttributes; + private static bool metricsEmitOldAttributes; + private static bool metricsEmitNewAttributes; // Fields for reflection private static FieldInfo connectionGroupListField; @@ -86,7 +94,8 @@ static HttpWebRequestActivitySource() PrepareReflectionObjects(); PerformInjection(); - Options = new HttpClientInstrumentationOptions(); + TracingOptions = new HttpClientInstrumentationOptions(); + MetricsOptions = new HttpClientMetricInstrumentationOptions(); } catch (Exception ex) { @@ -95,15 +104,27 @@ static HttpWebRequestActivitySource() } } - internal static HttpClientInstrumentationOptions Options + internal static HttpClientInstrumentationOptions TracingOptions { - get => options; + get => tracingOptions; set { - options = value; + tracingOptions = value; - emitOldAttributes = value.HttpSemanticConvention.HasFlag(HttpSemanticConvention.Old); - emitNewAttributes = value.HttpSemanticConvention.HasFlag(HttpSemanticConvention.New); + tracingEmitOldAttributes = value.HttpSemanticConvention.HasFlag(HttpSemanticConvention.Old); + tracingEmitNewAttributes = value.HttpSemanticConvention.HasFlag(HttpSemanticConvention.New); + } + } + + internal static HttpClientMetricInstrumentationOptions MetricsOptions + { + get => metricsOptions; + set + { + metricsOptions = value; + + metricsEmitOldAttributes = value.HttpSemanticConvention.HasFlag(HttpSemanticConvention.Old); + metricsEmitNewAttributes = value.HttpSemanticConvention.HasFlag(HttpSemanticConvention.New); } } @@ -115,7 +136,7 @@ private static void AddRequestTagsAndInstrumentRequest(HttpWebRequest request, A if (activity.IsAllDataRequested) { // see the spec https://github.com/open-telemetry/opentelemetry-specification/blob/v1.20.0/specification/trace/semantic_conventions/http.md - if (emitOldAttributes) + if (tracingEmitOldAttributes) { activity.SetTag(SemanticConventions.AttributeHttpMethod, request.Method); activity.SetTag(SemanticConventions.AttributeNetPeerName, request.RequestUri.Host); @@ -130,7 +151,7 @@ private static void AddRequestTagsAndInstrumentRequest(HttpWebRequest request, A } // see the spec https://github.com/open-telemetry/semantic-conventions/blob/v1.21.0/docs/http/http-spans.md - if (emitNewAttributes) + if (tracingEmitNewAttributes) { activity.SetTag(SemanticConventions.AttributeHttpRequestMethod, request.Method); activity.SetTag(SemanticConventions.AttributeServerAddress, request.RequestUri.Host); @@ -145,7 +166,7 @@ private static void AddRequestTagsAndInstrumentRequest(HttpWebRequest request, A try { - Options.EnrichWithHttpWebRequest?.Invoke(activity, request); + TracingOptions.EnrichWithHttpWebRequest?.Invoke(activity, request); } catch (Exception ex) { @@ -157,14 +178,16 @@ private static void AddRequestTagsAndInstrumentRequest(HttpWebRequest request, A [MethodImpl(MethodImplOptions.AggressiveInlining)] private static void AddResponseTags(HttpWebResponse response, Activity activity) { + Debug.Assert(activity != null, "Activity must not be null"); + if (activity.IsAllDataRequested) { - if (emitOldAttributes) + if (tracingEmitOldAttributes) { activity.SetTag(SemanticConventions.AttributeHttpStatusCode, TelemetryHelper.GetBoxedStatusCode(response.StatusCode)); } - if (emitNewAttributes) + if (tracingEmitNewAttributes) { activity.SetTag(SemanticConventions.AttributeHttpResponseStatusCode, TelemetryHelper.GetBoxedStatusCode(response.StatusCode)); } @@ -173,7 +196,7 @@ private static void AddResponseTags(HttpWebResponse response, Activity activity) try { - Options.EnrichWithHttpWebResponse?.Invoke(activity, response); + TracingOptions.EnrichWithHttpWebResponse?.Invoke(activity, response); } catch (Exception ex) { @@ -183,8 +206,12 @@ private static void AddResponseTags(HttpWebResponse response, Activity activity) } [MethodImpl(MethodImplOptions.AggressiveInlining)] - private static void AddExceptionTags(Exception exception, Activity activity) + private static void AddExceptionTags(Exception exception, Activity activity, out HttpStatusCode? statusCode) { + Debug.Assert(activity != null, "Activity must not be null"); + + statusCode = null; + if (!activity.IsAllDataRequested) { return; @@ -197,17 +224,19 @@ private static void AddExceptionTags(Exception exception, Activity activity) { if (wexc.Response is HttpWebResponse response) { - if (emitOldAttributes) + statusCode = response.StatusCode; + + if (tracingEmitOldAttributes) { - activity.SetTag(SemanticConventions.AttributeHttpStatusCode, (int)response.StatusCode); + activity.SetTag(SemanticConventions.AttributeHttpStatusCode, (int)statusCode); } - if (emitNewAttributes) + if (tracingEmitNewAttributes) { - activity.SetTag(SemanticConventions.AttributeHttpResponseStatusCode, (int)response.StatusCode); + activity.SetTag(SemanticConventions.AttributeHttpResponseStatusCode, (int)statusCode); } - status = SpanHelper.ResolveSpanStatusForHttpStatusCode(activity.Kind, (int)response.StatusCode); + status = SpanHelper.ResolveSpanStatusForHttpStatusCode(activity.Kind, (int)statusCode); } else { @@ -240,14 +269,14 @@ private static void AddExceptionTags(Exception exception, Activity activity) } activity.SetStatus(status, exceptionMessage); - if (Options.RecordException) + if (TracingOptions.RecordException) { activity.RecordException(exception); } try { - Options.EnrichWithException?.Invoke(activity, exception); + TracingOptions.EnrichWithException?.Invoke(activity, exception); } catch (Exception ex) { @@ -265,10 +294,14 @@ private static bool IsRequestInstrumented(HttpWebRequest request) private static void ProcessRequest(HttpWebRequest request) { - if (!WebRequestActivitySource.HasListeners() || !Options.EventFilterHttpWebRequest(request)) + // There are subscribers to the ActivitySource and no user-provided filter is + // filtering this request. + var enableTracing = WebRequestActivitySource.HasListeners() + && TracingOptions.EventFilterHttpWebRequest(request); + + if (!enableTracing && !HttpClientDuration.Enabled && !HttpClientRequestDuration.Enabled) { - // No subscribers to the ActivitySource or User provider Filter is - // filtering this request. + // Tracing and metrics are not enabled, so we can skip generating signals // Propagation must still be done in such cases, to allow // downstream services to continue from parent context, if any. // Eg: Parent could be the Asp.Net activity. @@ -283,25 +316,23 @@ private static void ProcessRequest(HttpWebRequest request) return; } - var activity = WebRequestActivitySource.StartActivity(ActivityName, ActivityKind.Client); + Activity activity = enableTracing + ? WebRequestActivitySource.StartActivity(ActivityName, ActivityKind.Client) + : null; + var activityContext = Activity.Current?.Context ?? default; // Propagation must still be done in all cases, to allow // downstream services to continue from parent context, if any. // Eg: Parent could be the Asp.Net activity. InstrumentRequest(request, activityContext); - if (activity == null) - { - // There is a listener but it decided not to sample the current request. - return; - } IAsyncResult asyncContext = writeAResultAccessor(request); if (asyncContext != null) { // Flow here is for [Begin]GetRequestStream[Async]. - AsyncCallbackWrapper callback = new AsyncCallbackWrapper(request, activity, asyncCallbackAccessor(asyncContext)); + AsyncCallbackWrapper callback = new AsyncCallbackWrapper(request, activity, asyncCallbackAccessor(asyncContext), Stopwatch.GetTimestamp()); asyncCallbackModifier(asyncContext, callback.AsyncCallback); } else @@ -309,17 +340,20 @@ private static void ProcessRequest(HttpWebRequest request) // Flow here is for [Begin]GetResponse[Async] without a prior call to [Begin]GetRequestStream[Async]. asyncContext = readAResultAccessor(request); - AsyncCallbackWrapper callback = new AsyncCallbackWrapper(request, activity, asyncCallbackAccessor(asyncContext)); + AsyncCallbackWrapper callback = new AsyncCallbackWrapper(request, activity, asyncCallbackAccessor(asyncContext), Stopwatch.GetTimestamp()); asyncCallbackModifier(asyncContext, callback.AsyncCallback); } - AddRequestTagsAndInstrumentRequest(request, activity); + if (activity != null) + { + AddRequestTagsAndInstrumentRequest(request, activity); + } } private static void HookOrProcessResult(HttpWebRequest request) { IAsyncResult writeAsyncContext = writeAResultAccessor(request); - if (writeAsyncContext == null || !(asyncCallbackAccessor(writeAsyncContext)?.Target is AsyncCallbackWrapper writeAsyncContextCallback)) + if (writeAsyncContext == null || asyncCallbackAccessor(writeAsyncContext)?.Target is not AsyncCallbackWrapper writeAsyncContextCallback) { // If we already hooked into the read result during ProcessRequest or we hooked up after the fact already we don't need to do anything here. return; @@ -340,19 +374,25 @@ private static void HookOrProcessResult(HttpWebRequest request) if (endCalledAccessor.Invoke(readAsyncContext) || readAsyncContext.CompletedSynchronously) { // We need to process the result directly because the read callback has already fired. Force a copy because response has likely already been disposed. - ProcessResult(readAsyncContext, null, writeAsyncContextCallback.Activity, resultAccessor(readAsyncContext), true); + ProcessResult(readAsyncContext, null, writeAsyncContextCallback.Activity, resultAccessor(readAsyncContext), true, request, writeAsyncContextCallback.StartTimestamp); return; } // Hook into the result callback if it hasn't already fired. - AsyncCallbackWrapper callback = new AsyncCallbackWrapper(writeAsyncContextCallback.Request, writeAsyncContextCallback.Activity, asyncCallbackAccessor(readAsyncContext)); + AsyncCallbackWrapper callback = new AsyncCallbackWrapper(writeAsyncContextCallback.Request, writeAsyncContextCallback.Activity, asyncCallbackAccessor(readAsyncContext), Stopwatch.GetTimestamp()); asyncCallbackModifier(readAsyncContext, callback.AsyncCallback); } - private static void ProcessResult(IAsyncResult asyncResult, AsyncCallback asyncCallback, Activity activity, object result, bool forceResponseCopy) + private static void ProcessResult(IAsyncResult asyncResult, AsyncCallback asyncCallback, Activity activity, object result, bool forceResponseCopy, HttpWebRequest request, long startTimestamp) { + HttpStatusCode? httpStatusCode = null; + + // Activity may be null if we are not tracing in these cases: + // 1. No listeners + // 2. Request was filtered out + // 3. Request was not sampled // We could be executing on a different thread now so restore the activity if needed. - if (Activity.Current != activity) + if (activity != null && Activity.Current != activity) { Activity.Current = activity; } @@ -361,7 +401,14 @@ private static void ProcessResult(IAsyncResult asyncResult, AsyncCallback asyncC { if (result is Exception ex) { - AddExceptionTags(ex, activity); + if (activity != null) + { + AddExceptionTags(ex, activity, out httpStatusCode); + } + else if (ex is WebException wexc && wexc.Response is HttpWebResponse response) + { + httpStatusCode = response.StatusCode; + } } else { @@ -382,11 +429,21 @@ private static void ProcessResult(IAsyncResult asyncResult, AsyncCallback asyncC isWebSocketResponseAccessor(response), connectionGroupNameAccessor(response), }); - AddResponseTags(responseCopy, activity); + if (activity != null) + { + AddResponseTags(responseCopy, activity); + } + + httpStatusCode = responseCopy.StatusCode; } else { - AddResponseTags(response, activity); + if (activity != null) + { + AddResponseTags(response, activity); + } + + httpStatusCode = response.StatusCode; } } } @@ -395,7 +452,65 @@ private static void ProcessResult(IAsyncResult asyncResult, AsyncCallback asyncC HttpInstrumentationEventSource.Log.FailedProcessResult(ex); } - activity.Stop(); + activity?.Stop(); + + if (HttpClientDuration.Enabled || HttpClientRequestDuration.Enabled) + { + double durationS; + double durationMs; + if (activity != null) + { + durationS = activity.Duration.TotalSeconds; + durationMs = activity.Duration.TotalMilliseconds; + } + else + { + var endTimestamp = Stopwatch.GetTimestamp(); + durationS = (endTimestamp - startTimestamp) / (double)Stopwatch.Frequency; + durationMs = durationS * 1000; + } + + if (metricsEmitOldAttributes) + { + TagList tags = default; + + tags.Add(SemanticConventions.AttributeHttpFlavor, HttpTagHelper.GetFlavorTagValueFromProtocolVersion(request.ProtocolVersion)); + tags.Add(SemanticConventions.AttributeHttpMethod, request.Method); + tags.Add(SemanticConventions.AttributeHttpScheme, request.RequestUri.Scheme); + tags.Add(SemanticConventions.AttributeNetPeerName, request.RequestUri.Host); + if (!request.RequestUri.IsDefaultPort) + { + tags.Add(SemanticConventions.AttributeNetPeerPort, request.RequestUri.Port); + } + + if (httpStatusCode.HasValue) + { + tags.Add(SemanticConventions.AttributeHttpStatusCode, (int)httpStatusCode.Value); + } + + HttpClientDuration.Record(durationMs, tags); + } + + if (metricsEmitNewAttributes) + { + TagList tags = default; + + tags.Add(SemanticConventions.AttributeHttpRequestMethod, request.Method); + tags.Add(SemanticConventions.AttributeServerAddress, request.RequestUri.Host); + tags.Add(SemanticConventions.AttributeNetworkProtocolVersion, HttpTagHelper.GetFlavorTagValueFromProtocolVersion(request.ProtocolVersion)); + if (!request.RequestUri.IsDefaultPort) + { + tags.Add(SemanticConventions.AttributeServerPort, request.RequestUri.Port); + } + + if (httpStatusCode.HasValue) + { + tags.Add(SemanticConventions.AttributeHttpResponseStatusCode, (int)httpStatusCode.Value); + } + + HttpClientRequestDuration.Record(durationS, tags); + } + } } private static void PrepareReflectionObjects() @@ -503,12 +618,8 @@ private static bool PrepareHttpWebResponseReflectionObjects(Assembly systemNetHt private static void PerformInjection() { - FieldInfo servicePointTableField = typeof(ServicePointManager).GetField("s_ServicePointTable", BindingFlags.Static | BindingFlags.NonPublic); - if (servicePointTableField == null) - { - // If anything went wrong here, just return false. There is nothing we can do. - throw new InvalidOperationException("Unable to access the ServicePointTable field"); - } + FieldInfo servicePointTableField = typeof(ServicePointManager).GetField("s_ServicePointTable", BindingFlags.Static | BindingFlags.NonPublic) + ?? throw new InvalidOperationException("Unable to access the ServicePointTable field"); Hashtable originalTable = servicePointTableField.GetValue(null) as Hashtable; ServicePointHashtable newTable = new ServicePointHashtable(originalTable ?? new Hashtable()); @@ -1109,11 +1220,12 @@ public override void Clear() /// private sealed class AsyncCallbackWrapper { - public AsyncCallbackWrapper(HttpWebRequest request, Activity activity, AsyncCallback originalCallback) + public AsyncCallbackWrapper(HttpWebRequest request, Activity activity, AsyncCallback originalCallback, long startTimestamp) { this.Request = request; this.Activity = activity; this.OriginalCallback = originalCallback; + this.StartTimestamp = startTimestamp; } public HttpWebRequest Request { get; } @@ -1122,12 +1234,21 @@ public AsyncCallbackWrapper(HttpWebRequest request, Activity activity, AsyncCall public AsyncCallback OriginalCallback { get; } + public long StartTimestamp { get; } + public void AsyncCallback(IAsyncResult asyncResult) { object result = resultAccessor(asyncResult); if (result is Exception || result is HttpWebResponse) { - ProcessResult(asyncResult, this.OriginalCallback, this.Activity, result, false); + ProcessResult( + asyncResult, + this.OriginalCallback, + this.Activity, + result, + forceResponseCopy: false, + this.Request, + this.StartTimestamp); } this.OriginalCallback?.Invoke(asyncResult); diff --git a/src/OpenTelemetry.Instrumentation.Http/MeterProviderBuilderExtensions.cs b/src/OpenTelemetry.Instrumentation.Http/MeterProviderBuilderExtensions.cs index 38d375436b0..c82138dbbc1 100644 --- a/src/OpenTelemetry.Instrumentation.Http/MeterProviderBuilderExtensions.cs +++ b/src/OpenTelemetry.Instrumentation.Http/MeterProviderBuilderExtensions.cs @@ -45,16 +45,30 @@ public static MeterProviderBuilder AddHttpClientInstrumentation( services.RegisterOptionsFactory(configuration => new HttpClientMetricInstrumentationOptions(configuration)); }); - // TODO: Implement an IDeferredMeterProviderBuilder - - // TODO: Handle HttpClientInstrumentationOptions + // TODO: Handle HttpClientMetricInstrumentationOptions // SetHttpFlavor - seems like this would be handled by views // Filter - makes sense for metric instrumentation // Enrich - do we want a similar kind of functionality for metrics? // RecordException - probably doesn't make sense for metric instrumentation - builder.AddMeter(HttpClientMetrics.InstrumentationName); - return builder.AddInstrumentation(sp => new HttpClientMetrics( - sp.GetRequiredService>().CurrentValue)); +#if NETFRAMEWORK + builder.AddMeter(HttpWebRequestActivitySource.MeterName); + + if (builder is IDeferredMeterProviderBuilder deferredMeterProviderBuilder) + { + deferredMeterProviderBuilder.Configure((sp, builder) => + { + var options = sp.GetRequiredService>().Get(Options.DefaultName); + + HttpWebRequestActivitySource.MetricsOptions = options; + }); + } +#else + builder.AddMeter(HttpHandlerMetricsDiagnosticListener.MeterName); + + builder.AddInstrumentation(sp => new HttpClientMetrics( + sp.GetRequiredService>().Get(Options.DefaultName))); +#endif + return builder; } } diff --git a/src/OpenTelemetry.Instrumentation.Http/README.md b/src/OpenTelemetry.Instrumentation.Http/README.md index 7ca9400d4b2..18b6192a105 100644 --- a/src/OpenTelemetry.Instrumentation.Http/README.md +++ b/src/OpenTelemetry.Instrumentation.Http/README.md @@ -67,9 +67,6 @@ public class Program #### Metrics -> **Note** -> Metrics are not available for .NET Framework. - The following example demonstrates adding `HttpClient` instrumentation with the extension method `.AddHttpClientInstrumentation()` on `MeterProviderBuilder` to a console application. This example also sets up the OpenTelemetry Console diff --git a/src/OpenTelemetry.Instrumentation.Http/TracerProviderBuilderExtensions.cs b/src/OpenTelemetry.Instrumentation.Http/TracerProviderBuilderExtensions.cs index 6763fc3e204..9429819b8a2 100644 --- a/src/OpenTelemetry.Instrumentation.Http/TracerProviderBuilderExtensions.cs +++ b/src/OpenTelemetry.Instrumentation.Http/TracerProviderBuilderExtensions.cs @@ -84,7 +84,7 @@ public static TracerProviderBuilder AddHttpClientInstrumentation( { var options = sp.GetRequiredService>().Get(name); - HttpWebRequestActivitySource.Options = options; + HttpWebRequestActivitySource.TracingOptions = options; }); } #else diff --git a/test/OpenTelemetry.Instrumentation.Http.Tests/HttpClientTests.Basic.cs b/test/OpenTelemetry.Instrumentation.Http.Tests/HttpClientTests.Basic.cs index d7b41b75359..e039f25ac12 100644 --- a/test/OpenTelemetry.Instrumentation.Http.Tests/HttpClientTests.Basic.cs +++ b/test/OpenTelemetry.Instrumentation.Http.Tests/HttpClientTests.Basic.cs @@ -26,23 +26,32 @@ using OpenTelemetry.Tests; using OpenTelemetry.Trace; using Xunit; +using Xunit.Abstractions; namespace OpenTelemetry.Instrumentation.Http.Tests; public partial class HttpClientTests : IDisposable { + private readonly ITestOutputHelper output; private readonly IDisposable serverLifeTime; + private readonly string host; + private readonly int port; private readonly string url; - public HttpClientTests() + public HttpClientTests(ITestOutputHelper output) { + this.output = output; + this.serverLifeTime = TestHttpServer.RunServer( (ctx) => { string traceparent = ctx.Request.Headers["traceparent"]; string custom_traceparent = ctx.Request.Headers["custom_traceparent"]; - if (string.IsNullOrWhiteSpace(traceparent) - && string.IsNullOrWhiteSpace(custom_traceparent)) + if ((ctx.Request.Headers["contextRequired"] == null + || bool.Parse(ctx.Request.Headers["contextRequired"])) + && + (string.IsNullOrWhiteSpace(traceparent) + && string.IsNullOrWhiteSpace(custom_traceparent))) { ctx.Response.StatusCode = 500; ctx.Response.StatusDescription = "Missing trace context"; @@ -56,6 +65,10 @@ public HttpClientTests() ctx.Response.RedirectLocation = "/"; ctx.Response.StatusCode = 302; } + else if (ctx.Request.Headers["responseCode"] != null) + { + ctx.Response.StatusCode = int.Parse(ctx.Request.Headers["responseCode"]); + } else { ctx.Response.StatusCode = 200; @@ -66,7 +79,11 @@ public HttpClientTests() out var host, out var port); + this.host = host; + this.port = port; this.url = $"http://{host}:{port}/"; + + this.output.WriteLine($"HttpServer started: {this.url}"); } [Fact] @@ -631,6 +648,7 @@ public async Task CustomPropagatorCalled(bool sample, bool createParentActivity) public void Dispose() { this.serverLifeTime?.Dispose(); + this.output.WriteLine($"HttpServer stopped: {this.url}"); Activity.Current = null; GC.SuppressFinalize(this); } diff --git a/test/OpenTelemetry.Instrumentation.Http.Tests/HttpClientTests.cs b/test/OpenTelemetry.Instrumentation.Http.Tests/HttpClientTests.cs index 0ceeaa1eb42..93bdfa54ecb 100644 --- a/test/OpenTelemetry.Instrumentation.Http.Tests/HttpClientTests.cs +++ b/test/OpenTelemetry.Instrumentation.Http.Tests/HttpClientTests.cs @@ -20,21 +20,141 @@ #endif using System.Reflection; using System.Text.Json; -using Moq; +using Microsoft.Extensions.Configuration; +using Microsoft.Extensions.DependencyInjection; using OpenTelemetry.Metrics; -using OpenTelemetry.Tests; using OpenTelemetry.Trace; using Xunit; +using static OpenTelemetry.Internal.HttpSemanticConventionHelper; namespace OpenTelemetry.Instrumentation.Http.Tests; public partial class HttpClientTests { - public static IEnumerable TestData => HttpTestData.ReadTestCases(); + public static readonly IEnumerable TestData = HttpTestData.ReadTestCases(); [Theory] [MemberData(nameof(TestData))] - public async Task HttpOutCallsAreCollectedSuccessfullyAsync(HttpTestData.HttpOutTestCase tc) + public async Task HttpOutCallsAreCollectedSuccessfullyTracesAndMetricsOldSemanticConventionsAsync(HttpTestData.HttpOutTestCase tc) + { + await HttpOutCallsAreCollectedSuccessfullyBodyAsync( + this.host, + this.port, + tc, + enableTracing: true, + enableMetrics: true, + semanticConvention: HttpSemanticConvention.Old).ConfigureAwait(false); + } + + [Theory] + [MemberData(nameof(TestData))] + public async Task HttpOutCallsAreCollectedSuccessfullyTracesAndMetricsNewSemanticConventionsAsync(HttpTestData.HttpOutTestCase tc) + { + await HttpOutCallsAreCollectedSuccessfullyBodyAsync( + this.host, + this.port, + tc, + enableTracing: true, + enableMetrics: true, + semanticConvention: HttpSemanticConvention.New).ConfigureAwait(false); + } + + [Theory] + [MemberData(nameof(TestData))] + public async Task HttpOutCallsAreCollectedSuccessfullyTracesAndMetricsDuplicateSemanticConventionsAsync(HttpTestData.HttpOutTestCase tc) + { + await HttpOutCallsAreCollectedSuccessfullyBodyAsync( + this.host, + this.port, + tc, + enableTracing: true, + enableMetrics: true, + semanticConvention: HttpSemanticConvention.Dupe).ConfigureAwait(false); + } + + [Theory] + [MemberData(nameof(TestData))] + public async Task HttpOutCallsAreCollectedSuccessfullyTracesOnlyAsync(HttpTestData.HttpOutTestCase tc) + { + await HttpOutCallsAreCollectedSuccessfullyBodyAsync( + this.host, + this.port, + tc, + enableTracing: true, + enableMetrics: false).ConfigureAwait(false); + } + + [Theory] + [MemberData(nameof(TestData))] + public async Task HttpOutCallsAreCollectedSuccessfullyMetricsOnlyAsync(HttpTestData.HttpOutTestCase tc) + { + await HttpOutCallsAreCollectedSuccessfullyBodyAsync( + this.host, + this.port, + tc, + enableTracing: false, + enableMetrics: true).ConfigureAwait(false); + } + + [Theory] + [MemberData(nameof(TestData))] + public async Task HttpOutCallsAreCollectedSuccessfullyNoSignalsAsync(HttpTestData.HttpOutTestCase tc) + { + await HttpOutCallsAreCollectedSuccessfullyBodyAsync( + this.host, + this.port, + tc, + enableTracing: false, + enableMetrics: false).ConfigureAwait(false); + } + + [Fact] + public async Task DebugIndividualTestAsync() + { + var input = JsonSerializer.Deserialize( + @" + [ + { + ""name"": ""Response code: 399"", + ""method"": ""GET"", + ""url"": ""http://{host}:{port}/"", + ""responseCode"": 399, + ""responseExpected"": true, + ""spanName"": ""HTTP GET"", + ""spanStatus"": ""Unset"", + ""spanKind"": ""Client"", + ""spanAttributes"": { + ""http.scheme"": ""http"", + ""http.method"": ""GET"", + ""net.peer.name"": ""{host}"", + ""net.peer.port"": ""{port}"", + ""http.status_code"": ""399"", + ""http.flavor"": ""{flavor}"", + ""http.url"": ""http://{host}:{port}/"" + } + } + ] + ", + new JsonSerializerOptions { PropertyNamingPolicy = JsonNamingPolicy.CamelCase }); + + var t = (Task)this.GetType().InvokeMember(nameof(this.HttpOutCallsAreCollectedSuccessfullyTracesAndMetricsOldSemanticConventionsAsync), BindingFlags.InvokeMethod, null, this, HttpTestData.GetArgumentsFromTestCaseObject(input).First()); + await t.ConfigureAwait(false); + } + + [Fact] + public async Task CheckEnrichmentWhenSampling() + { + await CheckEnrichment(new AlwaysOffSampler(), false, this.url).ConfigureAwait(false); + await CheckEnrichment(new AlwaysOnSampler(), true, this.url).ConfigureAwait(false); + } + + private static async Task HttpOutCallsAreCollectedSuccessfullyBodyAsync( + string host, + int port, + HttpTestData.HttpOutTestCase tc, + bool enableTracing, + bool enableMetrics, + HttpSemanticConvention? semanticConvention = null) { bool enrichWithHttpWebRequestCalled = false; bool enrichWithHttpWebResponseCalled = false; @@ -42,216 +162,326 @@ public async Task HttpOutCallsAreCollectedSuccessfullyAsync(HttpTestData.HttpOut bool enrichWithHttpResponseMessageCalled = false; bool enrichWithExceptionCalled = false; - using var serverLifeTime = TestHttpServer.RunServer( - (ctx) => - { - ctx.Response.StatusCode = tc.ResponseCode == 0 ? 200 : tc.ResponseCode; - ctx.Response.OutputStream.Close(); - }, - out var host, - out var port); + var testUrl = HttpTestData.NormalizeValues(tc.Url, host, port); + + var meterProviderBuilder = Sdk.CreateMeterProviderBuilder(); - var processor = new Mock>(); - tc.Url = HttpTestData.NormalizeValues(tc.Url, host, port); + if (enableMetrics) + { + meterProviderBuilder + .AddHttpClientInstrumentation() + .ConfigureServices( + s => s.AddSingleton(BuildConfigurationWithSemanticConventionOptIn(semanticConvention))); + } + + var tracerProviderBuilder = Sdk.CreateTracerProviderBuilder(); + + if (enableTracing) + { + tracerProviderBuilder + .AddHttpClientInstrumentation((opt) => + { + opt.EnrichWithHttpWebRequest = (activity, httpRequestMessage) => { enrichWithHttpWebRequestCalled = true; }; + opt.EnrichWithHttpWebResponse = (activity, httpResponseMessage) => { enrichWithHttpWebResponseCalled = true; }; + opt.EnrichWithHttpRequestMessage = (activity, httpRequestMessage) => { enrichWithHttpRequestMessageCalled = true; }; + opt.EnrichWithHttpResponseMessage = (activity, httpResponseMessage) => { enrichWithHttpResponseMessageCalled = true; }; + opt.EnrichWithException = (activity, exception) => { enrichWithExceptionCalled = true; }; + opt.RecordException = tc.RecordException ?? false; + }) + .ConfigureServices( + s => s.AddSingleton(BuildConfigurationWithSemanticConventionOptIn(semanticConvention))); + } var metrics = new List(); + var activities = new List(); - var meterProvider = Sdk.CreateMeterProviderBuilder() - .AddHttpClientInstrumentation() + var meterProvider = meterProviderBuilder .AddInMemoryExporter(metrics) .Build(); - using (Sdk.CreateTracerProviderBuilder() - .AddHttpClientInstrumentation((opt) => - { - opt.EnrichWithHttpWebRequest = (activity, httpRequestMessage) => { enrichWithHttpWebRequestCalled = true; }; - opt.EnrichWithHttpWebResponse = (activity, httpResponseMessage) => { enrichWithHttpWebResponseCalled = true; }; - opt.EnrichWithHttpRequestMessage = (activity, httpRequestMessage) => { enrichWithHttpRequestMessageCalled = true; }; - opt.EnrichWithHttpResponseMessage = (activity, httpResponseMessage) => { enrichWithHttpResponseMessageCalled = true; }; - opt.EnrichWithException = (activity, exception) => { enrichWithExceptionCalled = true; }; - opt.RecordException = tc.RecordException ?? false; - }) - .AddProcessor(processor.Object) - .Build()) + var tracerProvider = tracerProviderBuilder + .AddInMemoryExporter(activities) + .Build(); + + try { - try + using var c = new HttpClient(); + using var request = new HttpRequestMessage { - using var c = new HttpClient(); - using var request = new HttpRequestMessage - { - RequestUri = new Uri(tc.Url), - Method = new HttpMethod(tc.Method), + RequestUri = new Uri(testUrl), + Method = new HttpMethod(tc.Method), #if NETFRAMEWORK - Version = new Version(1, 1), + Version = new Version(1, 1), #else - Version = new Version(2, 0), + Version = new Version(2, 0), #endif - }; + }; - if (tc.Headers != null) + if (tc.Headers != null) + { + foreach (var header in tc.Headers) { - foreach (var header in tc.Headers) - { - request.Headers.Add(header.Key, header.Value); - } + request.Headers.Add(header.Key, header.Value); } - - await c.SendAsync(request).ConfigureAwait(false); } - catch (Exception) - { - // test case can intentionally send request that will result in exception - } - } - - meterProvider.Dispose(); - - var requestMetrics = metrics - .Where(metric => metric.Name == "http.client.duration") - .ToArray(); - Assert.Equal(5, processor.Invocations.Count); // SetParentProvider/OnStart/OnEnd/OnShutdown/Dispose called. - var activity = (Activity)processor.Invocations[2].Arguments[0]; + request.Headers.Add("contextRequired", "false"); + request.Headers.Add("responseCode", (tc.ResponseCode == 0 ? 200 : tc.ResponseCode).ToString()); - Assert.Equal(ActivityKind.Client, activity.Kind); - Assert.Equal(tc.SpanName, activity.DisplayName); - -#if NETFRAMEWORK - Assert.True(enrichWithHttpWebRequestCalled); - Assert.False(enrichWithHttpRequestMessageCalled); - if (tc.ResponseExpected) - { - Assert.True(enrichWithHttpWebResponseCalled); - Assert.False(enrichWithHttpResponseMessageCalled); + await c.SendAsync(request).ConfigureAwait(false); } -#else - Assert.False(enrichWithHttpWebRequestCalled); - Assert.True(enrichWithHttpRequestMessageCalled); - if (tc.ResponseExpected) + catch (Exception) { - Assert.False(enrichWithHttpWebResponseCalled); - Assert.True(enrichWithHttpResponseMessageCalled); + // test case can intentionally send request that will result in exception } -#endif - - // Assert.Equal(tc.SpanStatus, d[span.Status.CanonicalCode]); - Assert.Equal(tc.SpanStatus, activity.Status.ToString()); - - if (tc.SpanStatusHasDescription.HasValue) + finally { - var desc = activity.StatusDescription; - Assert.Equal(tc.SpanStatusHasDescription.Value, !string.IsNullOrEmpty(desc)); + tracerProvider.Dispose(); + meterProvider.Dispose(); } - var normalizedAttributes = activity.TagObjects.Where(kv => !kv.Key.StartsWith("otel.")).ToDictionary(x => x.Key, x => x.Value.ToString()); - var normalizedAttributesTestCase = tc.SpanAttributes.ToDictionary(x => x.Key, x => HttpTestData.NormalizeValues(x.Value, host, port)); + var requestMetrics = metrics + .Where(metric => metric.Name == "http.client.duration" || metric.Name == "http.client.request.duration") + .ToArray(); - Assert.Equal(normalizedAttributesTestCase.Count, normalizedAttributes.Count); + var normalizedAttributesTestCase = tc.SpanAttributes.ToDictionary(x => x.Key, x => HttpTestData.NormalizeValues(x.Value, host, port)); - foreach (var kv in normalizedAttributesTestCase) + if (!enableTracing) { - Assert.Contains(activity.TagObjects, i => i.Key == kv.Key && i.Value.ToString().Equals(kv.Value, StringComparison.OrdinalIgnoreCase)); + Assert.Empty(activities); } - - if (tc.RecordException.HasValue && tc.RecordException.Value) + else { - Assert.Single(activity.Events.Where(evt => evt.Name.Equals("exception"))); - Assert.True(enrichWithExceptionCalled); - } + var activity = Assert.Single(activities); + + Assert.Equal(ActivityKind.Client, activity.Kind); + Assert.Equal(tc.SpanName, activity.DisplayName); #if NETFRAMEWORK - Assert.Empty(requestMetrics); + Assert.True(enrichWithHttpWebRequestCalled); + Assert.False(enrichWithHttpRequestMessageCalled); + if (tc.ResponseExpected) + { + Assert.True(enrichWithHttpWebResponseCalled); + Assert.False(enrichWithHttpResponseMessageCalled); + } #else - Assert.Single(requestMetrics); + Assert.False(enrichWithHttpWebRequestCalled); + Assert.True(enrichWithHttpRequestMessageCalled); + if (tc.ResponseExpected) + { + Assert.False(enrichWithHttpWebResponseCalled); + Assert.True(enrichWithHttpResponseMessageCalled); + } +#endif - var metric = requestMetrics[0]; - Assert.NotNull(metric); - Assert.True(metric.MetricType == MetricType.Histogram); + // Assert.Equal(tc.SpanStatus, d[span.Status.CanonicalCode]); + Assert.Equal(tc.SpanStatus, activity.Status.ToString()); - var metricPoints = new List(); - foreach (var p in metric.GetMetricPoints()) - { - metricPoints.Add(p); - } + if (tc.SpanStatusHasDescription.HasValue) + { + var desc = activity.StatusDescription; + Assert.Equal(tc.SpanStatusHasDescription.Value, !string.IsNullOrEmpty(desc)); + } - Assert.Single(metricPoints); - var metricPoint = metricPoints[0]; + var normalizedAttributes = activity.TagObjects.Where(kv => !kv.Key.StartsWith("otel.")).ToDictionary(x => x.Key, x => x.Value.ToString()); - var count = metricPoint.GetHistogramCount(); - var sum = metricPoint.GetHistogramSum(); + var expectedAttributeCount = semanticConvention == HttpSemanticConvention.Dupe + ? 11 + (tc.ResponseExpected ? 2 : 0) + : semanticConvention == HttpSemanticConvention.New + ? 5 + (tc.ResponseExpected ? 1 : 0) + : 6 + (tc.ResponseExpected ? 1 : 0); - Assert.Equal(1L, count); - Assert.Equal(activity.Duration.TotalMilliseconds, sum); + Assert.Equal(expectedAttributeCount, normalizedAttributes.Count); - var attributes = new KeyValuePair[metricPoint.Tags.Count]; - int i = 0; - foreach (var tag in metricPoint.Tags) - { - attributes[i++] = tag; + if (semanticConvention == null || semanticConvention.Value.HasFlag(HttpSemanticConvention.Old)) + { + Assert.Contains(normalizedAttributes, kvp => kvp.Key == SemanticConventions.AttributeHttpMethod && kvp.Value.ToString() == normalizedAttributesTestCase[SemanticConventions.AttributeHttpMethod]); + Assert.Contains(normalizedAttributes, kvp => kvp.Key == SemanticConventions.AttributeNetPeerName && kvp.Value.ToString() == normalizedAttributesTestCase[SemanticConventions.AttributeNetPeerName]); + Assert.Contains(normalizedAttributes, kvp => kvp.Key == SemanticConventions.AttributeNetPeerPort && kvp.Value.ToString() == normalizedAttributesTestCase[SemanticConventions.AttributeNetPeerPort]); + Assert.Contains(normalizedAttributes, kvp => kvp.Key == SemanticConventions.AttributeHttpScheme && kvp.Value.ToString() == normalizedAttributesTestCase[SemanticConventions.AttributeHttpScheme]); + Assert.Contains(normalizedAttributes, kvp => kvp.Key == SemanticConventions.AttributeHttpUrl && kvp.Value.ToString() == normalizedAttributesTestCase[SemanticConventions.AttributeHttpUrl]); + Assert.Contains(normalizedAttributes, kvp => kvp.Key == SemanticConventions.AttributeHttpFlavor && kvp.Value.ToString() == normalizedAttributesTestCase[SemanticConventions.AttributeHttpFlavor]); + if (tc.ResponseExpected) + { + Assert.Contains(normalizedAttributes, kvp => kvp.Key == SemanticConventions.AttributeHttpStatusCode && kvp.Value.ToString() == normalizedAttributesTestCase[SemanticConventions.AttributeHttpStatusCode]); + } + else + { + Assert.DoesNotContain(normalizedAttributes, kvp => kvp.Key == SemanticConventions.AttributeHttpStatusCode); + } + } + + if (semanticConvention != null && semanticConvention.Value.HasFlag(HttpSemanticConvention.New)) + { + Assert.Contains(normalizedAttributes, kvp => kvp.Key == SemanticConventions.AttributeHttpRequestMethod && kvp.Value.ToString() == normalizedAttributesTestCase[SemanticConventions.AttributeHttpMethod]); + Assert.Contains(normalizedAttributes, kvp => kvp.Key == SemanticConventions.AttributeServerAddress && kvp.Value.ToString() == normalizedAttributesTestCase[SemanticConventions.AttributeNetPeerName]); + Assert.Contains(normalizedAttributes, kvp => kvp.Key == SemanticConventions.AttributeServerPort && kvp.Value.ToString() == normalizedAttributesTestCase[SemanticConventions.AttributeNetPeerPort]); + Assert.Contains(normalizedAttributes, kvp => kvp.Key == SemanticConventions.AttributeUrlFull && kvp.Value.ToString() == normalizedAttributesTestCase[SemanticConventions.AttributeHttpUrl]); + Assert.Contains(normalizedAttributes, kvp => kvp.Key == SemanticConventions.AttributeNetworkProtocolVersion && kvp.Value.ToString() == normalizedAttributesTestCase[SemanticConventions.AttributeHttpFlavor]); + if (tc.ResponseExpected) + { + Assert.Contains(normalizedAttributes, kvp => kvp.Key == SemanticConventions.AttributeHttpResponseStatusCode && kvp.Value.ToString() == normalizedAttributesTestCase[SemanticConventions.AttributeHttpStatusCode]); + } + else + { + Assert.DoesNotContain(normalizedAttributes, kvp => kvp.Key == SemanticConventions.AttributeHttpResponseStatusCode); + } + } + + if (tc.RecordException.HasValue && tc.RecordException.Value) + { + Assert.Single(activity.Events.Where(evt => evt.Name.Equals("exception"))); + Assert.True(enrichWithExceptionCalled); + } } - var method = new KeyValuePair(SemanticConventions.AttributeHttpMethod, tc.Method); - var scheme = new KeyValuePair(SemanticConventions.AttributeHttpScheme, "http"); - var statusCode = new KeyValuePair(SemanticConventions.AttributeHttpStatusCode, tc.ResponseCode == 0 ? 200 : tc.ResponseCode); - var flavor = new KeyValuePair(SemanticConventions.AttributeHttpFlavor, "2.0"); - var hostName = new KeyValuePair(SemanticConventions.AttributeNetPeerName, tc.ResponseExpected ? host : "sdlfaldfjalkdfjlkajdflkajlsdjf"); - var portNumber = new KeyValuePair(SemanticConventions.AttributeNetPeerPort, port); - Assert.Contains(hostName, attributes); - Assert.Contains(portNumber, attributes); - Assert.Contains(method, attributes); - Assert.Contains(scheme, attributes); - Assert.Contains(flavor, attributes); - if (tc.ResponseExpected) + if (!enableMetrics) { - Assert.Contains(statusCode, attributes); - Assert.Equal(6, attributes.Length); + Assert.Empty(requestMetrics); } else { - Assert.DoesNotContain(statusCode, attributes); - Assert.Equal(5, attributes.Length); - } -#endif - } + if (semanticConvention == HttpSemanticConvention.Dupe) + { + Assert.Equal(2, requestMetrics.Length); + } + else + { + Assert.Single(requestMetrics); + } - [Fact] - public async Task DebugIndividualTestAsync() - { - var input = JsonSerializer.Deserialize( - @" - [ - { - ""name"": ""Response code: 399"", - ""method"": ""GET"", - ""url"": ""http://{host}:{port}/"", - ""responseCode"": 399, - ""responseExpected"": true, - ""spanName"": ""HTTP GET"", - ""spanStatus"": ""Unset"", - ""spanKind"": ""Client"", - ""spanAttributes"": { - ""http.scheme"": ""http"", - ""http.method"": ""GET"", - ""net.peer.name"": ""{host}"", - ""net.peer.port"": ""{port}"", - ""http.status_code"": ""399"", - ""http.flavor"": ""{flavor}"", - ""http.url"": ""http://{host}:{port}/"" + if (semanticConvention == null || semanticConvention.Value.HasFlag(HttpSemanticConvention.Old)) + { + var metric = requestMetrics.FirstOrDefault(m => m.Name == "http.client.duration"); + Assert.NotNull(metric); + Assert.True(metric.MetricType == MetricType.Histogram); + + var metricPoints = new List(); + foreach (var p in metric.GetMetricPoints()) + { + metricPoints.Add(p); + } + + Assert.Single(metricPoints); + var metricPoint = metricPoints[0]; + + var count = metricPoint.GetHistogramCount(); + var sum = metricPoint.GetHistogramSum(); + + Assert.Equal(1L, count); + + if (enableTracing) + { + var activity = Assert.Single(activities); + Assert.Equal(activity.Duration.TotalMilliseconds, sum); + } + else + { + Assert.True(sum > 0); + } + + var attributes = new Dictionary(); + foreach (var tag in metricPoint.Tags) + { + attributes[tag.Key] = tag.Value; + } + + var expectedAttributeCount = 5 + (tc.ResponseExpected ? 1 : 0); + + Assert.Equal(expectedAttributeCount, attributes.Count); + + if (semanticConvention == null || semanticConvention.Value.HasFlag(HttpSemanticConvention.Old)) + { + Assert.Contains(attributes, kvp => kvp.Key == SemanticConventions.AttributeHttpMethod && kvp.Value.ToString() == normalizedAttributesTestCase[SemanticConventions.AttributeHttpMethod]); + Assert.Contains(attributes, kvp => kvp.Key == SemanticConventions.AttributeNetPeerName && kvp.Value.ToString() == normalizedAttributesTestCase[SemanticConventions.AttributeNetPeerName]); + Assert.Contains(attributes, kvp => kvp.Key == SemanticConventions.AttributeNetPeerPort && kvp.Value.ToString() == normalizedAttributesTestCase[SemanticConventions.AttributeNetPeerPort]); + Assert.Contains(attributes, kvp => kvp.Key == SemanticConventions.AttributeHttpScheme && kvp.Value.ToString() == normalizedAttributesTestCase[SemanticConventions.AttributeHttpScheme]); + Assert.Contains(attributes, kvp => kvp.Key == SemanticConventions.AttributeHttpFlavor && kvp.Value.ToString() == normalizedAttributesTestCase[SemanticConventions.AttributeHttpFlavor]); + if (tc.ResponseExpected) + { + Assert.Contains(attributes, kvp => kvp.Key == SemanticConventions.AttributeHttpStatusCode && kvp.Value.ToString() == normalizedAttributesTestCase[SemanticConventions.AttributeHttpStatusCode]); } - } - ] - ", - new JsonSerializerOptions { PropertyNamingPolicy = JsonNamingPolicy.CamelCase }); + else + { + Assert.DoesNotContain(attributes, kvp => kvp.Key == SemanticConventions.AttributeHttpStatusCode); + } + } + } - var t = (Task)this.GetType().InvokeMember(nameof(this.HttpOutCallsAreCollectedSuccessfullyAsync), BindingFlags.InvokeMethod, null, this, HttpTestData.GetArgumentsFromTestCaseObject(input).First()); - await t.ConfigureAwait(false); + if (semanticConvention != null && semanticConvention.Value.HasFlag(HttpSemanticConvention.New)) + { + var metric = requestMetrics.FirstOrDefault(m => m.Name == "http.client.request.duration"); + Assert.NotNull(metric); + Assert.True(metric.MetricType == MetricType.Histogram); + + var metricPoints = new List(); + foreach (var p in metric.GetMetricPoints()) + { + metricPoints.Add(p); + } + + Assert.Single(metricPoints); + var metricPoint = metricPoints[0]; + + var count = metricPoint.GetHistogramCount(); + var sum = metricPoint.GetHistogramSum(); + + Assert.Equal(1L, count); + + if (enableTracing) + { + var activity = Assert.Single(activities); + Assert.Equal(activity.Duration.TotalSeconds, sum); + } + else + { + Assert.True(sum > 0); + } + + var attributes = new Dictionary(); + foreach (var tag in metricPoint.Tags) + { + attributes[tag.Key] = tag.Value; + } + + var expectedAttributeCount = 4 + (tc.ResponseExpected ? 1 : 0); + + Assert.Equal(expectedAttributeCount, attributes.Count); + + Assert.Contains(attributes, kvp => kvp.Key == SemanticConventions.AttributeHttpRequestMethod && kvp.Value.ToString() == normalizedAttributesTestCase[SemanticConventions.AttributeHttpMethod]); + Assert.Contains(attributes, kvp => kvp.Key == SemanticConventions.AttributeServerAddress && kvp.Value.ToString() == normalizedAttributesTestCase[SemanticConventions.AttributeNetPeerName]); + Assert.Contains(attributes, kvp => kvp.Key == SemanticConventions.AttributeServerPort && kvp.Value.ToString() == normalizedAttributesTestCase[SemanticConventions.AttributeNetPeerPort]); + Assert.Contains(attributes, kvp => kvp.Key == SemanticConventions.AttributeNetworkProtocolVersion && kvp.Value.ToString() == normalizedAttributesTestCase[SemanticConventions.AttributeHttpFlavor]); + if (tc.ResponseExpected) + { + Assert.Contains(attributes, kvp => kvp.Key == SemanticConventions.AttributeHttpResponseStatusCode && kvp.Value.ToString() == normalizedAttributesTestCase[SemanticConventions.AttributeHttpStatusCode]); + } + else + { + Assert.DoesNotContain(attributes, kvp => kvp.Key == SemanticConventions.AttributeHttpResponseStatusCode); + } + } + } } - [Fact] - public async Task CheckEnrichmentWhenSampling() + private static IConfiguration BuildConfigurationWithSemanticConventionOptIn( + HttpSemanticConvention? semanticConvention) { - await CheckEnrichment(new AlwaysOffSampler(), false, this.url).ConfigureAwait(false); - await CheckEnrichment(new AlwaysOnSampler(), true, this.url).ConfigureAwait(false); + var builder = new ConfigurationBuilder(); + + if (semanticConvention != null && semanticConvention != HttpSemanticConvention.Old) + { + builder.AddInMemoryCollection( + new Dictionary + { + ["OTEL_SEMCONV_STABILITY_OPT_IN"] = semanticConvention == HttpSemanticConvention.Dupe + ? "http/dup" + : "http", + }); + } + + return builder.Build(); } private static async Task CheckEnrichment(Sampler sampler, bool enrichExpected, string url) @@ -262,7 +492,6 @@ private static async Task CheckEnrichment(Sampler sampler, bool enrichExpected, bool enrichWithHttpRequestMessageCalled = false; bool enrichWithHttpResponseMessageCalled = false; - var processor = new Mock>(); using (Sdk.CreateTracerProviderBuilder() .SetSampler(sampler) .AddHttpClientInstrumentation(options => @@ -273,7 +502,6 @@ private static async Task CheckEnrichment(Sampler sampler, bool enrichExpected, options.EnrichWithHttpRequestMessage = (activity, httpRequestMessage) => { enrichWithHttpRequestMessageCalled = true; }; options.EnrichWithHttpResponseMessage = (activity, httpResponseMessage) => { enrichWithHttpResponseMessageCalled = true; }; }) - .AddProcessor(processor.Object) .Build()) { using var c = new HttpClient(); diff --git a/test/OpenTelemetry.Instrumentation.Http.Tests/HttpWebRequestActivitySourceTests.netfx.cs b/test/OpenTelemetry.Instrumentation.Http.Tests/HttpWebRequestActivitySourceTests.netfx.cs index 5ad9f6f2d69..6eca1d40eed 100644 --- a/test/OpenTelemetry.Instrumentation.Http.Tests/HttpWebRequestActivitySourceTests.netfx.cs +++ b/test/OpenTelemetry.Instrumentation.Http.Tests/HttpWebRequestActivitySourceTests.netfx.cs @@ -32,7 +32,6 @@ public class HttpWebRequestActivitySourceTests : IDisposable private readonly IDisposable testServer; private readonly string testServerHost; private readonly int testServerPort; - private readonly string hostNameAndPort; private readonly string netPeerName; private readonly int netPeerPort; @@ -51,10 +50,8 @@ static HttpWebRequestActivitySourceTests() }, }; - HttpWebRequestActivitySource.Options = options; + HttpWebRequestActivitySource.TracingOptions = options; - // Need to touch something in HttpWebRequestActivitySource/Sdk to do the static injection. - GC.KeepAlive(HttpWebRequestActivitySource.Options); _ = Sdk.SuppressInstrumentation; } @@ -69,7 +66,6 @@ public HttpWebRequestActivitySourceTests() out this.testServerHost, out this.testServerPort); - this.hostNameAndPort = $"{this.testServerHost}:{this.testServerPort}"; this.netPeerName = this.testServerHost; this.netPeerPort = this.testServerPort; diff --git a/test/OpenTelemetry.Instrumentation.Http.Tests/HttpWebRequestActivitySourceTestsDupe.netfx.cs b/test/OpenTelemetry.Instrumentation.Http.Tests/HttpWebRequestActivitySourceTestsDupe.netfx.cs deleted file mode 100644 index 268f805d298..00000000000 --- a/test/OpenTelemetry.Instrumentation.Http.Tests/HttpWebRequestActivitySourceTestsDupe.netfx.cs +++ /dev/null @@ -1,311 +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. -// - -#if NETFRAMEWORK -using System.Collections.Concurrent; -using System.Diagnostics; -using System.Net; -using System.Net.Http; -using Microsoft.Extensions.Configuration; -using OpenTelemetry.Instrumentation.Http.Implementation; -using OpenTelemetry.Tests; -using OpenTelemetry.Trace; -using Xunit; -using static OpenTelemetry.Internal.HttpSemanticConventionHelper; - -namespace OpenTelemetry.Instrumentation.Http.Tests; - -// Tests for v1.21.0 Semantic Conventions for Http spans -// see the spec https://github.com/open-telemetry/semantic-conventions/blob/v1.21.0/docs/http/http-spans.md -// These tests emit both the new and older attributes. -// This test class can be deleted when this library is GA. -public class HttpWebRequestActivitySourceTestsDupe : IDisposable -{ - private readonly IDisposable testServer; - private readonly string testServerHost; - private readonly int testServerPort; - private readonly string hostNameAndPort; - private readonly string netPeerName; - private readonly int netPeerPort; - - static HttpWebRequestActivitySourceTestsDupe() - { - var configuration = new ConfigurationBuilder() - .AddInMemoryCollection(new Dictionary { [SemanticConventionOptInKeyName] = "http/dup" }) - .Build(); - - HttpClientInstrumentationOptions options = new(configuration) - { - EnrichWithHttpWebRequest = (activity, httpWebRequest) => - { - VerifyHeaders(httpWebRequest); - }, - }; - - HttpWebRequestActivitySource.Options = options; - - // Need to touch something in HttpWebRequestActivitySource/Sdk to do the static injection. - GC.KeepAlive(HttpWebRequestActivitySource.Options); - _ = Sdk.SuppressInstrumentation; - } - - public HttpWebRequestActivitySourceTestsDupe() - { - Assert.Null(Activity.Current); - Activity.DefaultIdFormat = ActivityIdFormat.W3C; - Activity.ForceDefaultIdFormat = false; - - this.testServer = TestHttpServer.RunServer( - ctx => ProcessServerRequest(ctx), - out this.testServerHost, - out this.testServerPort); - - this.hostNameAndPort = $"{this.testServerHost}:{this.testServerPort}"; - this.netPeerName = this.testServerHost; - this.netPeerPort = this.testServerPort; - - void ProcessServerRequest(HttpListenerContext context) - { - string redirects = context.Request.QueryString["redirects"]; - if (!string.IsNullOrWhiteSpace(redirects) && int.TryParse(redirects, out int parsedRedirects) && parsedRedirects > 0) - { - context.Response.Redirect(this.BuildRequestUrl(queryString: $"redirects={--parsedRedirects}")); - context.Response.OutputStream.Close(); - return; - } - - string responseContent; - if (context.Request.QueryString["skipRequestContent"] == null) - { - using StreamReader readStream = new StreamReader(context.Request.InputStream); - - responseContent = readStream.ReadToEnd(); - } - else - { - responseContent = $"{{\"Id\":\"{Guid.NewGuid()}\"}}"; - } - - string responseCode = context.Request.QueryString["responseCode"]; - if (!string.IsNullOrWhiteSpace(responseCode)) - { - context.Response.StatusCode = int.Parse(responseCode); - } - else - { - context.Response.StatusCode = 200; - } - - if (context.Response.StatusCode != 204) - { - using StreamWriter writeStream = new StreamWriter(context.Response.OutputStream); - - writeStream.Write(responseContent); - } - else - { - context.Response.OutputStream.Close(); - } - } - } - - public void Dispose() - { - this.testServer.Dispose(); - } - - /// - /// Test to make sure we get both request and response events. - /// - [Theory] - [InlineData("GET")] - [InlineData("POST")] - [InlineData("POST", "skipRequestContent=1")] - public async Task TestBasicReceiveAndResponseEvents(string method, string queryString = null) - { - var url = this.BuildRequestUrl(queryString: queryString); - - using var eventRecords = new ActivitySourceRecorder(); - - // Send a random Http request to generate some events - using (var client = new HttpClient()) - { - (method == "GET" - ? await client.GetAsync(url).ConfigureAwait(false) - : await client.PostAsync(url, new StringContent("hello world")).ConfigureAwait(false)).Dispose(); - } - - // We should have exactly one Start and one Stop event - Assert.Equal(2, eventRecords.Records.Count); - Assert.Equal(1, eventRecords.Records.Count(rec => rec.Key == "Start")); - Assert.Equal(1, eventRecords.Records.Count(rec => rec.Key == "Stop")); - - // Check to make sure: The first record must be a request, the next record must be a response. - Activity activity = AssertFirstEventWasStart(eventRecords); - - VerifyActivityStartTags(this.netPeerName, this.netPeerPort, method, url, activity); - - Assert.True(eventRecords.Records.TryDequeue(out var stopEvent)); - Assert.Equal("Stop", stopEvent.Key); - - VerifyActivityStopTags(200, activity); - } - - private static Activity AssertFirstEventWasStart(ActivitySourceRecorder eventRecords) - { - Assert.True(eventRecords.Records.TryDequeue(out KeyValuePair startEvent)); - Assert.Equal("Start", startEvent.Key); - return startEvent.Value; - } - - private static void VerifyHeaders(HttpWebRequest startRequest) - { - var tracestate = startRequest.Headers["tracestate"]; - Assert.Equal("some=state", tracestate); - - var baggage = startRequest.Headers["baggage"]; - Assert.Equal("k=v", baggage); - - var traceparent = startRequest.Headers["traceparent"]; - Assert.NotNull(traceparent); - Assert.Matches("^[0-9a-f]{2}-[0-9a-f]{32}-[0-9a-f]{16}-[0-9a-f]{2}$", traceparent); - } - - private static void VerifyActivityStartTags(string netPeerName, int? netPeerPort, string method, string url, Activity activity) - { - // New - Assert.NotNull(activity.TagObjects); - Assert.Equal(method, activity.GetTagValue(SemanticConventions.AttributeHttpRequestMethod)); - if (netPeerPort != null) - { - Assert.Equal(netPeerPort, activity.GetTagValue(SemanticConventions.AttributeServerPort)); - } - - Assert.Equal(netPeerName, activity.GetTagValue(SemanticConventions.AttributeServerAddress)); - - Assert.Equal(url, activity.GetTagValue(SemanticConventions.AttributeUrlFull)); - - // Old - Assert.NotNull(activity.TagObjects); - Assert.Equal(method, activity.GetTagValue(SemanticConventions.AttributeHttpMethod)); - if (netPeerPort != null) - { - Assert.Equal(netPeerPort, activity.GetTagValue(SemanticConventions.AttributeNetPeerPort)); - } - - Assert.Equal(netPeerName, activity.GetTagValue(SemanticConventions.AttributeNetPeerName)); - - Assert.Equal(url, activity.GetTagValue(SemanticConventions.AttributeHttpUrl)); - } - - private static void VerifyActivityStopTags(int statusCode, Activity activity) - { - // New - Assert.Equal(statusCode, activity.GetTagValue(SemanticConventions.AttributeHttpResponseStatusCode)); - - // Old - Assert.Equal(statusCode, activity.GetTagValue(SemanticConventions.AttributeHttpStatusCode)); - } - - private static void ActivityEnrichment(Activity activity, string method, object obj) - { - switch (method) - { - case "OnStartActivity": - Assert.True(obj is HttpWebRequest); - VerifyHeaders(obj as HttpWebRequest); - break; - - case "OnStopActivity": - Assert.True(obj is HttpWebResponse); - break; - - case "OnException": - Assert.True(obj is Exception); - break; - - default: - break; - } - } - - private static void ValidateBaggage(HttpWebRequest request) - { - string[] baggage = request.Headers["baggage"].Split(','); - - Assert.Equal(3, baggage.Length); - Assert.Contains("key=value", baggage); - Assert.Contains("bad%2Fkey=value", baggage); - Assert.Contains("goodkey=bad%2Fvalue", baggage); - } - - private string BuildRequestUrl(bool useHttps = false, string path = "echo", string queryString = null) - { - return $"{(useHttps ? "https" : "http")}://{this.testServerHost}:{this.testServerPort}/{path}{(string.IsNullOrWhiteSpace(queryString) ? string.Empty : $"?{queryString}")}"; - } - - private void CleanUpActivity() - { - while (Activity.Current != null) - { - Activity.Current.Stop(); - } - } - - /// - /// is a helper class for recording events. - /// - private class ActivitySourceRecorder : IDisposable - { - private readonly Action> onEvent; - private readonly ActivityListener activityListener; - - public ActivitySourceRecorder(Action> onEvent = null, ActivitySamplingResult activitySamplingResult = ActivitySamplingResult.AllDataAndRecorded) - { - this.activityListener = new ActivityListener - { - ShouldListenTo = (activitySource) => activitySource.Name == HttpWebRequestActivitySource.ActivitySourceName, - ActivityStarted = this.ActivityStarted, - ActivityStopped = this.ActivityStopped, - Sample = (ref ActivityCreationOptions options) => activitySamplingResult, - }; - - ActivitySource.AddActivityListener(this.activityListener); - - this.onEvent = onEvent; - } - - public ConcurrentQueue> Records { get; } = new ConcurrentQueue>(); - - public void Dispose() - { - this.activityListener.Dispose(); - } - - public void ActivityStarted(Activity activity) => this.Record("Start", activity); - - public void ActivityStopped(Activity activity) => this.Record("Stop", activity); - - private void Record(string eventName, Activity activity) - { - var record = new KeyValuePair(eventName, activity); - - this.Records.Enqueue(record); - this.onEvent?.Invoke(record); - } - } -} -#endif diff --git a/test/OpenTelemetry.Instrumentation.Http.Tests/HttpWebRequestActivitySourceTestsNew.netfx.cs b/test/OpenTelemetry.Instrumentation.Http.Tests/HttpWebRequestActivitySourceTestsNew.netfx.cs deleted file mode 100644 index 48661f69100..00000000000 --- a/test/OpenTelemetry.Instrumentation.Http.Tests/HttpWebRequestActivitySourceTestsNew.netfx.cs +++ /dev/null @@ -1,295 +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. -// - -#if NETFRAMEWORK -using System.Collections.Concurrent; -using System.Diagnostics; -using System.Net; -using System.Net.Http; -using Microsoft.Extensions.Configuration; -using OpenTelemetry.Instrumentation.Http.Implementation; -using OpenTelemetry.Tests; -using OpenTelemetry.Trace; -using Xunit; -using static OpenTelemetry.Internal.HttpSemanticConventionHelper; - -namespace OpenTelemetry.Instrumentation.Http.Tests; - -// Tests for v1.21.0 Semantic Conventions for Http spans -// see the spec https://github.com/open-telemetry/semantic-conventions/blob/v1.21.0/docs/http/http-spans.md -// These tests emit the new attributes. -// This test class can replace the other class when this library is GA. -public class HttpWebRequestActivitySourceTestsNew : IDisposable -{ - private readonly IDisposable testServer; - private readonly string testServerHost; - private readonly int testServerPort; - private readonly string hostNameAndPort; - private readonly string netPeerName; - private readonly int netPeerPort; - - static HttpWebRequestActivitySourceTestsNew() - { - var configuration = new ConfigurationBuilder() - .AddInMemoryCollection(new Dictionary { [SemanticConventionOptInKeyName] = "http" }) - .Build(); - - HttpClientInstrumentationOptions options = new(configuration) - { - EnrichWithHttpWebRequest = (activity, httpWebRequest) => - { - VerifyHeaders(httpWebRequest); - }, - }; - - HttpWebRequestActivitySource.Options = options; - - // Need to touch something in HttpWebRequestActivitySource/Sdk to do the static injection. - GC.KeepAlive(HttpWebRequestActivitySource.Options); - _ = Sdk.SuppressInstrumentation; - } - - public HttpWebRequestActivitySourceTestsNew() - { - Assert.Null(Activity.Current); - Activity.DefaultIdFormat = ActivityIdFormat.W3C; - Activity.ForceDefaultIdFormat = false; - - this.testServer = TestHttpServer.RunServer( - ctx => ProcessServerRequest(ctx), - out this.testServerHost, - out this.testServerPort); - - this.hostNameAndPort = $"{this.testServerHost}:{this.testServerPort}"; - this.netPeerName = this.testServerHost; - this.netPeerPort = this.testServerPort; - - void ProcessServerRequest(HttpListenerContext context) - { - string redirects = context.Request.QueryString["redirects"]; - if (!string.IsNullOrWhiteSpace(redirects) && int.TryParse(redirects, out int parsedRedirects) && parsedRedirects > 0) - { - context.Response.Redirect(this.BuildRequestUrl(queryString: $"redirects={--parsedRedirects}")); - context.Response.OutputStream.Close(); - return; - } - - string responseContent; - if (context.Request.QueryString["skipRequestContent"] == null) - { - using StreamReader readStream = new StreamReader(context.Request.InputStream); - - responseContent = readStream.ReadToEnd(); - } - else - { - responseContent = $"{{\"Id\":\"{Guid.NewGuid()}\"}}"; - } - - string responseCode = context.Request.QueryString["responseCode"]; - if (!string.IsNullOrWhiteSpace(responseCode)) - { - context.Response.StatusCode = int.Parse(responseCode); - } - else - { - context.Response.StatusCode = 200; - } - - if (context.Response.StatusCode != 204) - { - using StreamWriter writeStream = new StreamWriter(context.Response.OutputStream); - - writeStream.Write(responseContent); - } - else - { - context.Response.OutputStream.Close(); - } - } - } - - public void Dispose() - { - this.testServer.Dispose(); - } - - /// - /// Test to make sure we get both request and response events. - /// - [Theory] - [InlineData("GET")] - [InlineData("POST")] - [InlineData("POST", "skipRequestContent=1")] - public async Task TestBasicReceiveAndResponseEvents(string method, string queryString = null) - { - var url = this.BuildRequestUrl(queryString: queryString); - - using var eventRecords = new ActivitySourceRecorder(); - - // Send a random Http request to generate some events - using (var client = new HttpClient()) - { - (method == "GET" - ? await client.GetAsync(url).ConfigureAwait(false) - : await client.PostAsync(url, new StringContent("hello world")).ConfigureAwait(false)).Dispose(); - } - - // We should have exactly one Start and one Stop event - Assert.Equal(2, eventRecords.Records.Count); - Assert.Equal(1, eventRecords.Records.Count(rec => rec.Key == "Start")); - Assert.Equal(1, eventRecords.Records.Count(rec => rec.Key == "Stop")); - - // Check to make sure: The first record must be a request, the next record must be a response. - Activity activity = AssertFirstEventWasStart(eventRecords); - - VerifyActivityStartTags(this.netPeerName, this.netPeerPort, method, url, activity); - - Assert.True(eventRecords.Records.TryDequeue(out var stopEvent)); - Assert.Equal("Stop", stopEvent.Key); - - VerifyActivityStopTags(200, activity); - } - - private static Activity AssertFirstEventWasStart(ActivitySourceRecorder eventRecords) - { - Assert.True(eventRecords.Records.TryDequeue(out KeyValuePair startEvent)); - Assert.Equal("Start", startEvent.Key); - return startEvent.Value; - } - - private static void VerifyHeaders(HttpWebRequest startRequest) - { - var tracestate = startRequest.Headers["tracestate"]; - Assert.Equal("some=state", tracestate); - - var baggage = startRequest.Headers["baggage"]; - Assert.Equal("k=v", baggage); - - var traceparent = startRequest.Headers["traceparent"]; - Assert.NotNull(traceparent); - Assert.Matches("^[0-9a-f]{2}-[0-9a-f]{32}-[0-9a-f]{16}-[0-9a-f]{2}$", traceparent); - } - - private static void VerifyActivityStartTags(string netPeerName, int? netPeerPort, string method, string url, Activity activity) - { - Assert.NotNull(activity.TagObjects); - Assert.Equal(method, activity.GetTagValue(SemanticConventions.AttributeHttpRequestMethod)); - if (netPeerPort != null) - { - Assert.Equal(netPeerPort, activity.GetTagValue(SemanticConventions.AttributeServerPort)); - } - - Assert.Equal(netPeerName, activity.GetTagValue(SemanticConventions.AttributeServerAddress)); - - Assert.Equal(url, activity.GetTagValue(SemanticConventions.AttributeUrlFull)); - } - - private static void VerifyActivityStopTags(int statusCode, Activity activity) - { - Assert.Equal(statusCode, activity.GetTagValue(SemanticConventions.AttributeHttpResponseStatusCode)); - } - - private static void ActivityEnrichment(Activity activity, string method, object obj) - { - switch (method) - { - case "OnStartActivity": - Assert.True(obj is HttpWebRequest); - VerifyHeaders(obj as HttpWebRequest); - - break; - - case "OnStopActivity": - Assert.True(obj is HttpWebResponse); - break; - - case "OnException": - Assert.True(obj is Exception); - break; - - default: - break; - } - } - - private static void ValidateBaggage(HttpWebRequest request) - { - string[] baggage = request.Headers["baggage"].Split(','); - - Assert.Equal(3, baggage.Length); - Assert.Contains("key=value", baggage); - Assert.Contains("bad%2Fkey=value", baggage); - Assert.Contains("goodkey=bad%2Fvalue", baggage); - } - - private string BuildRequestUrl(bool useHttps = false, string path = "echo", string queryString = null) - { - return $"{(useHttps ? "https" : "http")}://{this.testServerHost}:{this.testServerPort}/{path}{(string.IsNullOrWhiteSpace(queryString) ? string.Empty : $"?{queryString}")}"; - } - - private void CleanUpActivity() - { - while (Activity.Current != null) - { - Activity.Current.Stop(); - } - } - - /// - /// is a helper class for recording events. - /// - private class ActivitySourceRecorder : IDisposable - { - private readonly Action> onEvent; - private readonly ActivityListener activityListener; - - public ActivitySourceRecorder(Action> onEvent = null, ActivitySamplingResult activitySamplingResult = ActivitySamplingResult.AllDataAndRecorded) - { - this.activityListener = new ActivityListener - { - ShouldListenTo = (activitySource) => activitySource.Name == HttpWebRequestActivitySource.ActivitySourceName, - ActivityStarted = this.ActivityStarted, - ActivityStopped = this.ActivityStopped, - Sample = (ref ActivityCreationOptions options) => activitySamplingResult, - }; - - ActivitySource.AddActivityListener(this.activityListener); - - this.onEvent = onEvent; - } - - public ConcurrentQueue> Records { get; } = new ConcurrentQueue>(); - - public void Dispose() - { - this.activityListener.Dispose(); - } - - public void ActivityStarted(Activity activity) => this.Record("Start", activity); - - public void ActivityStopped(Activity activity) => this.Record("Stop", activity); - - private void Record(string eventName, Activity activity) - { - var record = new KeyValuePair(eventName, activity); - - this.Records.Enqueue(record); - this.onEvent?.Invoke(record); - } - } -} -#endif diff --git a/test/OpenTelemetry.Tests/Shared/TestHttpServer.cs b/test/OpenTelemetry.Tests/Shared/TestHttpServer.cs index e54f4a0a957..b59062f1042 100644 --- a/test/OpenTelemetry.Tests/Shared/TestHttpServer.cs +++ b/test/OpenTelemetry.Tests/Shared/TestHttpServer.cs @@ -39,10 +39,17 @@ public static IDisposable RunServer(Action action, out stri } catch (HttpListenerException) { + server?.Dispose(); + server = null; retryCount--; } } + if (server == null) + { + throw new InvalidOperationException("Server could not be started."); + } + return server; }