prscs: proxy response status code setter#12484
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR introduces a plugin status setter (plss) feature to help identify which plugin or component set a transaction's HTTP response status. This is valuable for debugging complex setups with multiple plugins that may modify response status codes.
- Adds a new logging field 'plss' (Plugin Last Set Status) to track which entity set the transaction's status
- Enhances TSHttpTxnStatusSet and TSHttpHdrStatusSet APIs with optional setter parameter
- Updates all plugins and internal components to use the new setter tracking functionality
Reviewed Changes
Copilot reviewed 57 out of 57 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| include/ts/ts.h | Adds new API functions with setter parameters and documentation |
| src/api/InkAPI.cc | Implements the core setter tracking functionality |
| include/proxy/http/HttpTransact.h | Adds status setter name field to transaction state |
| src/proxy/logging/Log.cc, LogAccess.cc | Implements plss log field for status setter tracking |
| src/proxy/http/*.cc | Updates core components to set "ip_allow" as setter |
| plugins/*/**.cc | Updates all plugins to pass their names as setter parameters |
| tests/gold_tests/* | Adds tests to verify the new logging functionality |
| doc/* | Adds comprehensive documentation for new API functions |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
bneradt
force-pushed
the
plss
branch
2 times, most recently
from
213f3fd to
275aa4e
Compare
bneradt
changed the title
cmcfarlen
left a comment
cmcfarlen
left a comment
There was a problem hiding this comment.
Setting the new setter field in TSHttpTxnStatusSet makes sense, but setting it in TSHttpHdrStatusSet seems strange to me. Also, wasn't there some discussion about removing proxy from the log field name?
|
Thanks for the review. A quick answer to this question:
The concern was not with "proxy" but with my previous "plugin" terminology. "plugin" is wrong because aspects of ATS core can set the response, not just plugins. This now uses "proxy". I think that's better. "proxy" in prscs fits well with the other p* log fields which are also proxy related. |
bneradt
force-pushed
the
plss
branch
2 times, most recently
from
07e02a8 to
da67920
Compare
In complex setups where there are multiple plugins that can be setting an error response, it can be non-trival to determine which component intervened for any given transaction. This adds the prscs log field, proxy response status code setter, which provides an identifying string for the component (a plugin name, or ip_allow, for instance) that set the transaction's response status.
In complex setups where there are multiple plugins that can be setting an error response, it can be non-trival to determine which component intervened for any given transaction. This adds the prscs log field, proxy response status code setter, which provides an identifying string for the component (a plugin name, or ip_allow, for instance) that set the transaction's response status. (cherry picked from commit 48dbca7)
In complex setups where there are multiple plugins that can be setting
an error response, it can be non-trival to determine which component
intervened for any given transaction. This adds the prscs log field,
proxy response status code setter, which provides an identifying string
for the component (a plugin name, or ip_allow, for instance) that set
the transaction's response status.