Skip to content

[Hosting] Avoid allocations for OpenTelemetry#67440

Open
martincostello wants to merge 2 commits into
dotnet:mainfrom
martincostello:improve-otel-perf
Open

[Hosting] Avoid allocations for OpenTelemetry#67440
martincostello wants to merge 2 commits into
dotnet:mainfrom
martincostello:improve-otel-perf

Conversation

@martincostello

Copy link
Copy Markdown
Member

Avoid allocations for OpenTelemetry

Reduce allocations for traces generated for HTTP requests.

Description

I was investigating why I wasn't getting the improvements I would expect with ASP.NET Core 11 preview.5 and OpenTelemetry.Instrumentation.AspNetCore 1.16.0 when the Microsoft.AspNetCore.Hosting.SuppressActivityOpenTelemetryData AppContext switch is set to false with Claude, which identified a number of possible ways to avoid allocations when OpenTelemetry is being generated.

The changes in this PR are essentially all the possible ways to avoid some of the allocations, whether they are worth doing and/or should be done in the ways suggested are for the team to decide.

The display name cache is probably the least controversial. The port boxing one could potentially be slimmed down to not care about 5000/5001/8080 and user-configured ports, and just keep the last-seen value for anything not 80 or 443, or just drop that part entirely.

Specific changes:

  • Pre-box common/expected server.port values.
  • Cache activity display names for endpoints.

I've proposed some similar changes for the OTel instrumentation for ASP.NET Core itself as well: open-telemetry/opentelemetry-dotnet-contrib#4600

- Pre-box common/expected `server.port` values.
- Cache activity display names for endpoints.
@martincostello martincostello requested a review from halter73 as a code owner June 26, 2026 15:54
Copilot AI review requested due to automatic review settings June 26, 2026 15:54
@dotnet-policy-service dotnet-policy-service Bot added the community-contribution Indicates that the PR has been added by a community member label Jun 26, 2026
@dotnet-policy-service

Copy link
Copy Markdown
Contributor

Thanks for your PR, @martincostello. Someone from the team will get assigned to your PR shortly and we'll get it reviewed.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Reduces per-request allocations in the Hosting OpenTelemetry path by reusing boxed server.port values and caching Activity display names derived from HTTP method + route.

Changes:

  • Reuse boxed instances for common ports and add a configurable pre-boxing mechanism for server-bound ports.
  • Cache Activity.DisplayName strings for (method, route) to avoid per-request string construction.
  • Register/unregister pre-boxed server ports during GenericWebHostService start/stop to avoid cross-host accumulation.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
src/Hosting/Hosting/src/Internal/HostingTelemetryHelpers.cs Adds boxed-port reuse + registration APIs and introduces a display name cache for method/route pairs.
src/Hosting/Hosting/src/GenericHost/GenericWebHostService.cs Collects bound/configured ports on startup and registers them for boxing reuse; unregisters on stop.
Comment thread src/Hosting/Hosting/src/Internal/HostingTelemetryHelpers.cs Outdated
Comment thread src/Hosting/Hosting/src/Internal/HostingTelemetryHelpers.cs
- Use a `ConditionalWeakTable` to avoid keeping the display strings around forever.
- Update comment.
@martincostello martincostello added the area-hosting Includes Hosting label Jun 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-hosting Includes Hosting community-contribution Indicates that the PR has been added by a community member

3 participants