-
Notifications
You must be signed in to change notification settings - Fork 900
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
Collect timings related with TLS handshake and protocol upgrade #3647
Comments
I believe as a starting point for this issue, it is enough to
I think we can add fields to ClientConnectionTimingsBuilder like the following: private long tlsHandshakeStartTimeMicros;
private long tlsHandshakeStartNanos;
private long tlsHandshakeEndNanos;
private boolean tlsHandshakeEndSet; Afterwards, we can mark the above fields whenever a TLS handshake is started or completed. A simple walkthrough of the process of establishing the connection is as follows:
In order to pass around the static final AttributeKey<ClientConnectionTimingsBuilder> TIMINGS_BUILDER_KEY =
AttributeKey.valueOf(HttpChannelPool.class, "TIMINGS_BUILDER_KEY");
...
// set the ClientConnectionTimingsBuilder to the channel
channel.attr(TIMINGS_BUILDER_KEY).set(timingsBuilder);
// later retrieve the builder
final ClientConnectionTimingsBuilder timingsBuilder =
ctx.channel().attr(TIMINGS_BUILDER_KEY).get();
timingsBuilder.tlsHandshakeStart();
... Lastly, we will want to expose the new field at ClientConnectionTimings. Misc
Also, this means that TLS may be attempted two times. I think it's fine to just record the last attempt for now (So if two attempts are made, we just overwrite the first attempt). In terms of starting point, one can probably write a simple test which spins up a server, and a client makes a request via tls. @RegisterExtension
static ServerExtension server = new ServerExtension() {
@Override
protected void configure(ServerBuilder sb) throws Exception {
sb.https(0);
sb.tlsSelfSigned();
sb.service("/", (ctx, req) -> HttpResponse.of(200));
}
};
@Test
void testTlsSelfSigned() {
try (ClientFactory clientFactory =
ClientFactory.builder()
.tlsNoVerify()
.build()) {
final AggregatedHttpResponse res =
server.blockingWebClient(cb -> cb.factory(clientFactory))
.get("/");
assertThat(res.status().code()).isEqualTo(200);
}
} reserved for @Leewongi0731 |
Thanks. I'll try to resolve this issue!! |
Motivation: Collect the timings related with TLS handshake. If a request was the first in a connection, armeria could also provide it in a RequestLog to tell a user that the request timing has been affected by TLS handshake. Modifications: - Add TLS handshake related field in `ClientConnectionTimings` - Add TLS handshake duration metric field at MetricCollectingClient - Start collecting the TLS handshake timer in the case below. - ~~If the client is enabled as `TCP fast open` in the first request, start the timer before the TCP connection.~~ - start the timer when netty calls `SslHandler.channelActive()` Result: - Closes #3647
Motivation: Collect the timings related with TLS handshake. If a request was the first in a connection, armeria could also provide it in a RequestLog to tell a user that the request timing has been affected by TLS handshake. Modifications: - Add TLS handshake related field in `ClientConnectionTimings` - Add TLS handshake duration metric field at MetricCollectingClient - Start collecting the TLS handshake timer in the case below. - ~~If the client is enabled as `TCP fast open` in the first request, start the timer before the TCP connection.~~ - start the timer when netty calls `SslHandler.channelActive()` Result: - Closes line#3647
We should collect the timings related with TLS handshake - at least how long it took to finish TLS handshake. If a request was the first in a connection, we could also provide it in a
RequestLog
to tell a user that the request timing has been affected by TLS handshake.On top of this, we could also collect timings related with protocol upgrade:
These timings are common in both client and server, so we could take advantage of this to introduce some type hierarchy to
ClientConnectionTimings
:ClientConnectionTimings
extendsConnectionTimings
and provides only client-side timings;ConnectionTimings
provides TLS handshake and protocol upgrade timings; andRequestLog.connectionTimings()
that returns aConnectionTimings
which can be downcasted to aClientConnectionTimings
.The text was updated successfully, but these errors were encountered: