Copilot commented on code in PR #13154:
URL: https://github.com/apache/trafficserver/pull/13154#discussion_r3220474361
##########
include/proxy/ProxyTransaction.h:
##########
@@ -146,18 +146,19 @@ class ProxyTransaction : public VConnection
void mark_as_tunnel_endpoint() override;
- /** Emit a best-effort access log entry for a request that failed before
- * HttpSM creation.
+ /** Emit a best-effort access log entry for a request without an HttpSM.
*
* Call this when a malformed request is rejected at the protocol layer
* (e.g. during HTTP/2 or HTTP/3 header decoding) and no HttpSM was
- * created. The method populates a PreTransactionLogData from the
+ * created. The method populates a NonHttpSmLogData from the
* session and the partially decoded request, then invokes Log::access.
+ * If an HttpSM exists, callers should use the normal transaction logging
+ * path instead.
*
* @param[in] request The decoded (possibly partial) request header.
* @param[in] protocol_str Protocol string for the log entry (e.g. "http/2").
*/
- void log_pre_transaction_access(HTTPHdr const *request, const char
*protocol_str);
+ void log_non_http_sm_access(HTTPHdr const *request, const char
*protocol_str);
Review Comment:
Renaming `log_pre_transaction_access` to `log_non_http_sm_access` in a
public header under `include/` is a source-breaking API change for any external
code calling this method. If this repository maintains API stability for
consumers/plugins, consider keeping the old method name as a deprecated inline
wrapper that forwards to `log_non_http_sm_access` (and likewise a deprecated
`PreTransactionLogData` alias/wrapper header) for at least one release cycle.
##########
src/proxy/logging/TransactionLogData.cc:
##########
@@ -22,7 +22,7 @@
*/
#include "proxy/logging/TransactionLogData.h"
-#include "proxy/PreTransactionLogData.h"
+#include "proxy/NonHttpSmLogData.h"
Review Comment:
This PR updates multiple translation units to include
`proxy/NonHttpSmLogData.h`, but the only header shown in the diff is still at
`include/proxy/PreTransactionLogData.h` (now containing `class
NonHttpSmLogData`). If `NonHttpSmLogData.h` is not actually added/renamed in
the build, these includes will fail to compile. Fix by renaming the header file
to `NonHttpSmLogData.h` (and updating build/install lists), or by introducing a
new `NonHttpSmLogData.h` header that declares the class and leaving
`PreTransactionLogData.h` as a compatibility wrapper.
--
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]
