-
Notifications
You must be signed in to change notification settings - Fork 840
prscs: proxy response status code setter #12484
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
213f3fd
to
275aa4e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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. |
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.