From 00f3632d67d1b78fc706e08f8befe2ae7588d4be Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Tue, 4 Apr 2023 12:03:55 -0700 Subject: [PATCH 1/4] Avoid boxing of status code tags. --- .../CHANGELOG.md | 7 +- .../HttpHandlerDiagnosticListener.cs | 2 +- .../HttpHandlerMetricsDiagnosticListener.cs | 2 +- .../HttpWebRequestActivitySource.netfx.cs | 2 +- .../Implementation/TelemetryHelper.cs | 111 ++++++++++++++++++ 5 files changed, 119 insertions(+), 5 deletions(-) create mode 100644 src/OpenTelemetry.Instrumentation.Http/Implementation/TelemetryHelper.cs diff --git a/src/OpenTelemetry.Instrumentation.Http/CHANGELOG.md b/src/OpenTelemetry.Instrumentation.Http/CHANGELOG.md index 715a06ff6dd..823ed09b068 100644 --- a/src/OpenTelemetry.Instrumentation.Http/CHANGELOG.md +++ b/src/OpenTelemetry.Instrumentation.Http/CHANGELOG.md @@ -3,8 +3,11 @@ ## Unreleased * Fixed an issue of missing `http.client.duration` metric data in case of -network failures (when response is not available). -([#4098](https://github.com/open-telemetry/opentelemetry-dotnet/pull/4098)) + network failures (when response is not available). + ([#4098](https://github.com/open-telemetry/opentelemetry-dotnet/pull/4098)) + +* Improve perf by avoiding boxing of common status codes values. + ([#XXXX](https://github.com/open-telemetry/opentelemetry-dotnet/pull/XXXX)) ## 1.0.0-rc9.14 diff --git a/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpHandlerDiagnosticListener.cs b/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpHandlerDiagnosticListener.cs index a60049eb939..fa10137333b 100644 --- a/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpHandlerDiagnosticListener.cs +++ b/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpHandlerDiagnosticListener.cs @@ -209,7 +209,7 @@ public void OnStopActivity(Activity activity, object payload) if (this.stopResponseFetcher.TryFetch(payload, out HttpResponseMessage response) && response != null) { - activity.SetTag(SemanticConventions.AttributeHttpStatusCode, (int)response.StatusCode); + activity.SetTag(SemanticConventions.AttributeHttpStatusCode, TelemetryHelper.GetBoxedStatusCode(response.StatusCode)); if (currentStatusCode == ActivityStatusCode.Unset) { diff --git a/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpHandlerMetricsDiagnosticListener.cs b/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpHandlerMetricsDiagnosticListener.cs index 8a533f342cf..4fc5a4f982e 100644 --- a/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpHandlerMetricsDiagnosticListener.cs +++ b/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpHandlerMetricsDiagnosticListener.cs @@ -62,7 +62,7 @@ public override void OnEventWritten(string name, object payload) if (this.stopResponseFetcher.TryFetch(payload, out HttpResponseMessage response) && response != null) { - tags.Add(new KeyValuePair(SemanticConventions.AttributeHttpStatusCode, (int)response.StatusCode)); + 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. diff --git a/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpWebRequestActivitySource.netfx.cs b/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpWebRequestActivitySource.netfx.cs index c1c20ac9218..7150ce8ddde 100644 --- a/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpWebRequestActivitySource.netfx.cs +++ b/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpWebRequestActivitySource.netfx.cs @@ -123,7 +123,7 @@ private static void AddResponseTags(HttpWebResponse response, Activity activity) { if (activity.IsAllDataRequested) { - activity.SetTag(SemanticConventions.AttributeHttpStatusCode, (int)response.StatusCode); + activity.SetTag(SemanticConventions.AttributeHttpStatusCode, TelemetryHelper.GetBoxedStatusCode(response.StatusCode)); activity.SetStatus(SpanHelper.ResolveSpanStatusForHttpStatusCode(activity.Kind, (int)response.StatusCode)); diff --git a/src/OpenTelemetry.Instrumentation.Http/Implementation/TelemetryHelper.cs b/src/OpenTelemetry.Instrumentation.Http/Implementation/TelemetryHelper.cs new file mode 100644 index 00000000000..3ba2f6fbd3b --- /dev/null +++ b/src/OpenTelemetry.Instrumentation.Http/Implementation/TelemetryHelper.cs @@ -0,0 +1,111 @@ +// +// 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.Net; + +namespace OpenTelemetry.Instrumentation.Http.Implementation; + +internal static class TelemetryHelper +{ +#pragma warning disable SA1509 // Opening braces should not be preceded by blank line + // Status Codes listed at http://www.iana.org/assignments/http-status-codes/http-status-codes.xhtml + private static readonly Dictionary BoxedStatusCodes = new() + { + { HttpStatusCode.Continue, 100 }, + { HttpStatusCode.SwitchingProtocols, 101 }, + /*{ 102, 102 },*/ + + { HttpStatusCode.OK, 200 }, + { HttpStatusCode.Created, 201 }, + { HttpStatusCode.Accepted, 202 }, + { HttpStatusCode.NonAuthoritativeInformation, 203 }, + { HttpStatusCode.NoContent, 204 }, + { HttpStatusCode.ResetContent, 205 }, + { HttpStatusCode.PartialContent, 206 }, + /*{ 207, 207 }, + { 208, 208 }, + { 226, 226 },*/ + + { HttpStatusCode.MultipleChoices, 300 }, + { HttpStatusCode.Ambiguous, 300 }, + { HttpStatusCode.MovedPermanently, 301 }, + { HttpStatusCode.Moved, 301 }, + { HttpStatusCode.Found, 302 }, + { HttpStatusCode.Redirect, 302 }, + { HttpStatusCode.SeeOther, 303 }, + { HttpStatusCode.RedirectMethod, 303 }, + { HttpStatusCode.NotModified, 304 }, + { HttpStatusCode.UseProxy, 305 }, + { HttpStatusCode.Unused, 306 }, + { HttpStatusCode.TemporaryRedirect, 307 }, + { HttpStatusCode.RedirectKeepVerb, 307 }, + /*{ 308, 308 },*/ + + { HttpStatusCode.BadRequest, 400 }, + { HttpStatusCode.Unauthorized, 401 }, + { HttpStatusCode.PaymentRequired, 402 }, + { HttpStatusCode.Forbidden, 403 }, + { HttpStatusCode.NotFound, 404 }, + { HttpStatusCode.MethodNotAllowed, 405 }, + { HttpStatusCode.NotAcceptable, 406 }, + { HttpStatusCode.ProxyAuthenticationRequired, 407 }, + { HttpStatusCode.RequestTimeout, 408 }, + { HttpStatusCode.Conflict, 409 }, + { HttpStatusCode.Gone, 410 }, + { HttpStatusCode.LengthRequired, 411 }, + { HttpStatusCode.PreconditionFailed, 412 }, + { HttpStatusCode.RequestEntityTooLarge, 413 }, + { HttpStatusCode.RequestUriTooLong, 414 }, + { HttpStatusCode.UnsupportedMediaType, 415 }, + { HttpStatusCode.RequestedRangeNotSatisfiable, 416 }, + { HttpStatusCode.ExpectationFailed, 417 }, + /*{ 418, 418 }, + { 419, 419 }, + { 421, 421 }, + { 422, 422 }, + { 423, 423 }, + { 424, 424 },*/ + { HttpStatusCode.UpgradeRequired, 426 }, + /*{ 428, 428 }, + { 429, 429 }, + { 431, 431 }, + { 451, 451 }, + { 499, 499 },*/ + + { HttpStatusCode.InternalServerError, 500 }, + { HttpStatusCode.NotImplemented, 501 }, + { HttpStatusCode.BadGateway, 502 }, + { HttpStatusCode.ServiceUnavailable, 503 }, + { HttpStatusCode.GatewayTimeout, 504 }, + { HttpStatusCode.HttpVersionNotSupported, 505 }, + /*{ 506, 506 }, + { 507, 507 }, + { 508, 508 }, + { 510, 510 }, + { 511, 511 },*/ + }; +#pragma warning restore SA1509 // Opening braces should not be preceded by blank line + + public static object GetBoxedStatusCode(HttpStatusCode statusCode) + { + if (BoxedStatusCodes.TryGetValue(statusCode, out var result)) + { + return result; + } + + return (int)statusCode; + } +} From 92ceb9aa13c2cda810bc2c777683be10fe65d6bc Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Tue, 4 Apr 2023 12:05:19 -0700 Subject: [PATCH 2/4] CHANGELOG patch. --- src/OpenTelemetry.Instrumentation.Http/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/OpenTelemetry.Instrumentation.Http/CHANGELOG.md b/src/OpenTelemetry.Instrumentation.Http/CHANGELOG.md index 823ed09b068..c9329f714e5 100644 --- a/src/OpenTelemetry.Instrumentation.Http/CHANGELOG.md +++ b/src/OpenTelemetry.Instrumentation.Http/CHANGELOG.md @@ -7,7 +7,7 @@ ([#4098](https://github.com/open-telemetry/opentelemetry-dotnet/pull/4098)) * Improve perf by avoiding boxing of common status codes values. - ([#XXXX](https://github.com/open-telemetry/opentelemetry-dotnet/pull/XXXX)) + ([#4361](https://github.com/open-telemetry/opentelemetry-dotnet/pull/4361)) ## 1.0.0-rc9.14 From dd3ab2a421136233ba670799f4c2b6a7c5bbacfe Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Tue, 4 Apr 2023 14:39:00 -0700 Subject: [PATCH 3/4] Remove duplicate mappings. --- .../Implementation/TelemetryHelper.cs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/OpenTelemetry.Instrumentation.Http/Implementation/TelemetryHelper.cs b/src/OpenTelemetry.Instrumentation.Http/Implementation/TelemetryHelper.cs index 3ba2f6fbd3b..de2d34713fb 100644 --- a/src/OpenTelemetry.Instrumentation.Http/Implementation/TelemetryHelper.cs +++ b/src/OpenTelemetry.Instrumentation.Http/Implementation/TelemetryHelper.cs @@ -40,18 +40,18 @@ internal static class TelemetryHelper { 226, 226 },*/ { HttpStatusCode.MultipleChoices, 300 }, - { HttpStatusCode.Ambiguous, 300 }, + /* { HttpStatusCode.Ambiguous, 300 }, */ { HttpStatusCode.MovedPermanently, 301 }, - { HttpStatusCode.Moved, 301 }, + /* { HttpStatusCode.Moved, 301 }, */ { HttpStatusCode.Found, 302 }, - { HttpStatusCode.Redirect, 302 }, + /* { HttpStatusCode.Redirect, 302 }, */ { HttpStatusCode.SeeOther, 303 }, - { HttpStatusCode.RedirectMethod, 303 }, + /* { HttpStatusCode.RedirectMethod, 303 }, */ { HttpStatusCode.NotModified, 304 }, { HttpStatusCode.UseProxy, 305 }, { HttpStatusCode.Unused, 306 }, { HttpStatusCode.TemporaryRedirect, 307 }, - { HttpStatusCode.RedirectKeepVerb, 307 }, + /* { HttpStatusCode.RedirectKeepVerb, 307 }, */ /*{ 308, 308 },*/ { HttpStatusCode.BadRequest, 400 }, From b4ba9dc19030aaf0e213fb0dec58ba8207baaf01 Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Tue, 4 Apr 2023 15:30:32 -0700 Subject: [PATCH 4/4] Array lookup for better perf. --- .../Implementation/TelemetryHelper.cs | 93 +++---------------- 1 file changed, 13 insertions(+), 80 deletions(-) diff --git a/src/OpenTelemetry.Instrumentation.Http/Implementation/TelemetryHelper.cs b/src/OpenTelemetry.Instrumentation.Http/Implementation/TelemetryHelper.cs index de2d34713fb..b0cdce4eb11 100644 --- a/src/OpenTelemetry.Instrumentation.Http/Implementation/TelemetryHelper.cs +++ b/src/OpenTelemetry.Instrumentation.Http/Implementation/TelemetryHelper.cs @@ -20,92 +20,25 @@ namespace OpenTelemetry.Instrumentation.Http.Implementation; internal static class TelemetryHelper { -#pragma warning disable SA1509 // Opening braces should not be preceded by blank line - // Status Codes listed at http://www.iana.org/assignments/http-status-codes/http-status-codes.xhtml - private static readonly Dictionary BoxedStatusCodes = new() - { - { HttpStatusCode.Continue, 100 }, - { HttpStatusCode.SwitchingProtocols, 101 }, - /*{ 102, 102 },*/ - - { HttpStatusCode.OK, 200 }, - { HttpStatusCode.Created, 201 }, - { HttpStatusCode.Accepted, 202 }, - { HttpStatusCode.NonAuthoritativeInformation, 203 }, - { HttpStatusCode.NoContent, 204 }, - { HttpStatusCode.ResetContent, 205 }, - { HttpStatusCode.PartialContent, 206 }, - /*{ 207, 207 }, - { 208, 208 }, - { 226, 226 },*/ - - { HttpStatusCode.MultipleChoices, 300 }, - /* { HttpStatusCode.Ambiguous, 300 }, */ - { HttpStatusCode.MovedPermanently, 301 }, - /* { HttpStatusCode.Moved, 301 }, */ - { HttpStatusCode.Found, 302 }, - /* { HttpStatusCode.Redirect, 302 }, */ - { HttpStatusCode.SeeOther, 303 }, - /* { HttpStatusCode.RedirectMethod, 303 }, */ - { HttpStatusCode.NotModified, 304 }, - { HttpStatusCode.UseProxy, 305 }, - { HttpStatusCode.Unused, 306 }, - { HttpStatusCode.TemporaryRedirect, 307 }, - /* { HttpStatusCode.RedirectKeepVerb, 307 }, */ - /*{ 308, 308 },*/ - - { HttpStatusCode.BadRequest, 400 }, - { HttpStatusCode.Unauthorized, 401 }, - { HttpStatusCode.PaymentRequired, 402 }, - { HttpStatusCode.Forbidden, 403 }, - { HttpStatusCode.NotFound, 404 }, - { HttpStatusCode.MethodNotAllowed, 405 }, - { HttpStatusCode.NotAcceptable, 406 }, - { HttpStatusCode.ProxyAuthenticationRequired, 407 }, - { HttpStatusCode.RequestTimeout, 408 }, - { HttpStatusCode.Conflict, 409 }, - { HttpStatusCode.Gone, 410 }, - { HttpStatusCode.LengthRequired, 411 }, - { HttpStatusCode.PreconditionFailed, 412 }, - { HttpStatusCode.RequestEntityTooLarge, 413 }, - { HttpStatusCode.RequestUriTooLong, 414 }, - { HttpStatusCode.UnsupportedMediaType, 415 }, - { HttpStatusCode.RequestedRangeNotSatisfiable, 416 }, - { HttpStatusCode.ExpectationFailed, 417 }, - /*{ 418, 418 }, - { 419, 419 }, - { 421, 421 }, - { 422, 422 }, - { 423, 423 }, - { 424, 424 },*/ - { HttpStatusCode.UpgradeRequired, 426 }, - /*{ 428, 428 }, - { 429, 429 }, - { 431, 431 }, - { 451, 451 }, - { 499, 499 },*/ + private static readonly object[] BoxedStatusCodes; - { HttpStatusCode.InternalServerError, 500 }, - { HttpStatusCode.NotImplemented, 501 }, - { HttpStatusCode.BadGateway, 502 }, - { HttpStatusCode.ServiceUnavailable, 503 }, - { HttpStatusCode.GatewayTimeout, 504 }, - { HttpStatusCode.HttpVersionNotSupported, 505 }, - /*{ 506, 506 }, - { 507, 507 }, - { 508, 508 }, - { 510, 510 }, - { 511, 511 },*/ - }; -#pragma warning restore SA1509 // Opening braces should not be preceded by blank line + static TelemetryHelper() + { + BoxedStatusCodes = new object[500]; + for (int i = 0, c = 100; i < BoxedStatusCodes.Length; i++, c++) + { + BoxedStatusCodes[i] = c; + } + } public static object GetBoxedStatusCode(HttpStatusCode statusCode) { - if (BoxedStatusCodes.TryGetValue(statusCode, out var result)) + int intStatusCode = (int)statusCode; + if (intStatusCode >= 100 && intStatusCode < 600) { - return result; + return BoxedStatusCodes[intStatusCode - 100]; } - return (int)statusCode; + return intStatusCode; } }