Add per-plugin workload counters by moonchen · Pull Request #13278 · apache/trafficserver

moonchen · 2026-06-15T21:28:55Z

Add per-plugin metrics that allow us to track how much work each plugin is doing by counting their invocations, intercept bytes, and intercept transfers.

Add proxy.process.plugin..{invocations,bytes, transfers}, keyed by the plugin DSO basename and bounded by the number of loaded plugins.

Global plugins load via raw dlopen and previously carried no identity, so they are given a PluginThreadContext around TSPluginInit; the continuations they create then carry plugin identity the same way remap plugins already do.

Copilot

Pull request overview

This PR adds per-plugin workload metrics to Apache Traffic Server so operators can attribute plugin activity to specific plugin DSOs (by basename), covering callback invocations as well as PluginVC intercept transport bytes/transfers. It also adds a gold test to validate that the new counters increment for both global and remap plugins.

Changes:

Add proxy.process.plugin.<name>.{invocations,bytes,transfers} counters and increment them from key dispatch / transport paths.
Introduce a PluginThreadContext wrapper for global plugins so continuations they create carry plugin identity (similar to remap plugins).
Add a gold test that drives traffic through a global plugin, a remap plugin, and a PluginVC intercept plugin and asserts the counters are non-zero.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
tests/gold_tests/pluginTest/per_plugin_metrics/rules/global.conf	Adds a header_rewrite rule to ensure global-plugin continuation dispatch occurs per transaction.
tests/gold_tests/pluginTest/per_plugin_metrics/per_plugin_metrics.test.py	New gold test validating per-plugin invocation/bytes/transfers metrics via `traffic_ctl metric get`.
src/proxy/PluginVC.cc	Increments per-plugin bytes/transfers counters for PluginVC intercept data movement and captures counters at core allocation time.
include/proxy/PluginVC.h	Stores per-plugin counter pointers in `PluginVCCore` for intercept accounting.
src/proxy/Plugin.cc	Wraps global plugin initialization in a `PluginThreadContext` so created continuations carry plugin identity.
src/proxy/http/remap/RemapPlugins.cc	Counts remap plugin invocations at the remap dispatch site.
include/proxy/http/remap/PluginDso.h	Adds metric registration/increment helpers and per-plugin counter pointers to `PluginThreadContext`.
src/proxy/http/remap/PluginDso.cc	Implements metric-name tokenization and registers per-plugin counters on successful plugin load.
src/api/InkContInternal.cc	Counts per-plugin invocations for TSCont (continuation) callback dispatches.

Add per-plugin metrics that allow us to track how much work each plugin is doing by counting their invocations, intercept bytes, and intercept transfers. Add proxy.process.plugin.<name>.{invocations,bytes, transfers}, keyed by the plugin DSO basename and bounded by the number of loaded plugins. Global plugins load via raw dlopen and previously carried no identity, so they are given a PluginThreadContext around TSPluginInit; the continuations they create then carry plugin identity the same way remap plugins already do. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

moonchen · 2026-06-18T20:37:03Z

[approve ci autest]

moonchen · 2026-06-18T20:42:54Z

[approve ci osx]

moonchen · 2026-06-18T20:43:00Z

[approve ci rocky]

moonchen · 2026-06-18T21:26:39Z

[approve ci rocky]

The per-plugin metrics change pulls the real IPAllow.o into test_proxy (Plugin.o now references PluginDso's registerPluginMetrics), so the stub definition of IpAllow::subjects duplicated the real one and tripped AddressSanitizer's ODR check on the Rocky (ASAN) build. Drop the stub; the linked IPAllow.o now provides the symbol. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.

regex_revalidate and lua gold tests diff `traffic_ctl metric match <name>` against a fixed set, but the new proxy.process.plugin.<name>.* workload counters also match that substring, adding lines and breaking the gold compare. Anchor the match to ^plugin.<name>. so it captures only each plugin's own metrics. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

_transfers was incremented before the other_ntodo / lock-miss / buffer-space early returns, so write-side passes that moved zero bytes were counted. Move the increment after transfer_bytes and gate it on a positive byte count, matching the _bytes counter. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.

GlobalPluginContext in Plugin.cc (ts::proxy) called the out-of-line PluginThreadContext::registerPluginMetrics() defined in PluginDso.cc (ts::http_remap), adding a ts::proxy -> ts::http_remap link dependency. Move the implementation (and its token helper) into the header so the symbol is resolved locally and the cross-library edge goes away. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

moonchen · 2026-06-19T02:04:45Z

[approve ci autest]

bneradt

Good idea to start adding visibility to plugin invocations.

bneradt · 2026-06-20T16:44:14Z

+    for (auto &c : token) {
+      if (!(std::isalnum(static_cast<unsigned char>(c)) || c == '_' || c == '-')) {
+        c = '_';
+      }
+    }


replace_if is made for this and by virtue of its name will communicate intent better:

std::string token{name}; std::replace_if(token.begin(), token.end(), [](unsigned char c) { return !(std::isalnum(c) || c == '_' || c == '-'); }, '_');

Done in becf659 — switched to std::replace_if.

bneradt · 2026-06-20T16:48:22Z

+Test.Summary = '''
+Verify the per-plugin workload counters proxy.process.plugin.<name>.invocations (global and remap
+dispatch), .bytes and .transfers (PluginVC intercept transport).
+'''
+
+Test.ContinueOnFail = True
+
+ts = Test.MakeATSProcess("ts")
+server = Test.MakeOriginServer("server")
+
+request_header = {"headers": "GET / HTTP/1.1\r\nHost: test.example\r\n\r\n", "timestamp": "1469733493.993", "body": ""}


Can you please ask your clanker to reorganize this as a test class?

class TestPerPluginWorkloadCounters: def __init__(self, params): # Create a TestRun and setup up the needed servers and configure the client TestPerPluginWorkloadCounters(params1) TestPerPluginWorkloadCounters(params2)

Done in 4042653 — reorganized as a TestPerPluginWorkloadCounters class.

bneradt · 2026-06-20T16:52:36Z

+  void
+  registerPluginMetrics(std::string_view plugin_name)
+  {
+    std::string prefix = "proxy.process.plugin." + _metric_token(plugin_name) + ".";
+
+    _invocations = ts::Metrics::Counter::createPtr(prefix + "invocations");
+    _bytes       = ts::Metrics::Counter::createPtr(prefix + "bytes");
+    _transfers   = ts::Metrics::Counter::createPtr(prefix + "transfers");
+  }


Can we put the implementation of this and these other functions in PluginDso.cc?

Good call on getting these out of the header. One wrinkle: PluginThreadContext is used by both remap plugins (PluginDso, in ts::http_remap) and global plugins (GlobalPluginContext, in ts::proxy), and the link dependency only runs one way — ts::http_remap → ts::proxy. If the implementations went into PluginDso.cc they would land in ts::http_remap, which would force ts::proxy to link back against ts::http_remap to resolve them for the global-plugin path — that is what broke test_proxy earlier and why they got inlined.

So I have pulled PluginThreadContext into its own file in ts::proxy (PluginThreadContext.{h,cc}) with the implementations in the .cc. ts::http_remap already links ts::proxy, so both plugin paths resolve the symbols and the header stays thin. Does that address your concern?

Addresses review on apache#13278. PluginThreadContext is shared by both remap plugins (PluginDso, ts::http_remap) and global plugins (GlobalPluginContext, ts::proxy), but the link dependency only runs ts::http_remap -> ts::proxy. Pulling it into its own ts::proxy file lets its metric helpers be defined out-of-line without ts::proxy taking a back-dependency on ts::http_remap, which was why they had been left inline in the header. Also switch the metric-token sanitization to std::replace_if. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Addresses review on apache#13278: wrap the per-plugin workload counter checks in a TestPerPluginWorkloadCounters class, matching the class-based gold test idiom. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated 1 comment.

Addresses Copilot review: skip the test unless header_rewrite.so, conf_remap.so and generator.so are present, matching the convention used by other plugin gold tests. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

moonchen · 2026-06-20T23:56:04Z

[approve ci]

cmcfarlen · 2026-06-26T00:29:18Z

Cherry-picked to 10.2.x

Add per-plugin metrics that allow us to track how much work each plugin is doing by counting their invocations, intercept bytes, and intercept transfers. Add proxy.process.plugin.<name>.{invocations,bytes, transfers}, keyed by the plugin DSO basename and bounded by the number of loaded plugins. Global plugins load via raw dlopen and previously carried no identity, so they are given a PluginThreadContext around TSPluginInit; the continuations they create then carry plugin identity the same way remap plugins already do. (cherry picked from commit 990bd04)

* proxy/unit_tests: restore IpAllow::subjects stub definition #13278 dropped this definition, but test_proxy links ts::http (which references IpAllow::subjects) ahead of ts::proxy (which defines it). GNU ld's single-pass archive scan then leaves the symbol unresolved, breaking the Linux build; macOS links fine. master avoids this incidentally via test_PluginYAML.cc from the unbackported plugin.yaml migration (#13070). * tls autests: use ssl_multicert.config instead of yaml These tests were backported from master, where the default cert config is ssl_multicert.yaml and the harness exposes ts.Disk.ssl_multicert_yaml. On 10.2.x the default is still legacy ssl_multicert.config and no ssl_multicert_yaml Disk attribute is registered, so the tests failed at collection. Switch them to the ssl_multicert.config one-liner used by the other 10.2.x TLS tests. * curl 8.20 test update: curl PROXY destination changes (#13239) curl 8.20 intentionally mirrors --haproxy-clientip into both PROXY addresses to keep the header address family consistent. The TSVConnPPInfo AuTest still expected the older destination address, so jobs with newer curl failed even though ATS preserved the PROXY metadata it received. This relaxes the destination-address expectation to accept either curl behavior while continuing to verify the source address and PROXY metadata. This also wraps the long curl command strings while leaving the test's request flow unchanged. (cherry picked from commit ad0ce02) * fedora:44: Trim remap ACL reload waits (#13237) The remap ACL AuTests run hundreds of reload scenarios in a single case, and the Fedora 44 shard is sensitive to extra reload-wait overhead, causing the tests to hang. Their reload sentinel also counted only explicit reloads, even though the log contains the startup load marker too. This replaces the long-lived sleep Ready helper with a short command that exits once the expected reload marker count is present. This also waits for the startup marker plus the explicit reload count, so each scenario observes the reload it just requested. (cherry picked from commit c52eeda) --------- Co-authored-by: Brian Neradt <brian.neradt@gmail.com>

Add per-plugin metrics that allow us to track how much work each plugin is doing by counting their invocations, intercept bytes, and intercept transfers. Add proxy.process.plugin.<name>.{invocations,bytes, transfers}, keyed by the plugin DSO basename and bounded by the number of loaded plugins. Global plugins load via raw dlopen and previously carried no identity, so they are given a PluginThreadContext around TSPluginInit; the continuations they create then carry plugin identity the same way remap plugins already do.

Copilot AI review requested due to automatic review settings June 15, 2026 21:28

Copilot started reviewing on behalf of moonchen June 15, 2026 21:29 View session

moonchen self-assigned this Jun 15, 2026

moonchen added Plugins Metrics PluginVC labels Jun 15, 2026

moonchen added this to ATS v10.2.x Jun 15, 2026

Copilot AI reviewed Jun 15, 2026

View reviewed changes

Comment thread src/proxy/http/remap/PluginDso.cc Outdated

Comment thread src/api/InkContInternal.cc Outdated

moonchen added this to the 10.2.0 milestone Jun 15, 2026

moonchen force-pushed the per-plugin-cpu-metrics branch from 2d9c816 to a36b73c Compare June 15, 2026 21:52

cmcfarlen modified the milestones: 10.2.0, 11.0.0 Jun 15, 2026

cmcfarlen requested a review from bneradt June 15, 2026 22:08

Copilot AI review requested due to automatic review settings June 18, 2026 21:55

Copilot started reviewing on behalf of moonchen June 18, 2026 21:56 View session

Copilot AI reviewed Jun 18, 2026

View reviewed changes

Comment thread src/proxy/PluginVC.cc Outdated

moonchen and others added 2 commits June 18, 2026 18:03

Copilot AI review requested due to automatic review settings June 18, 2026 23:42

Copilot started reviewing on behalf of moonchen June 18, 2026 23:43 View session

Copilot AI reviewed Jun 18, 2026

View reviewed changes

Comment thread src/proxy/Plugin.cc

bneradt requested changes Jun 20, 2026

View reviewed changes

moonchen and others added 2 commits June 20, 2026 18:31

test: reorganize per_plugin_metrics as a test class

4042653

Addresses review on apache#13278: wrap the per-plugin workload counter checks in a TestPerPluginWorkloadCounters class, matching the class-based gold test idiom. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Copilot AI review requested due to automatic review settings June 20, 2026 23:31

Copilot started reviewing on behalf of moonchen June 20, 2026 23:32 View session

Copilot AI reviewed Jun 20, 2026

View reviewed changes

Comment thread tests/gold_tests/pluginTest/per_plugin_metrics/per_plugin_metrics.test.py

test: gate per_plugin_metrics on its plugins

a033680

Addresses Copilot review: skip the test unless header_rewrite.so, conf_remap.so and generator.so are present, matching the convention used by other plugin gold tests. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

moonchen requested a review from bneradt June 21, 2026 00:45

bneradt approved these changes Jun 22, 2026

View reviewed changes

moonchen merged commit 990bd04 into apache:master Jun 22, 2026
15 checks passed

github-project-automation Bot moved this to For v10.2.0 in ATS v10.2.x Jun 22, 2026

cmcfarlen moved this from For v10.2.0 to Picked v10.2.0 in ATS v10.2.x Jun 26, 2026

cmcfarlen modified the milestones: 11.0.0, 10.2.0 Jun 26, 2026

masaori335 mentioned this pull request Jun 26, 2026

[10.2.x] Fix unit test & AuTest #13341

Merged

Uh oh!

Conversation

moonchen commented Jun 15, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

moonchen commented Jun 18, 2026

Uh oh!

moonchen commented Jun 18, 2026

Uh oh!

moonchen commented Jun 18, 2026

Uh oh!

moonchen commented Jun 18, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

moonchen commented Jun 19, 2026

Uh oh!

bneradt left a comment

Choose a reason for hiding this comment

Uh oh!

bneradt Jun 20, 2026

Choose a reason for hiding this comment

Uh oh!

moonchen Jun 20, 2026

Choose a reason for hiding this comment

Uh oh!

bneradt Jun 20, 2026

Choose a reason for hiding this comment

Uh oh!

moonchen Jun 20, 2026

Choose a reason for hiding this comment

Uh oh!

bneradt Jun 20, 2026

Choose a reason for hiding this comment

Uh oh!

moonchen Jun 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

moonchen commented Jun 20, 2026

Uh oh!

Uh oh!

cmcfarlen commented Jun 26, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants