Skip to content
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

docs: add logging.InjectFields usage description #540

Closed
wants to merge 69 commits into from

Conversation

aimuz
Copy link
Contributor

@aimuz aimuz commented Mar 8, 2023

  • I added CHANGELOG entry for this change.
  • Change is not relevant to the end user.

Changes

Verification

bwplotka and others added 30 commits March 5, 2020 12:07
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 <[email protected]>
Signed-off-by: Bartlomiej Plotka <[email protected]>
Updated README with note that it's under development.
…testpb. (grpc-ecosystem#291)

* Formatted code; Added goimports to Makefile, Renamed pb_testproto to testpb.

Signed-off-by: Bartlomiej Plotka <[email protected]>

* Formatted code; Added goimports to Makefile, Renamed pb_testproto to testpb.

Signed-off-by: Bartlomiej Plotka <[email protected]>

* Addressed Johan's comments.

Signed-off-by: Bartlomiej Plotka <[email protected]>
…#292)

* Fixed providers go modules, examples and consistency.

Signed-off-by: Bartlomiej Plotka <[email protected]>

* Fixed typos.

Signed-off-by: Bartlomiej Plotka <[email protected]>
It is now relative to GOGOPROTO_PATH
…ystem#296)

* Moved to GH actions; Added lint; Added issue/PR templates; Fixed vet

Signed-off-by: Bartlomiej Plotka <[email protected]>

* Fixed linter detected issues.

Signed-off-by: Bartlomiej Plotka <[email protected]>

* Modify the require_clean_work_tree function (grpc-ecosystem#307)

* Fixed linter detected issues.

Signed-off-by: Bartlomiej Plotka <[email protected]>

* Add format-only flag

Signed-off-by: Yash Sharma <[email protected]>

* 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 <[email protected]>

Added a git script

Signed-off-by: Yash Sharma <[email protected]>

chmod change

Signed-off-by: Yash Sharma <[email protected]>

changes to git-tree.sh

Signed-off-by: Yash Sharma <[email protected]>

Modified makefile

Signed-off-by: Yash Sharma <[email protected]>

clean up changes

Signed-off-by: Yash Sharma <[email protected]>

* Bump timeout deadline to 100ms

Signed-off-by: Yash Sharma <[email protected]>

Co-authored-by: Bartlomiej Plotka <[email protected]>

* Skipped retry call timeout tests, will fix in later PR.

Signed-off-by: Bartlomiej Plotka <[email protected]>

* Fixed race.

Signed-off-by: Bartlomiej Plotka <[email protected]>

Co-authored-by: Yash Sharma <[email protected]>
* 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 grpc-ecosystem#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
…rpc-ecosystem#323)

- Usually the pattern is to establish a stream and read messages
in the loop until io.EOF. If the server become unavailable after
sending a good message in the stream, the client has to retry from
establish stream and handle the backoff logic.
* 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
…system#337)

* remove 1.12.x from build config for consistency with master

* fix quote

Co-authored-by: Johan Brandhorst-Satzkorn <[email protected]>

Co-authored-by: Johan Brandhorst-Satzkorn <[email protected]>
…ecosystem#340)

* Fix the special case for jaeger format traceid extraction being overridden regression introduced with grpc-ecosystem#262

* Add unit test for Jaeger trace id parsing fix
* add initial setup for the request logger

Signed-off-by: Yash Sharma <[email protected]>

* compose the request logger in the original logger itself

Signed-off-by: Yash Sharma <[email protected]>

* modify the tests to use the new param signature

Signed-off-by: Yash Sharma <[email protected]>

* configure the option to enable or disable request logger

Signed-off-by: Yash Sharma <[email protected]>

* added logging of metadata of the request object

Signed-off-by: Yash Sharma <[email protected]>

* nitpickings

Signed-off-by: Yash Sharma <[email protected]>

* added pre call implementation

Signed-off-by: Yash Sharma <[email protected]>

* Nitpickings
* Change the naming of PostRequestLoggingDecider to PreRequestLoggingDecider
* Log only request details not response

Signed-off-by: Yash Sharma <[email protected]>

* 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 <[email protected]>

* Removed the decider from the tests

Signed-off-by: Yash Sharma <[email protected]>

* 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 <[email protected]>

* Added a decider for enabling request logging

Signed-off-by: Yash Sharma <[email protected]>

* Added level of logging and contextual message for identifying the start and end of request and response start messages

Signed-off-by: Yash Sharma <[email protected]>

* add more fields to record the start and end of the call

Signed-off-by: Yash Sharma <[email protected]>

* modified the logging message

Signed-off-by: Yash Sharma <[email protected]>

* Added tests for the request logging
* Unary Server testing
* Stream server testing
* Unary Error testing

Signed-off-by: Yash Sharma <[email protected]>

nitpickings

Signed-off-by: Yash Sharma <[email protected]>

more nitpickings

Signed-off-by: Yash Sharma <[email protected]>

* Added changes as suggested by Bartek
* return early for multiple cases
* intialize the level early

Signed-off-by: Yash Sharma <[email protected]>

* 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 <[email protected]>

* Added underscore for proper spacing and added grpc.error for protocol encoding

Signed-off-by: Yash Sharma <[email protected]>

* 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 <[email protected]>

* Modified the tests for the new API

Signed-off-by: Yash Sharma <[email protected]>

* nitpickings

Signed-off-by: Yash Sharma <[email protected]>

* 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 <[email protected]>

* 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 <[email protected]>

* Modified the decider to use only method for deciding the logging

Signed-off-by: Yash Sharma <[email protected]>

* 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 <[email protected]>

* added some comments for usage of decider

Signed-off-by: Yash Sharma <[email protected]>

* renamed LogAllCall to LogStartAndFinishCall

Signed-off-by: Yash Sharma <[email protected]>

* remove unused comments

Signed-off-by: Yash Sharma <[email protected]>

More comments removal

Signed-off-by: Yash Sharma <[email protected]>

* modified the examples in the provider

Signed-off-by: Yash Sharma <[email protected]>
* comment some tests and add a lock

Signed-off-by: Yash Sharma <[email protected]>

* Added the latest version of grpc middleware

Signed-off-by: Yash Sharma <[email protected]>

* Added the new version of middleware commit for importing the new API for logging in the example tests

Signed-off-by: Yash Sharma <[email protected]>

* more changes

Signed-off-by: Yash Sharma <[email protected]>

* Added some locks and checks

Signed-off-by: Yash Sharma <[email protected]>

* added some changes as suggested by reviewer

Signed-off-by: Yash Sharma <[email protected]>

* Extra changes

Signed-off-by: Yash Sharma <[email protected]>

* Renamed the baseMockLogger to sharedResults and added some more locks

Signed-off-by: Yash Sharma <[email protected]>

* 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 <[email protected]>
* make ratelimit interface context aware.

* cleanup ratelimit testcases
* Export `SplitMethodName` and `StreamRPCType` function

* Add skip interceptor

* Fix lint

* Apply suggestions from code review

Co-authored-by: Bartlomiej Plotka <[email protected]>

* Use allowlist

* Add skip interceptor into README

Co-authored-by: Bartlomiej Plotka <[email protected]>
* 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
iamrajiv and others added 28 commits May 27, 2021 19:32
* added dependabot

* Update client.go

* Update server.go

* Delete dependabot.yml
* v2: Add support for the phuslog logging provider

* Update providers/phuslog/logger.go

fallback to error when no level is detected.
Add unknown level With to log and log error message
`Logger.Log` calls a zap's log function inside of this function.
As a result, caller annotations always point `logger.go`.

This commit adds caller skip so that each log message has the
correct annotation.
* removed previous copyright header

Signed-off-by: Yash Sharma <[email protected]>

added a copyright header check in the lint target of Makefile

Signed-off-by: Yash Sharma <[email protected]>

modified copyright headers

Signed-off-by: Yash Sharma <[email protected]>

added efficientgo/copyright to bingo

Signed-off-by: Yash Sharma <[email protected]>

used bingo makefile commands for running copyright target

Signed-off-by: Yash Sharma <[email protected]>

reverted copyright headers

Signed-off-by: Yash Sharma <[email protected]>

renamed copyright naming

Signed-off-by: Yash Sharma <[email protected]>

renamed copyright headers

Signed-off-by: Yash Sharma <[email protected]>

added efficientgo/copyright in go mod

Signed-off-by: Yash Sharma <[email protected]>

Signed-off-by: Yash Sharma <[email protected]>

ran go mod for openn metrics

Signed-off-by: Yash Sharma <[email protected]>

* rearranged some commands

Signed-off-by: Yash Sharma <[email protected]>

* fix extra line at end

Signed-off-by: Yash Sharma <[email protected]>
* update grpc to v1.37.0

Signed-off-by: Yash Sharma <[email protected]>

* updated other dependencies

Signed-off-by: Yash Sharma <[email protected]>

* updated some issues due to err-check

Signed-off-by: Yash Sharma <[email protected]>

* Added mustEmbedUnimplementedGreeterServer for upstreaming changes from grpc examples package

Signed-off-by: Yash Sharma <[email protected]>

* renamed tocken to token

Signed-off-by: Yash Sharma <[email protected]>

* Apply suggestions from code review

Signed-off-by: Yash Sharma <[email protected]>

* make lint

Signed-off-by: Yash Sharma <[email protected]>

* make lint changes from prev projects

Signed-off-by: Yash Sharma <[email protected]>
* Add all-validator support

* Use single argument instead of variadic

Since we are working on v2, keeping the signature
compatible with v1 is not actually necessary.

Use a clearer single argument instead. Also add comments
for the new argument.
…tem#390)

* removed backoffutils and added the files to retry package

Signed-off-by: Yash Sharma <[email protected]>

* linting changes

Signed-off-by: Yash Sharma <[email protected]>

* move the jitter up function

Signed-off-by: Yash Sharma <[email protected]>

* renamed exponentbase2 to private

Signed-off-by: Yash Sharma <[email protected]>

* renamed the test package to retry, making it internal

Signed-off-by: Yash Sharma <[email protected]>
…bility. (grpc-ecosystem#394)

* Removed tags; Simplified interceptor code; Added logging fields editability.

Fixes grpc-ecosystem#382

Signed-off-by: Bartlomiej Plotka <[email protected]>

Removed tags; Simplified interceptor code; Added logging fields editability.

Fixes grpc-ecosystem#382

Signed-off-by: Bartlomiej Plotka <[email protected]>

Fixed tests.

Signed-off-by: Bartlomiej Plotka <[email protected]>

Fixed open metrics test.

Signed-off-by: Bartlomiej Plotka <[email protected]>

* Addressed comments.

Signed-off-by: Bartlomiej Plotka <[email protected]>
* 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 <[email protected]>

* Remove kv package

* Remove keyvalue

Co-authored-by: Yash Sharma <[email protected]>
* Add opentracing provider

* Add copyright

* Fix format

* Fix copyright
* Added logr as logging Library provider

Closes grpc-ecosystem#473
Closes grpc-ecosystem#430

* fixed formatting
@google-cla
Copy link

google-cla bot commented Mar 8, 2023

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.

@aimuz aimuz closed this Mar 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

v2: Move metautils to it's own package