[PR] Add per-plugin CPU workload counters [trafficserver]

["moonchen (via GitHub)" <gi...@apache.org>]

moonchen opened a new pull request, #13270:
URL: https://github.com/apache/trafficserver/pull/13270

   A CPU-usage model that attributes process CPU to workload needs a per-plugin
   driver; today plugin CPU has no metric and lands in an unexplained baseline.
   Add proxy.process.plugin.<name>.{invocations,bytes,transfers}, keyed by the
   plugin DSO basename and bounded by the number of loaded plugins.
   
   invocations counts hook and remap callback dispatches, incremented at
   INKContInternal::handle_event and RemapPlugins::run_plugin.  bytes and transfers
   attribute PluginVC intercept transport to the owning plugin, captured on
   PluginVCCore at alloc.  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.
   
   transfers is bumped once per PluginVC write-side pass; it reflects buffering and
   reenable behavior, so it is a diagnostic of transport activity rather than an
   external workload driver.
   


-- 
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: github-unsubscribe@trafficserver.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] Add per-plugin CPU workload counters [trafficserver]

["Copilot (via GitHub)" <gi...@apache.org>]

Copilot commented on code in PR #13270:
URL: https://github.com/apache/trafficserver/pull/13270#discussion_r3416331293


##########
src/proxy/http/remap/PluginDso.cc:
##########
@@ -55,8 +61,45 @@ concat_error(std::string &error, const std::string &msg)
   }
 }
 
+// plugin_metric_token derives a metric-safe token from a plugin path or name: it
+// takes the file basename, drops the extension, and replaces any character outside
+// [A-Za-z0-9_-] with '_', so the result is a single dotted segment in
+// proxy.process.plugin.<token>.* (e.g. "/.../header_rewrite.so" -> "header_rewrite",
+// "asset_token.cc" -> "asset_token").
+std::string
+plugin_metric_token(std::string_view name)
+{
+  if (auto slash = name.find_last_of('/'); slash != std::string_view::npos) {
+    name.remove_prefix(slash + 1);
+  }
+  if (auto dot = name.find_first_of('.'); dot != std::string_view::npos) {
+    name = name.substr(0, dot);
+  }

Review Comment:
   `plugin_metric_token()` claims to "drop the extension", but it truncates at the *first* '.' in the basename. For plugin filenames containing dots (e.g. `foo.bar.so`), this will collapse distinct plugins to the same metric token (`foo`) and contradict the documented behavior. Use the last '.' to strip only the extension.



##########
src/proxy/PluginVC.cc:
##########
@@ -1002,6 +1017,9 @@ PluginVCCore::alloc(Continuation *acceptor, int64_t buffer_index, int64_t buffer
   PluginVCCore *pvc = new PluginVCCore;
   pvc->init(buffer_index, buffer_water_mark);
   pvc->connect_to = acceptor;
+  // Capture the plugin that is creating this intercept so its transport bytes can be attributed to
+  // proxy.process.plugin.<name>.bytes; null for core-internal PluginVCs.
+  pvc->_owner = pluginThreadContext;
   return pvc;

Review Comment:
   `PluginVCCore::_owner` captures `pluginThreadContext` but doesn't retain it. For remap plugins, `PluginDso::release()` can unload the DSO when the refcount reaches 0; a long-lived PluginVC intercept can then later dereference a freed context in `transfer_bytes()` / `process_write_side()`. Acquire the context when capturing it, and release it when the core is destroyed (global-plugin contexts can remain no-ops).



-- 
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: github-unsubscribe@trafficserver.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] Add per-plugin CPU workload counters [trafficserver]

["moonchen (via GitHub)" <gi...@apache.org>]

moonchen closed pull request #13270: Add per-plugin CPU workload counters
URL: https://github.com/apache/trafficserver/pull/13270


-- 
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: github-unsubscribe@trafficserver.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org