bneradt commented on a change in pull request #6645:
URL: https://github.com/apache/trafficserver/pull/6645#discussion_r413039761
##########
File path: plugins/experimental/traffic_dump/traffic_dump.cc
##########
@@ -597,9 +601,29 @@ global_ssn_handler(TSCont contp, TSEvent event, void
*edata)
return TS_SUCCESS;
}
case TS_EVENT_HTTP_SSN_START: {
- // Grab session id to do sampling
+ // Grab session id for logging against a global value rather than the local
+ // session_counter.
int64_t id = TSHttpSsnIdGet(ssnp);
- if (id % sample_pool_size != 0) {
+
+ // If the user has asked for SNI filtering, filter on that first because
+ // any sampling will apply just to that subset of connections that match
+ // that SNI.
+ if (!sni_filter.empty()) {
+ TSVConn ssn_vc = TSHttpSsnClientVConnGet(ssnp);
+ TSSslConnection ssl_conn = TSVConnSslConnectionGet(ssn_vc);
+ SSL *ssl_obj = (SSL *)ssl_conn;
+ if (ssl_obj == nullptr) {
+ TSDebug(PLUGIN_NAME, "global_ssn_handler(): Ignore non-HTTPS session
%" PRId64 "...", id);
+ break;
+ }
+ const std::string sni = SSL_get_servername(ssl_obj,
TLSEXT_NAMETYPE_host_name);
+ if (sni != sni_filter) {
+ TSDebug(PLUGIN_NAME, "global_ssn_handler(): Ignore HTTPS session with
non-filtered SNI: %s", sni.c_str());
+ break;
+ }
+ }
+ const auto this_session_count = session_counter++;
+ if (this_session_count % sample_pool_size != 0) {
Review comment:
Thanks for the thoughts. I think we should keep this local variable
though. This use of the local variable was actually only accidentally a
workaround to the bug concerning the (lack of) unique values for the global
session id. That is, I made this change before finding the bug with the global
session id. We need this local counter because, now that we may filter on SNI,
our sampling should be based on a local counter of sessions after the
application of the SNI filter rather than the global one that applies before
the filter (and potentially any other filters we may add in the future). That
is, if the sampling is set to 20, the user wants to dump 1/20 sessions matching
the given SNI, not 1/20 of the global session id that no longer directly
corresponds with the sessions after the filter.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
