Copilot commented on code in PR #13282:
URL: https://github.com/apache/trafficserver/pull/13282#discussion_r3421659028
##########
doc/admin-guide/monitoring/statistics/core/network-io.en.rst:
##########
@@ -79,10 +79,18 @@ Network I/O
:type: counter
:units: bytes
+ Application-layer bytes read from client and origin connections. For TLS
+ connections this is the decrypted payload, symmetric with ``write_bytes``;
it
+ does not include TLS handshake or record-layer framing.
Review Comment:
This PR changes the meaning of `proxy.process.net.read_bytes_count` as well
(it’s now incremented alongside decrypted `read_bytes`, not raw socket reads).
The docs updated here only describe `read_bytes`/`write_bytes`; please also
update the documentation entry for `read_bytes_count` (if present in this
document set) to match the new TLS semantics.
##########
src/iocore/net/SSLNetVConnection.cc:
##########
@@ -287,6 +287,9 @@ SSLNetVConnection::_ssl_read_from_net(int64_t &ret)
Dbg(dbg_ctl_ssl, "bytes_read=%" PRId64, bytes_read);
s->vio.ndone += bytes_read;
+ // Decrypted application bytes, to match write_bytes (also plaintext for
TLS).
+ Metrics::Counter::increment(net_rsb.read_bytes, bytes_read);
+ Metrics::Counter::increment(net_rsb.read_bytes_count);
Review Comment:
Consider explicitly guarding (or asserting) that `bytes_read > 0` before
incrementing counters. If `bytes_read` can ever be `0` (clean shutdown) or
negative (error paths) in this block due to future refactors, this would either
skew `read_bytes_count` or potentially underflow/record an unintended value
depending on `increment`'s parameter type.
--
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]
