-
Notifications
You must be signed in to change notification settings - Fork 5.3k
thrift proxy: add request shadowing support #17544
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
Merged
mattklein123
merged 27 commits into
envoyproxy:main
from
rgs1:thrift-add-shadowing-support
Aug 9, 2021
Merged
Changes from all commits
Commits
Show all changes
27 commits
Select commit
Hold shift + click to select a range
901bc0c
thrift proxy: add request shadowing support
16a4664
Fix version history
93ef840
Fix format
fb8ae8f
Fix clang-tidy
14ee7e9
Merge remote-tracking branch 'upstream/main' into thrift-add-shadowin…
4fe99c8
Fix changelog ordering
d09d0ee
Merge remote-tracking branch 'upstream/main' into thrift-add-shadowin…
43fc946
Expand testing & coverage
633582c
Merge remote-tracking branch 'upstream/main' into thrift-add-shadowin…
d77d786
More coverage
2131d07
More coverage
53c3220
Ignore upstreamData()'s return value
386b560
Initialize bools
6314439
More missing initializations
95d6f62
Delete unused code
86d0c3b
Use real RequestMirrorPolicyImpl instead of a mock
7cf0996
continueDecoding() with no pending callbacks
0a43ef8
More NullResponseDecoder coverage
98581cf
Drop TODO
39944e2
DRY things up
4e8d94a
ShadowWriter: coverage for exception paths
f5c0743
Test local close before response
6140c6e
Convert redundant check into assertion
3e1a014
Simplify requestInProgress()
4a0bd52
Test Oneway shadow call
96f5248
Exercise more field types with shadow request
3986a18
Map/List/Set
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
30 changes: 29 additions & 1 deletion
30
api/envoy/extensions/filters/network/thrift_proxy/v4alpha/route.proto
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
27 changes: 26 additions & 1 deletion
27
generated_api_shadow/envoy/extensions/filters/network/thrift_proxy/v3/route.proto
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
30 changes: 29 additions & 1 deletion
30
generated_api_shadow/envoy/extensions/filters/network/thrift_proxy/v4alpha/route.proto
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
I think you should remove the "or if the key is not present, the default value," as it complicates things, and you already wrote above "If not specified, all requests..." (I suggest moving this sentence to be the last, so you first explain what this field is, and then note the default value).
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.
This comes verbatim from the HTTP message, shall I clean up the description for both in a follow-up? Per my other comment, I rather keep them separate but happy to clarify both of them in one go.