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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
