Implement IConnectionEndPointFeature in Kestrel, HttpSys, and IIS#62162
Conversation
Co-authored-by: BrennanConroy <7574801+BrennanConroy@users.noreply.github.com>
IConnectionEndPointFeature|
Looking good, can you add a test that verifies the feature is in the feature collection and has values? |
Co-authored-by: BrennanConroy <7574801+BrennanConroy@users.noreply.github.com>
Added comprehensive tests in TransportConnectionFeatureCollectionTests.cs that verify IConnectionEndPointFeature is available in the feature collection and properly returns/sets endpoint values. The tests follow existing patterns and validate both getting and setting LocalEndPoint and RemoteEndPoint through the feature interface. (dbd9f49) Warning Firewall rules blocked me from connecting to one or more addressesI tried to connect to the following addresses, but was blocked by firewall rules:If you need me to access, download, or install something from one of these locations, you can either:
|
|
Can you refactor https://github.com/dotnet/aspnetcore/blob/main/src/Servers/Kestrel/Core/src/Internal/Infrastructure/KestrelMetrics.cs#L322 to use the |
Co-authored-by: BrennanConroy <7574801+BrennanConroy@users.noreply.github.com>
Refactored the connection endpoint logic into a shared utility in src/Shared/ConnectionEndpointTags.cs that uses IConnectionEndPointFeature. Updated KestrelMetrics.cs to use the shared code and modified SignalR's DefaultHubDispatcher to include endpoint tags via the same shared utility. The SignalR server.address tag is now uncommented and working. (6c1244f) |
|
Looks like there is a test failure:
Wonder if Http3 connections implement the feature collection differently |
…ion for HTTP/3 Co-authored-by: BrennanConroy <7574801+BrennanConroy@users.noreply.github.com>
Fixed the HTTP/3 test failure by adding IConnectionEndPointFeature support to TransportMultiplexedConnection. HTTP/3 connections use MultiplexedConnectionContext instead of regular TransportConnection, so the feature wasn't available. Added feature configuration, updated generated code, and improved shared utility with fallback logic. (45fbce8) |
|
Looks like the test fix worked, thanks! Can you add a test for the SignalR part now? I imagine we might need to add the |
…ignalR endpoint tags test Co-authored-by: BrennanConroy <7574801+BrennanConroy@users.noreply.github.com>
Added IConnectionEndPointFeature implementation to HttpConnectionContext and a comprehensive test that verifies SignalR activities include endpoint tags. The test validates server.address, server.port, network.type, and network.transport tags are properly added to both connect and disconnect activities. (630a05c) |
Done in 93ed79f:
|
|
@copilot add a couple smoke tests for this in iis and httpsys. Looks like there might be a couple tests using |
Agent-Logs-Url: https://github.com/dotnet/aspnetcore/sessions/6555e55e-2ba8-4ee0-8af9-2a28cdf45b12 Co-authored-by: BrennanConroy <7574801+BrennanConroy@users.noreply.github.com>
Added smoke tests in 49c4fd8:
|
There was a problem hiding this comment.
Pull request overview
Adds production-server support for IConnectionEndPointFeature (Kestrel, HttpSys, IIS) and centralizes OTel connection endpoint tagging so SignalR and Kestrel metrics can attach consistent server.* / network.* tags.
Changes:
- Implement/enable
IConnectionEndPointFeaturein Kestrel transport connections, HttpSysRequestContext, and IISIISHttpContextfeature collections. - Introduce shared
ConnectionEndpointTagshelper and wire it into Kestrel metrics + SignalR activities. - Add coverage tests across Kestrel, HttpSys, IIS, and SignalR activity tagging.
Reviewed changes
Copilot reviewed 21 out of 21 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/SignalR/server/SignalR/test/Microsoft.AspNetCore.SignalR.Tests/HubConnectionHandlerTests.Activity.cs | Adds an activity-tagging test validating endpoint/network tags on hub activities. |
| src/SignalR/server/Core/src/Microsoft.AspNetCore.SignalR.Core.csproj | Links the new shared ConnectionEndpointTags.cs into SignalR Core. |
| src/SignalR/server/Core/src/Internal/DefaultHubDispatcher.cs | Extends StartActivity to add endpoint tags when a connection is available. |
| src/SignalR/common/Http.Connections/src/Microsoft.AspNetCore.Http.Connections.csproj | Links the new shared ConnectionEndpointTags.cs into Http.Connections. |
| src/SignalR/common/Http.Connections/src/Internal/HttpConnectionContext.cs | Registers IConnectionEndPointFeature on SignalR HttpConnectionContext. |
| src/Shared/ConnectionEndpointTags.cs | New shared utility to add server.address, server.port, network.type, network.transport tags. |
| src/Servers/Kestrel/tools/CodeGenerator/TransportMultiplexedConnectionFeatureCollection.cs | Updates codegen configuration to include IConnectionEndPointFeature for multiplexed connections. |
| src/Servers/Kestrel/tools/CodeGenerator/TransportConnectionFeatureCollection.cs | Updates codegen configuration to include IConnectionEndPointFeature for transport connections. |
| src/Servers/Kestrel/shared/TransportMultiplexedConnection.Generated.cs | Regenerated feature collection plumbing to support IConnectionEndPointFeature. |
| src/Servers/Kestrel/shared/TransportMultiplexedConnection.FeatureCollection.cs | Implements IConnectionEndPointFeature by delegating to LocalEndPoint/RemoteEndPoint. |
| src/Servers/Kestrel/shared/TransportConnection.Generated.cs | Regenerated feature collection plumbing to support IConnectionEndPointFeature. |
| src/Servers/Kestrel/shared/TransportConnection.FeatureCollection.cs | Implements IConnectionEndPointFeature by delegating to LocalEndPoint/RemoteEndPoint. |
| src/Servers/Kestrel/Core/test/TransportConnectionFeatureCollectionTests.cs | New tests validating presence and get/set behavior of IConnectionEndPointFeature. |
| src/Servers/Kestrel/Core/src/Microsoft.AspNetCore.Server.Kestrel.Core.csproj | Links the new shared ConnectionEndpointTags.cs into Kestrel Core. |
| src/Servers/Kestrel/Core/src/Internal/Infrastructure/KestrelMetrics.cs | Replaces inline endpoint-tag logic with the shared helper. |
| src/Servers/IIS/IIS/test/IIS.Tests/ConnectionEndPointFeatureTests.cs | Adds IIS functional smoke test for IConnectionEndPointFeature. |
| src/Servers/IIS/IIS/src/Core/IISHttpContext.Features.cs | Adds fast-path feature lookup plumbing for IConnectionEndPointFeature. |
| src/Servers/IIS/IIS/src/Core/IISHttpContext.FeatureCollection.cs | Implements IConnectionEndPointFeature on IIS via IHttpConnectionFeature values. |
| src/Servers/HttpSys/test/FunctionalTests/ConnectionEndPointFeatureTests.cs | Adds HttpSys functional smoke test for IConnectionEndPointFeature. |
| src/Servers/HttpSys/src/StandardFeatureCollection.cs | Registers IConnectionEndPointFeature in HttpSys standard feature lookup. |
| src/Servers/HttpSys/src/RequestProcessing/RequestContext.FeatureCollection.cs | Implements IConnectionEndPointFeature on RequestContext via IHttpConnectionFeature values. |
|
@copilot react to the feedback above. Also, the SignalR change doesn't look like it would work outside of the test. It never sets the RemoteEndpoint or LocalEndpoint. Maybe it should be grabbing the |
…, E2E test Agent-Logs-Url: https://github.com/dotnet/aspnetcore/sessions/70229d9d-6fe0-4d78-808d-3d63d8e637d6 Co-authored-by: BrennanConroy <7574801+BrennanConroy@users.noreply.github.com>
Addressed all feedback in 1358d26:
|
…her provides it Agent-Logs-Url: https://github.com/dotnet/aspnetcore/sessions/ab2d00c7-775a-4e62-90a6-24cf17db6af3 Co-authored-by: BrennanConroy <7574801+BrennanConroy@users.noreply.github.com>
Implements
IConnectionEndPointFeatureacross multiple server implementations (Kestrel, HttpSys, and IIS), refactors connection endpoint tagging into a shared utility, and extends the feature to SignalR viaHttpConnectionDispatcher.Description
IConnectionEndPointFeaturewas previously only implemented inDefaultConnectionContext, which is primarily used in tests. This PR makes the feature available in production connections across Kestrel, HttpSys, and IIS, and consolidates endpoint tag logic into a shared utility.Changes
TransportConnectionandTransportMultiplexedConnection— AddedIConnectionEndPointFeaturesupport by updating the code generation configuration, regenerating the feature collection code, and implementing interface methods that delegate to the existingLocalEndPointandRemoteEndPointproperties. This covers both regular TCP connections (HTTP/1.x, HTTP/2) and multiplexed QUIC connections (HTTP/3).HttpSys(RequestContext+StandardFeatureCollection) — AddedIConnectionEndPointFeatureto theRequestContextpartial class andStandardFeatureCollectionfeature lookup. The implementation constructsIPEndPointon demand from the existing lazy-initializedIHttpConnectionFeatureproperties (LocalIpAddress/LocalPortandRemoteIpAddress/RemotePort).IIS(IISHttpContext) — AddedIConnectionEndPointFeaturetoIISHttpContext, including the full fast-path feature lookup support inFeatures.cs(static type field, backing field,Initialize(),FastFeatureGet,FastFeatureSet, andFastEnumerable). The implementation also delegates to the existingIHttpConnectionFeatureproperties.src/Shared/ConnectionEndpointTags.cs(new shared file,Microsoft.AspNetCore.Sharednamespace) — Extracted the connection endpoint tag logic fromKestrelMetrics.csinto a shared internal utility usingIConnectionEndPointFeature. Exposes two overloads:AddConnectionEndpointTags(ref TagList, IFeatureCollection)— used by SignalR; records transport as TCP and does not attempt QUIC detectionAddConnectionEndpointTags(ref TagList, BaseConnectionContext)— used by Kestrel metrics; includes QUIC/TCP detection viaMultiplexedConnectionContextA private
AddEndpointTagshelper eliminates duplication between overloads. Tags follow OTel semantic conventions (server.address,server.portasint,network.type,network.transport).KestrelMetrics.cs— UpdatedInitializeConnectionTagsto delegate to the shared utility.DefaultHubDispatcher.cs— UpdatedStartActivityto include connection endpoint tags in SignalR hub method activities.HttpConnectionDispatcher— Added propagation ofIConnectionEndPointFeaturefrom the incomingHttpContextto the SignalR connection (alongside the existingIHttpConnectionFeaturepropagation), ensuring realLocalEndPoint/RemoteEndPointvalues from Kestrel/HttpSys/IIS are available for telemetry.HttpConnectionContextno longer implementsIConnectionEndPointFeatureitself — the real endpoint feature from the underlying server is forwarded instead.Tests:
TransportConnectionFeatureCollectionTests.cs— VerifiesIConnectionEndPointFeatureis present in the feature collection and correctly returns/setsLocalEndPointandRemoteEndPointfor bothTransportConnectionandTransportMultiplexedConnection.HubConnectionHandlerTests.Activity.cs— Verifies that SignalR hub activities includeserver.address,server.port,network.type, andnetwork.transporttags when endpoint information is available. Note:server.portis stored asintper OTel conventions and must be checked viaActivity.TagObjectsrather thanActivity.Tags.HttpSys/test/FunctionalTests/ConnectionEndPointFeatureTests.cs— Smoke test verifying thatIConnectionEndPointFeatureis available fromhttpContext.Featuresin an in-process HttpSys server and returns non-nullIPEndPointinstances for bothLocalEndPointandRemoteEndPoint.IIS/IIS/test/IIS.Tests/ConnectionEndPointFeatureTests.cs— Smoke test verifying thatIConnectionEndPointFeatureis available fromctx.Featuresin an in-process IIS test server and returns non-nullIPEndPointinstances for bothLocalEndPointandRemoteEndPoint.HubConnectionTests.Tracing.cs— E2E functional test (InvokeAsync_ServerActivityIncludesConnectionEndpointTags) using a real Kestrel-backed server that verifies server-side SignalR activities includeserver.address,server.port,network.type, andnetwork.transporttags across all hub protocol/transport/path combinations.Usage
Notes
LocalEndPoint/RemoteEndPointproperties continues to work.Activity.Tagsonly enumerates string-valued tags. Integer-valued tags such asserver.portare only accessible viaActivity.TagObjects.ConnectionEndpointTagsis placed in theMicrosoft.AspNetCore.Sharednamespace, consistent with other shared utilities insrc/Shared/.Fixes #43786