Add ability for stats over http to run as a remap plugin#12316
Add ability for stats over http to run as a remap plugin#12316ezelkow1 wants to merge 2 commits intoapache:masterfrom
Conversation
|
I suppose this should also have a docs change, which I can add tomorrow, but any feedback appreciated since I whipped this up pretty quick from what I vaguely remember doing before for this |
|
[approve ci autest] |
|
|
||
| This plugin can also run as a remap plugin with an optional stats over http config file as a parameter:: | ||
|
|
||
| map / http://target @plugin=stats_over_http.so @pparam=config.file |
There was a problem hiding this comment.
Nitpicky, but this seems like a dangerous example :). Shouldn't we use a more specific path here, like
map /_stats_over_http ...
or maybe even more specific (since the above captures the path for all hosts)?
Also, maybe mention that the "target" doesn't imply / mean anything, and can be whatever ?
| #include <zlib.h> | ||
|
|
||
| #include <ts/remap.h> | ||
| #include <ts/remap_version.h> |
There was a problem hiding this comment.
Separate issue, but seeing this, makes me thing we ought to add the define from this file into ts/remap.h.
| } | ||
|
|
||
| TSRemapStatus | ||
| TSRemapDoRemap(void *id, TSHttpTxn rh, TSRemapRequestInfo * /* ATS_UNUSED */) |
There was a problem hiding this comment.
Why id ? Typically we call this ih
| (config_holder->config_path == nullptr)) { | ||
| config_holder->config->stats_path = argv[2] + ('/' == argv[2][0] ? 1 : 0); | ||
| } else if ((config_holder->config != nullptr) && (config_holder->config->stats_path.empty())) { | ||
| config_holder->config->stats_path = DEFAULT_URL_PATH; |
There was a problem hiding this comment.
Maybe I'm not reading this right, but shouldn't the default path here be the "from_url" path in the RRI (the remap.config "from URL") ?
It kinda feels weird that you can override the path with the pparam or config file. What would happen if I do
map http://localhost/_stats_over_http http://default @plugin=stats_over_http.so @pparam=/bar
How could that ever match on the path now ?
|
This pull request has been automatically marked as stale because it has not had recent activity. Marking it stale to flag it for further consideration by the community. |
3 participants
Tested as global and remap, with path as param as well as config file