Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[release/5.0-rc2] Implement SocketsHttpHandler.ConnectCallback #42075

Merged
merged 10 commits into from
Sep 10, 2020

Conversation

github-actions[bot]
Copy link
Contributor

@github-actions github-actions bot commented Sep 10, 2020

Backport of #41955 to release/5.0-rc2
Fixes #41949

/cc @geoffkizer

Customer Impact

We removed System.Net.Connections from 5.0. This resulted in also removing the ConnectionFactory property on SocketsHttpHandler, which allowed you to provide your own connection that SocketsHttpHandler would use instead of simply doing a standard TCP connection to the specified server.

We have heard from several customers, particularly Bing and gRPC, that the loss of this functionality impacts scenarios they want to enable on top of 5.0. In particular, Bing said that this functionality is "the #1 feature in .NET 5 that will solve actual business problems for us”.

The ConnectCallback property is a replacement for ConnectionFactory that provides similar capabilities without relying on the System.Net.Connections code.

In particular, it allows you to provide your own transport (e.g. Unix domain sockets, or in-memory pipes, etc), or augment a standard TCP connection with your own logic (e.g. bandwidth monitoring, wrapping with your own prefix etc), or configure a standard TCP connection (e.g. bind to a specific local IP address, or enable TCP keepalive, etc).

Testing

We have validated with Bing and gRPC that the ConnectCallback property provides the functionality they need to enable their scenarios.

We've added tests that validate the feature, including modifying the old ConnectionFactory tests to use ConnectCallback as well as additional tests.

Risk

The risk here is relatively low. We have some experience with providing this hook from the previous ConnectionFactory property. The new property is implemented in a similar way, just using a delegate instead of the Connections infrastructure.

@Dotnet-GitSync-Bot
Copy link
Collaborator

Note regarding the new-api-needs-documentation label:

This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, to please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change.

@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI new-api-needs-documentation labels Sep 10, 2020
@karelz karelz added area-System.Net.Http and removed area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI labels Sep 10, 2020
@ghost
Copy link

ghost commented Sep 10, 2020

Tagging subscribers to this area: @dotnet/ncl
See info in area-owners.md if you want to be subscribed.

@karelz karelz added this to the 5.0.0 milestone Sep 10, 2020
@geoffkizer
Copy link
Contributor

/azp run runtime-libraries-coreclr outerloop

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@Pilchie Pilchie added the Servicing-consider Issue for next servicing release review label Sep 10, 2020
@leecow leecow added Servicing-approved Approved for servicing release and removed Servicing-consider Issue for next servicing release review labels Sep 10, 2020
@geoffkizer geoffkizer merged commit bb1257b into release/5.0-rc2 Sep 10, 2020
@jkotas jkotas deleted the backport/pr-41955-to-release/5.0-rc2 branch September 11, 2020 21:24
@ghost ghost locked as resolved and limited conversation to collaborators Dec 7, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants