cmcfarlen commented on code in PR #12860:
URL: https://github.com/apache/trafficserver/pull/12860#discussion_r2879100876
##########
src/proxy/http/HttpSM.cc:
##########
@@ -1881,21 +1881,39 @@ HttpSM::state_http_server_open(int event, void *data)
case CONNECT_EVENT_RETRY:
do_http_server_open();
break;
- case CONNECT_EVENT_TXN:
+ case CONNECT_EVENT_TXN: {
+ // Multiplexed connection path (e.g. HTTP/2): ConnectingEntry created the
session
+ // and dispatched this event. Get the netvc from the PoolableSession.
SMDbg(dbg_ctl_http, "Connection handshake complete via CONNECT_EVENT_TXN");
- if (this->create_server_txn(static_cast<PoolableSession *>(data))) {
+ PoolableSession *session = static_cast<PoolableSession *>(data);
+ ink_assert(session != nullptr);
+ // Capture server TLS handshake timing from the session's netvc
+ if (auto *netvc = session->get_netvc()) {
+ if (auto tbs = netvc->get_service<TLSBasicSupport>()) {
+ milestones[TS_MILESTONE_SERVER_TLS_HANDSHAKE_START] =
tbs->get_tls_handshake_begin_time();
+ milestones[TS_MILESTONE_SERVER_TLS_HANDSHAKE_END] =
tbs->get_tls_handshake_end_time();
Review Comment:
This sets both milestones after the end time is known. Should we set the
start milestone when it starts vs here? As it is, if this code isn't reached,
the start milestone won't be set.
##########
src/proxy/http/HttpSM.cc:
##########
@@ -1881,21 +1881,39 @@ HttpSM::state_http_server_open(int event, void *data)
case CONNECT_EVENT_RETRY:
do_http_server_open();
break;
- case CONNECT_EVENT_TXN:
+ case CONNECT_EVENT_TXN: {
+ // Multiplexed connection path (e.g. HTTP/2): ConnectingEntry created the
session
+ // and dispatched this event. Get the netvc from the PoolableSession.
SMDbg(dbg_ctl_http, "Connection handshake complete via CONNECT_EVENT_TXN");
- if (this->create_server_txn(static_cast<PoolableSession *>(data))) {
+ PoolableSession *session = static_cast<PoolableSession *>(data);
+ ink_assert(session != nullptr);
+ // Capture server TLS handshake timing from the session's netvc
+ if (auto *netvc = session->get_netvc()) {
+ if (auto tbs = netvc->get_service<TLSBasicSupport>()) {
+ milestones[TS_MILESTONE_SERVER_TLS_HANDSHAKE_START] =
tbs->get_tls_handshake_begin_time();
+ milestones[TS_MILESTONE_SERVER_TLS_HANDSHAKE_END] =
tbs->get_tls_handshake_end_time();
+ }
+ }
+ if (this->create_server_txn(session)) {
t_state.current.server->clear_connect_fail();
handle_http_server_open();
} else { // Failed to create transaction. Maybe too many active
transactions already
// Try again (probably need a bounding counter here)
do_http_server_open(false);
}
return 0;
+ }
case VC_EVENT_READ_COMPLETE:
case VC_EVENT_WRITE_READY:
case VC_EVENT_WRITE_COMPLETE:
- // Update the time out to the regular connection timeout.
+ // Direct connection path (e.g. HTTP/1): HttpSM owns the netvc directly
+ // and creates the session itself.
SMDbg(dbg_ctl_http_ss, "Connection handshake complete");
+ // Capture server TLS handshake timing if this is a TLS connection
+ if (auto tbs = _netvc->get_service<TLSBasicSupport>()) {
+ milestones[TS_MILESTONE_SERVER_TLS_HANDSHAKE_START] =
tbs->get_tls_handshake_begin_time();
+ milestones[TS_MILESTONE_SERVER_TLS_HANDSHAKE_END] =
tbs->get_tls_handshake_end_time();
+ }
Review Comment:
Why are these set again?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
