From fa731b223d9a529944042b41eb3dd62427926dd9 Mon Sep 17 00:00:00 2001 From: Mo Chen Date: Mon, 15 Jun 2026 14:24:21 -0500 Subject: [PATCH] header_rewrite: count operators and conditions run The per-hook invocation count cannot tell a small ruleset from a large one, so it does not reflect how much work header_rewrite does per transaction. Add proxy.process.plugin.header_rewrite.{operators,conditions}, incremented in the inline Operator::do_exec and Condition::do_eval chain walkers, to make that workload visible. do_eval short-circuits, so only conditions actually evaluated are counted. The counters are created once, from both TSPluginInit and TSRemapInit. --- plugins/header_rewrite/condition.h | 4 +++ plugins/header_rewrite/header_rewrite.cc | 4 +++ plugins/header_rewrite/operator.h | 3 ++ plugins/header_rewrite/statement.cc | 18 +++++++++++ plugins/header_rewrite/statement.h | 8 +++++ .../autest-site/ats_replay.test.ext | 30 ++++++++++++++----- .../header_rewrite_bundle.replay.yaml | 6 ++++ 7 files changed, 65 insertions(+), 8 deletions(-) diff --git a/plugins/header_rewrite/condition.h b/plugins/header_rewrite/condition.h index 1400f3433a0..63f2da6773f 100644 --- a/plugins/header_rewrite/condition.h +++ b/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. + if (hrw_stat_conditions != nullptr) { + ts::Metrics::Counter::increment(hrw_stat_conditions); + } bool rt = eval(res); if (has_modifier(_mods, CondModifiers::NOT)) { diff --git a/plugins/header_rewrite/header_rewrite.cc b/plugins/header_rewrite/header_rewrite.cc index 16f8b0c6170..9a43a73813b 100644 --- a/plugins/header_rewrite/header_rewrite.cc +++ b/plugins/header_rewrite/header_rewrite.cc @@ -569,6 +569,8 @@ TSPluginInit(int argc, const char *argv[]) return; } + init_hrw_work_stats(); + std::string geoDBpath; int inboundIpSource = 0; int timezone = 0; @@ -663,6 +665,8 @@ TSRemapInit(TSRemapInterface *api_info, char *errbuf, int errbuf_size) CHECK_REMAP_API_COMPATIBILITY(api_info, errbuf, errbuf_size); Dbg(pi_dbg_ctl, "Remap plugin is successfully initialized"); + init_hrw_work_stats(); + return TS_SUCCESS; } diff --git a/plugins/header_rewrite/operator.h b/plugins/header_rewrite/operator.h index bc23c93aabb..a44211ba97a 100644 --- a/plugins/header_rewrite/operator.h +++ b/plugins/header_rewrite/operator.h @@ -78,6 +78,9 @@ class Operator : public Statement unsigned do_exec(const Resources &res) const { + if (hrw_stat_operators != nullptr) { + ts::Metrics::Counter::increment(hrw_stat_operators); + } unsigned no_reenable{exec(res) ? 0U : 1U}; if (nullptr != _next) { no_reenable += static_cast(_next)->do_exec(res); diff --git a/plugins/header_rewrite/statement.cc b/plugins/header_rewrite/statement.cc index 6e2607da634..8bce404a2d4 100644 --- a/plugins/header_rewrite/statement.cc +++ b/plugins/header_rewrite/statement.cc @@ -21,6 +21,24 @@ // #include "statement.h" +ts::Metrics::Counter::AtomicType *hrw_stat_operators = nullptr; +ts::Metrics::Counter::AtomicType *hrw_stat_conditions = nullptr; + +void +init_hrw_work_stats() +{ + // createPtr() is idempotent and internally synchronized: it returns the existing counter when + // one already has the given name. header_rewrite.so can be loaded both globally (TSPluginInit) + // and as a remap plugin (TSRemapInit), so init runs more than once; every call resolves to the + // same process-wide counter regardless of ordering or which thread runs it. + if (nullptr == hrw_stat_operators) { + hrw_stat_operators = ts::Metrics::Counter::createPtr("proxy.process.plugin.header_rewrite.operators"); + } + if (nullptr == hrw_stat_conditions) { + hrw_stat_conditions = ts::Metrics::Counter::createPtr("proxy.process.plugin.header_rewrite.conditions"); + } +} + void Statement::append(Statement *stmt) { diff --git a/plugins/header_rewrite/statement.h b/plugins/header_rewrite/statement.h index 2557d4482c1..1f16c24246b 100644 --- a/plugins/header_rewrite/statement.h +++ b/plugins/header_rewrite/statement.h @@ -27,11 +27,19 @@ #include #include "ts/ts.h" +#include "tsutil/Metrics.h" #include "resources.h" #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 null until then, so increments are guarded. +extern ts::Metrics::Counter::AtomicType *hrw_stat_operators; +extern ts::Metrics::Counter::AtomicType *hrw_stat_conditions; +void init_hrw_work_stats(); + namespace header_rewrite_ns { constexpr int NUM_STATE_FLAGS = 16; diff --git a/tests/gold_tests/autest-site/ats_replay.test.ext b/tests/gold_tests/autest-site/ats_replay.test.ext index d6379025b20..b34e30ed2e4 100644 --- a/tests/gold_tests/autest-site/ats_replay.test.ext +++ b/tests/gold_tests/autest-site/ats_replay.test.ext @@ -323,14 +323,28 @@ def ATSReplayTest(obj, replay_file: str): for check in metric_checks: metric_name = check['metric'] - expected_value = check['value'] - check_tr = obj.AddTestRun(f'Verify {metric_name} == {expected_value}') - check_tr.Processes.Default.Command = f'traffic_ctl metric get {metric_name}' - check_tr.Processes.Default.Env = ts.Env - check_tr.Processes.Default.ReturnCode = 0 - check_tr.Processes.Default.Streams.All = Testers.ContainsExpression( - f'^{re.escape(metric_name)}\\s+{expected_value}$', f'{metric_name} should be {expected_value}') - check_tr.StillRunningAfter = ts + if 'value' in check: + expected_value = check['value'] + check_tr = obj.AddTestRun(f'Verify {metric_name} == {expected_value}') + check_tr.Processes.Default.Command = f'traffic_ctl metric get {metric_name}' + check_tr.Processes.Default.Env = ts.Env + check_tr.Processes.Default.ReturnCode = 0 + check_tr.Processes.Default.Streams.All = Testers.ContainsExpression( + f'^{re.escape(metric_name)}\\s+{expected_value}$', f'{metric_name} should be {expected_value}') + check_tr.StillRunningAfter = ts + elif 'min' in check: + minimum = check['min'] + check_tr = obj.AddTestRun(f'Verify {metric_name} >= {minimum}') + # Fail if the metric value (field 2) is below the minimum, or absent. + # $$ escapes the awk field ref from autest's command templating. + check_tr.Processes.Default.Command = ( + f"traffic_ctl metric get {metric_name} | " + f"awk 'BEGIN {{ rc = 1 }} {{ if ($$2 + 0 >= {minimum}) rc = 0 }} END {{ exit rc }}'") + check_tr.Processes.Default.Env = ts.Env + check_tr.Processes.Default.ReturnCode = 0 + check_tr.StillRunningAfter = ts + else: + raise ValueError(f"metric_checks entry for '{metric_name}' must specify 'value' or 'min'") # wait for error log if log_validation and log_validation.get('error_log'): diff --git a/tests/gold_tests/pluginTest/header_rewrite/header_rewrite_bundle.replay.yaml b/tests/gold_tests/pluginTest/header_rewrite/header_rewrite_bundle.replay.yaml index 97e1d6d2852..b444acaae86 100644 --- a/tests/gold_tests/pluginTest/header_rewrite/header_rewrite_bundle.replay.yaml +++ b/tests/gold_tests/pluginTest/header_rewrite/header_rewrite_bundle.replay.yaml @@ -176,6 +176,12 @@ autest: args: - "rules/rule_session_vars.conf" + metric_checks: + - metric: "proxy.process.plugin.header_rewrite.operators" + min: 1 + - metric: "proxy.process.plugin.header_rewrite.conditions" + min: 1 + log_validation: traffic_out: excludes: