Merge v2 into main (with -X theirs) #556
Merged
Conversation
This file contains 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
The main goals are to have separate modules per each implementation and all reusing the same code.
This way we can ensure consistency, and we reduce LOC to a minimum, including tests.
## Changes:
* [x] Removed grpc_ prefix from all packages like `grpc_middleware`. Users can add them on their own.
* [x] Moved all specific implementations to separate modules. The exception is opentracing, as opentracing itself
is just an amazing interface, so.. perfect (:
* [x] Added generic interceptor code that both monitoring, logging, and tracing will use under /interceptors
* [x] Reuse generic interceptors in interceptor/{logging,tracing,metrics,tags}. This allows huge simplification of those.
* [x] No context logger. Only ctxtags.Tags responsible for propagating tags in context and proto messages.
* [x] Tags are now map[string]string to discourage context value abuse.
* [x] PayloadUnary works as a standalone interceptor as well.
* [x] Tags will actually use map tags instead of noop if not created.
* [x] Added scripts for a proto generation.
* [x] Let tag.Interceptor to extract request data for all types of requests.
Signed-off-by: Bartlomiej Plotka <bwplotka@gmail.com>
Signed-off-by: Bartlomiej Plotka <bwplotka@gmail.com>
Initial change for v2.
Signed-off-by: Bartlomiej Plotka <bwplotka@gmail.com>
Updated README with note that it's under development.
Signed-off-by: Bartlomiej Plotka <bwplotka@gmail.com>
…testpb. (#291) * Formatted code; Added goimports to Makefile, Renamed pb_testproto to testpb. Signed-off-by: Bartlomiej Plotka <bwplotka@gmail.com> * Formatted code; Added goimports to Makefile, Renamed pb_testproto to testpb. Signed-off-by: Bartlomiej Plotka <bwplotka@gmail.com> * Addressed Johan's comments. Signed-off-by: Bartlomiej Plotka <bwplotka@gmail.com>
* Fixed providers go modules, examples and consistency. Signed-off-by: Bartlomiej Plotka <bwplotka@gmail.com> * Fixed typos. Signed-off-by: Bartlomiej Plotka <bwplotka@gmail.com>
Signed-off-by: Yash Sharma <yashrsharma44@gmail.com>
It is now relative to GOGOPROTO_PATH
* Moved to GH actions; Added lint; Added issue/PR templates; Fixed vet Signed-off-by: Bartlomiej Plotka <bwplotka@gmail.com> * Fixed linter detected issues. Signed-off-by: Bartlomiej Plotka <bwplotka@gmail.com> * Modify the require_clean_work_tree function (#307) * Fixed linter detected issues. Signed-off-by: Bartlomiej Plotka <bwplotka@gmail.com> * Add format-only flag Signed-off-by: Yash Sharma <yashrsharma44@gmail.com> * Modify the require_clean_work_tree function. Taken from - https://github.com/git/git/blob/master/git-sh-setup.sh#L211 Signed-off-by: Yash Sharma <yashrsharma44@gmail.com> Added a git script Signed-off-by: Yash Sharma <yashrsharma44@gmail.com> chmod change Signed-off-by: Yash Sharma <yashrsharma44@gmail.com> changes to git-tree.sh Signed-off-by: Yash Sharma <yashrsharma44@gmail.com> Modified makefile Signed-off-by: Yash Sharma <yashrsharma44@gmail.com> clean up changes Signed-off-by: Yash Sharma <yashrsharma44@gmail.com> * Bump timeout deadline to 100ms Signed-off-by: Yash Sharma <yashrsharma44@gmail.com> Co-authored-by: Bartlomiej Plotka <bwplotka@gmail.com> * Skipped retry call timeout tests, will fix in later PR. Signed-off-by: Bartlomiej Plotka <bwplotka@gmail.com> * Fixed race. Signed-off-by: Bartlomiej Plotka <bwplotka@gmail.com> Co-authored-by: Yash Sharma <yashrsharma44@gmail.com>
* inline localhost certificate into go file This change allows library clients correctly run test suites when vendoring modules. Before this commit, these files would be pruned when vendoring, causing the tests to fail or deadlock. resolves #285 * add deadline to server start to avoid deadlock This commit fixes an issue which occurs if either of the assertions on L61 or L64 fail. In this case, the `serverRunning` chan will never be written or closed and the test will deadlock forever or until the test suite times out (default 10 minutes). * revert erroneous go.mod change
Return new stream with wrapped context instead of original so changes will be passed further the chain
* test certs - cherry-pick PR325 on v2 * remove old test certificates * set CommonName to example.com
* add all make target, reword instructions * remove extra line, reword per suggestion
* remove 1.12.x from build config for consistency with master * fix quote Co-authored-by: Johan Brandhorst-Satzkorn <johan.brandhorst@gmail.com> Co-authored-by: Johan Brandhorst-Satzkorn <johan.brandhorst@gmail.com>
* Fix the special case for jaeger format traceid extraction being overridden regression introduced with #262 * Add unit test for Jaeger trace id parsing fix
* add initial setup for the request logger Signed-off-by: Yash Sharma <yashrsharma44@gmail.com> * compose the request logger in the original logger itself Signed-off-by: Yash Sharma <yashrsharma44@gmail.com> * modify the tests to use the new param signature Signed-off-by: Yash Sharma <yashrsharma44@gmail.com> * configure the option to enable or disable request logger Signed-off-by: Yash Sharma <yashrsharma44@gmail.com> * added logging of metadata of the request object Signed-off-by: Yash Sharma <yashrsharma44@gmail.com> * nitpickings Signed-off-by: Yash Sharma <yashrsharma44@gmail.com> * added pre call implementation Signed-off-by: Yash Sharma <yashrsharma44@gmail.com> * Nitpickings * Change the naming of PostRequestLoggingDecider to PreRequestLoggingDecider * Log only request details not response Signed-off-by: Yash Sharma <yashrsharma44@gmail.com> * Removed the request decider and added another option for enabling the request logging * This ensures that the current logging works as it was working earlier * If the user wants to enable the request logging, use the options for enabling the logging * The request logging is switched off by default Signed-off-by: Yash Sharma <yashrsharma44@gmail.com> * Removed the decider from the tests Signed-off-by: Yash Sharma <yashrsharma44@gmail.com> * Apply suggestions from code review Add changes from bartek's review * Added logging of start and end messages of the request and response from the server side Signed-off-by: Yash Sharma <yashrsharma44@gmail.com> * Added a decider for enabling request logging Signed-off-by: Yash Sharma <yashrsharma44@gmail.com> * Added level of logging and contextual message for identifying the start and end of request and response start messages Signed-off-by: Yash Sharma <yashrsharma44@gmail.com> * add more fields to record the start and end of the call Signed-off-by: Yash Sharma <yashrsharma44@gmail.com> * modified the logging message Signed-off-by: Yash Sharma <yashrsharma44@gmail.com> * Added tests for the request logging * Unary Server testing * Stream server testing * Unary Error testing Signed-off-by: Yash Sharma <yashrsharma44@gmail.com> nitpickings Signed-off-by: Yash Sharma <yashrsharma44@gmail.com> more nitpickings Signed-off-by: Yash Sharma <yashrsharma44@gmail.com> * Added changes as suggested by Bartek * return early for multiple cases * intialize the level early Signed-off-by: Yash Sharma <yashrsharma44@gmail.com> * Reverted logging strategy * Now enabling logging logs out all the details of the request/response * Breaks the initial logging api Signed-off-by: Yash Sharma <yashrsharma44@gmail.com> * Added underscore for proper spacing and added grpc.error for protocol encoding Signed-off-by: Yash Sharma <yashrsharma44@gmail.com> * Changes as requested by the reviewer - * Human Readable Messages - All the log entries are changed to use space instead of `_`. * Using key-value pairs for storing the kind, type and component. * Changed to start call and finished call. Signed-off-by: Yash Sharma <yashrsharma44@gmail.com> * Modified the tests for the new API Signed-off-by: Yash Sharma <yashrsharma44@gmail.com> * nitpickings Signed-off-by: Yash Sharma <yashrsharma44@gmail.com> * Multiple changes * Abstracted out repititive fields into a single method * Added test fields, instead of creating a whole suite * Changed field values to adhere the common convention Signed-off-by: Yash Sharma <yashrsharma44@gmail.com> * Added tests for the finish call\n*Test the finish call in every test suite rather than creating a new suite Signed-off-by: Yash Sharma <yashrsharma44@gmail.com> * Modified the decider to use only method for deciding the logging Signed-off-by: Yash Sharma <yashrsharma44@gmail.com> * Added ENUM based decider for logging * Currently 3 types of logging configured * NoLogCall - No logging * LogFinishCall - For logging the end of the call * LogAllCall - For logging the start and end of the call Signed-off-by: Yash Sharma <yashrsharma44@gmail.com> * added some comments for usage of decider Signed-off-by: Yash Sharma <yashrsharma44@gmail.com> * renamed LogAllCall to LogStartAndFinishCall Signed-off-by: Yash Sharma <yashrsharma44@gmail.com> * remove unused comments Signed-off-by: Yash Sharma <yashrsharma44@gmail.com> More comments removal Signed-off-by: Yash Sharma <yashrsharma44@gmail.com> * modified the examples in the provider Signed-off-by: Yash Sharma <yashrsharma44@gmail.com>
* comment some tests and add a lock Signed-off-by: Yash Sharma <yashrsharma44@gmail.com> * Added the latest version of grpc middleware Signed-off-by: Yash Sharma <yashrsharma44@gmail.com> * Added the new version of middleware commit for importing the new API for logging in the example tests Signed-off-by: Yash Sharma <yashrsharma44@gmail.com> * more changes Signed-off-by: Yash Sharma <yashrsharma44@gmail.com> * Added some locks and checks Signed-off-by: Yash Sharma <yashrsharma44@gmail.com> * added some changes as suggested by reviewer Signed-off-by: Yash Sharma <yashrsharma44@gmail.com> * Extra changes Signed-off-by: Yash Sharma <yashrsharma44@gmail.com> * Renamed the baseMockLogger to sharedResults and added some more locks Signed-off-by: Yash Sharma <yashrsharma44@gmail.com> * Renamed the sharedResults to mockStdOutput and sorted the stdoutput slice so that even due to concurrency, the output remains in a fixed sorted order Signed-off-by: Yash Sharma <yashrsharma44@gmail.com>
* make ratelimit interface context aware. * cleanup ratelimit testcases
Signed-off-by: Yash Sharma <yashrsharma44@gmail.com>
* Export `SplitMethodName` and `StreamRPCType` function * Add skip interceptor * Fix lint * Apply suggestions from code review Co-authored-by: Bartlomiej Plotka <bwplotka@gmail.com> * Use allowlist * Add skip interceptor into README Co-authored-by: Bartlomiej Plotka <bwplotka@gmail.com>
* remove dependency on middleware.Chain from providers/kit * skip wont be able to bypass multiple interceptors without an API change * update examples by removing middleware chains
…bility. (#394) * Removed tags; Simplified interceptor code; Added logging fields editability. Fixes #382 Signed-off-by: Bartlomiej Plotka <bwplotka@gmail.com> Removed tags; Simplified interceptor code; Added logging fields editability. Fixes #382 Signed-off-by: Bartlomiej Plotka <bwplotka@gmail.com> Fixed tests. Signed-off-by: Bartlomiej Plotka <bwplotka@gmail.com> Fixed open metrics test. Signed-off-by: Bartlomiej Plotka <bwplotka@gmail.com> * Addressed comments. Signed-off-by: Bartlomiej Plotka <bwplotka@gmail.com>
Co-authored-by: levanli <levanli@tencent.com>
Signed-off-by: Juraci Paixão Kröhling <juraci@kroehling.de>
Signed-off-by: Juraci Paixão Kröhling <juraci@kroehling.de>
* Fix providers/kit for latest v2 * providers/kit: Fix linting
* Add tracing interceptor * Fix CI * Follow dependencies changes * Fix comments * Avoid redundant else and return early * Update interceptors/tracing/interceptors_test.go Co-authored-by: Yash Sharma <yashrsharma44@gmail.com> * Remove kv package * Remove keyvalue Co-authored-by: Yash Sharma <yashrsharma44@gmail.com>
* Add opentracing provider * Add copyright * Fix format * Fix copyright
Signed-off-by: aimuz <mr.imuz@gmail.com> Signed-off-by: aimuz <mr.imuz@gmail.com>
Signed-off-by: aimuz <mr.imuz@gmail.com> Signed-off-by: aimuz <mr.imuz@gmail.com>
Signed-off-by: aimuz <mr.imuz@gmail.com> Signed-off-by: aimuz <mr.imuz@gmail.com>
Signed-off-by: aimuz <mr.imuz@gmail.com>
…re (#543) * v2: Cleanup, exemplars and examples. Signed-off-by: bwplotka <bwplotka@gmail.com> * interface -> any Signed-off-by: bwplotka <bwplotka@gmail.com> --------- Signed-off-by: bwplotka <bwplotka@gmail.com>
* feat: add error logging in validator used logging.Logger interface to add error logging in validator interceptor addition: #494 * feat: update interceptor implementation made fast fail and logger as optional args addition to that instead of providing values dynamically at the time of initialization made it more dynamic * fix: update options args updated args based on review * refactor: update validate func restructured if statement in-order to make code execution based on shouldFailFast flag more relevant. * refactor: shifted interceptors into new file restructured code in order to separate the concern. ie: in terms of code struct and testcases wise. * test: updated the testcases modified testcases based on the current modifications made in the code base. * fix: add copyright headers * fix: update comment and code updated code based on reviews
…ng providers to examples only. (#552) Fixes #517 * Simpler interface, only one method. This allows adapter to be ultra simple, so moved them to examples only (!). * Fields are string, values are any. * "More" slog compatibility. I did not go for using slog natively as it's still experimental (see bigger discussion: https://gophers.slack.com/archives/C029RQSEE/p1679860885943619) Signed-off-by: bwplotka <bwplotka@gmail.com>
Signed-off-by: bwplotka <bwplotka@gmail.com>
Signed-off-by: bwplotka <bwplotka@gmail.com>
Signed-off-by: bwplotka <bwplotka@gmail.com>
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
|
@danielhochman do you mind signing CLA (see https://github.com/grpc-ecosystem/go-grpc-middleware/pull/556/checks?check_run_id=12500982424) - looks like we missed this in your past commit. Otherwise I will have to rebase and remove your commit from this chain ): You can sign by clicking here |
Signed-off-by: bwplotka <bwplotka@gmail.com>
johanbrandhorst
approved these changes
Apr 4, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This effectively replaces v1 (old master) code with v2, with v1 history.
Current master (now main) is still available in v1