Copilot commented on code in PR #13273:
URL: https://github.com/apache/trafficserver/pull/13273#discussion_r3416333417
##########
src/proxy/logging/LogObject.cc:
##########
@@ -638,6 +638,10 @@ LogObject::log(LogAccess *lad, std::string_view text_entry)
bytes_needed = m_format->field_count() * INK_MIN_ALIGN;
} else if (lad) {
bytes_needed = m_format->m_field_list.marshal_len(lad);
+ // Drives the logging component's per-field marshal CPU: bytes_needed
(from marshal_len) sums
+ // the exact per-field serialized length the marshal pass then writes,
capturing both field
+ // count and per-field/URL size that the per-record count cannot.
+ Metrics::Counter::increment(log_rsb.marshalled_bytes, bytes_needed);
} else if (!text_entry.empty()) {
bytes_needed = INK_ALIGN_DEFAULT(text_entry.size() + 1); // must include
null terminator.
}
Review Comment:
`marshalled_bytes` is only incremented in the non-aggregate `lad` path. For
aggregate log formats, `bytes_needed` is also computed (and the entry is
marshalled/written via `marshal_agg()`), but the counter never increments,
which will undercount marshalling work for aggregate formats. Consider
incrementing the counter for any `lad`-based entry after `bytes_needed` is
determined, rather than only in the `else if (lad)` branch.
--
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]
