JosiahWI commented on code in PR #13286:
URL: https://github.com/apache/trafficserver/pull/13286#discussion_r3442722034
##########
plugins/header_rewrite/condition.h:
##########
@@ -49,6 +49,10 @@ class Condition : public Statement
bool
do_eval(const Resources &res)
{
+ // In do_eval, so AND/OR short-circuited conditions aren't counted.
Review Comment:
Good comment.
##########
plugins/header_rewrite/statement.cc:
##########
@@ -21,6 +21,24 @@
//
#include "statement.h"
+int hrw_stat_operators = TS_ERROR;
+int hrw_stat_conditions = TS_ERROR;
+
+void
+init_hrw_work_stats()
+{
+ // Plugin init is single-threaded, so the create-once guard is safe.
header_rewrite.so can
+ // be loaded both globally and as a remap plugin; both must share one
process-wide stat.
+ if (TS_ERROR == hrw_stat_operators) {
+ hrw_stat_operators =
TSStatCreate("proxy.process.plugin.header_rewrite.operators",
TS_RECORDDATATYPE_INT,
Review Comment:
Our API documentation says that `TsStatCreate` is deprecated since ATS 10,
and that we should use the Metrics.h APIs instead. I think we should heed that
if possible.
##########
plugins/header_rewrite/statement.h:
##########
@@ -32,6 +32,13 @@
#include "parser.h"
#include "lulu.h"
+// Counters for the number of header_rewrite operators executed and conditions
evaluated; the
+// bare hook invocation count cannot tell a small ruleset from a large one.
Created once via
+// init_hrw_work_stats(); both are TS_ERROR until then, so increments are
guarded.
Review Comment:
Is `init_hrw_work_stats` not serialized with the hook registration?
--
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]