http3: add ConnContext to the server

[rthellend]

ConnContext can be used to modify the context used by a new http Request. This is equivalent to ConnContext in http.Server, but with a quic.Connection instead of a net.Conn.

// ConnContext optionally specifies a function that modifies
// the context used for a new connection c. The provided ctx
// has a ServerContextKey value.
ConnContext func(ctx context.Context, c quic.Connection) context.Context

[google-cla]

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

[rthellend]

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

This check is ✔️ now

[marten-seemann]

Have you seen the discussion in #3603? Is this PR attempting to resolve that issue? If so, can you explain how your PR resolves the problems raised there?

[codecov]

Codecov Report

Attention: 2 lines in your changes are missing coverage. Please review.

Comparison is base (1083d1f) 84.15% compared to head (883e0ef) 84.15%.
Report is 3 commits behind head on master.

Files	Patch %	Lines
http3/server.go	50.00%	1 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #4230      +/-   ##
==========================================
- Coverage   84.15%   84.15%   -0.00%     
==========================================
  Files         149      149              
  Lines       15386    15378       -8     
==========================================
- Hits        12948    12941       -7     
+ Misses       1941     1940       -1     
  Partials      497      497              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[rthellend]

Have you seen the discussion in #3603? Is this PR attempting to resolve that issue? If so, can you explain how your PR resolves the problems raised there?

I had not actually seen that discussion. Thanks for pointing it out. I would generally agree with the motivation behind it.

The way I use ConnContext is pretty straight forward. I wrap the underlying connection (tls.Conn or quic.Connection here) in my own struct so that I can keep track of metrics and some annotations for each connection. The http handlers need access to some of that data to do what they need to do. (Happy to provide more details if you're interested)

Here's a concrete example the wrapper for net.Conn: https://github.com/c2FmZQ/tlsproxy/blob/main/proxy/internal/netw/netw.go#L82

I have something similar for quic.Connection.

With http.Server, I have:

    ConnContext: func(ctx context.Context, c net.Conn) context.Context {
        return context.WithValue(ctx, connCtxKey, c)
     },

With http3.Server, I would have:

    ConnContext: func(ctx context.Context, c quic.Connection) context.Context {
        return context.WithValue(ctx, connCtxKey, c)
    },

Then, when the handler needs to access the wrapper's data, it does something like:

if wrappedConn, ok := req.Context().Value(connCtxKey).(*netw.Conn); ok {
  // use wrappedConn
}

The alternative I've been using so far is to create a new http3.Server for each connection and then call ServeQUICConn(conn):

    conn := ...

    serv := &http3.Server{
        Handler: http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
            req = req.WithContext(context.WithValue(req.Context(), connCtxKey, conn))
            realHandler.ServeHTTP(w, req)
         }),
    }
    serv.ServeQUICConn(conn);

It works, but it is not very elegant.

[marten-seemann]

Could you also add an integration test for this in integrationtests/self/http_test.go? The test should also assert that this statement remains true:
The provided ctx has a ServerContextKey value.

[rthellend]

Could you also add an integration test for this in integrationtests/self/http_test.go? The test should also assert that this statement remains true: The provided ctx has a ServerContextKey value.

done

[marten-seemann]

Thank you @rthellend! This looks good to me now.

I'll keep the PR open for a few more days to give @omz13, @carljkempe and @mattrobenolt a chance to chime in.

This will definitely make it into the next release, I just added it to the milestone.

[omz13]

@marten-seemann What happened to waiting a few days?

A quick look and it looks ok. 👍

Now its in master I guess I have something to play with 🎉

[mattrobenolt]

I might be misunderstanding the implementation here, but I just started poking around, and one thing different, is this ConnContext gets called per-http3 request, not per-QUIC connection, like I'd anticipate.

In TCP based http handler, we get one ConnContext called the first time the TCP stream is connected, and that is called once for the duration of the TCP connection.

From my testing now with quic-go, it seems every single HTTP3 request triggers the ConnContext, which is not what I'd anticipate.

This also might be more my misunderstandings of how QUIC works or some implementation detail, just wanted to note the behavior differs than anticipated.

Without a ton of understanding right now, the RemoteAddr() on the quic.Connection even reflects the same remote port, so it seems to suggest it's the same QUIC stream. But again, might be my lack of understanding of QUIC here.

I've tested using curl with the --http3 flag built with quiche, as well as a quic-go http3.RoundTripper client.

[marten-seemann]

@nanokatze Any thoughts on this?

I would've expected ConnContext to work as it does on HTTP/2, since it also uses stream multiplexing. @mattrobenolt I haven't dug into this yet. Maybe you could point us to the code in the standard library where HTTP/2 handles this?

[mattrobenolt]

@marten-seemann it doesn't happen in the http2 code path, it's purely in net/http right after Accepting a TCP connection.

https://cs.opensource.google/go/go/+/master:src/net/http/server.go;l=3257-3290?q=ConnContext&ss=go%2Fgo

So it is called as the first thing once a TCP connection is accepted, and this context is used for the lifetime until it disconnects.

For HTTP3/QUIC, I'd anticipate a similar behavior, once a QUIC connection is established, we'd share the context for the lifetime.

[marten-seemann]

Thank you @mattrobenolt! This makes sense to me. Unless anyone here disagrees, I'd say we should fix this as you suggest. I opened #4422 to track this.

Given that I'm working on a big refactor of the http3 package for #3522, I suggest holding off on a fix until that refactor has landed, otherwise we'll have deal with a ton of merge conflicts.

[mattrobenolt]

No worries. The implementation as-is replaces the hack I had in place which was also per-request and not per-connection. So swapping to using this should yield a free win once that's fixed to behave more correct. :) It's just some minor optimization for performance to do some computations and cache them for the lifetime of a connection when possible.

[rthellend]

The difference is that net/http.Server sets the context when a new TCP connection is received, whereas http3.Server sets it when a new quic stream is opened. I wouldn't expect the difference to be very significant.

[mattrobenolt]

This is where my knowledge of QUIC and details may differ.

So let me explain why I use this functionality and what my goal is.

A simple example that we leverage is binding a logger to the context with the RemoteAddr. Then everywhere down the chain spawning from that connection should have the same IP address for multiple requests sharing that same connection.

In structured logging sense, effectively doing a logger.With("remote_addr", conn.RemoteAddr().String())

I would expect one QUIC connection to have the same RemoteAddr which multiple streams multiplex over.

Or are my fundamental understandings here wrong?

[marten-seemann]

I would expect one QUIC connection to have the same RemoteAddr which multiple streams multiplex over.

Not necessarily. In principle, QUIC allows connection migration, which means that the client could migrate, for example from a WiFi interface to a cellular interface. This of course will change its IP address.

[mattrobenolt]

Right, so does that mean there is no start/end for a connection analagous to TCP? Is per-request the most general we can expect to get with QUIC?

[marten-seemann]

I wouldn't put it that way. You still have an underlying QUIC connection, it's just that the remote address is not necessarily stable. But to give a counter example, if you authenticate on that connection somehow (TLS client certs?), this would apply to all requests sent on this connection.

[mattrobenolt]

That's a good example, we don't do that, but I'd say similar vibes. I understand and would expect the remote addr to not be stable. But I would anticipate some signal for connection/start and end. There are other similar things we can do per-connection that are similar to auth, but we utilize some other caches and memory pooling on a per-connection basis when possible.

[marten-seemann]

I’m not sure how the ConnContext would help with that. If you want to be able to perform some cleanup at the end of the connection, you can use the ServeQUICConn API.

[mattrobenolt]

I'm not sure what you mean. Unless the reason is that QUIC behaves in a way that's different here than TCP.

With TCP I can guarantee a single connection is at minimum the same client.

By cleanup, in Go it's just GCing away. But binding stuff in this case to the Context using WithValue or whatever, and maintaining that same Context across multiple requests.

Are you suggesting that the QUIC APIs just fundamentally can't support this with ConnContext? If not, this frankly doesn't differ from the context already on a Request and not sure what additional value a ConnContext brings other than having a hook to get the quic.Connection.

I don't mind, to be clear, if this is a limitation. I'm just trying to understand the disconnect. As-is currently this doesn't mirror the behavior of the typical net/http ConnContext, and if the behavior is explicitly different it might just be worth documenting that difference.

[marten-seemann]

How does ConnContext allow you to find out when the underlying connection is closed?

[mattrobenolt]

It doesn't. Maybe I confused things by mentioning that. My intent is that we use this hook to do effectively connection-local caching of some expensive computation rather than per request.

In our case, we often have TCP connections lasting many hours to days with thousands to millions of requests per second over 1 connection.

So being able to reuse anything connection wide is beneficial.

Currently our workloads and traffic over QUIC and HTTP/3 is relatively low, so this optimization hasn't been too important yet.

And we rely on just normal Go GC to cleanup the connection-local caching we do when the connection is closed and the Context falls out of scope.

So we don't rely or expect this hook to handle connections closing. But as per the TCP hook, it's useful if we can establish one Context that's shared across every request over that same quic.Connection rather than a net.Conn.

I hope that helps explain.