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 <bwplotka@gmail.com>
Signed-off-by: Bartlomiej Plotka <bwplotka@gmail.com>
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. (grpc-ecosystem#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>
…#292)

* 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>
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 <bwplotka@gmail.com>

* Fixed linter detected issues.

Signed-off-by: Bartlomiej Plotka <bwplotka@gmail.com>

* Modify the require_clean_work_tree function (grpc-ecosystem#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 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 <johan.brandhorst@gmail.com>

Co-authored-by: Johan Brandhorst-Satzkorn <johan.brandhorst@gmail.com>
…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 <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
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 <yashrsharma44@gmail.com>

added a copyright header check in the lint target of Makefile

Signed-off-by: Yash Sharma <yashrsharma44@gmail.com>

modified copyright headers

Signed-off-by: Yash Sharma <yashrsharma44@gmail.com>

added efficientgo/copyright to bingo

Signed-off-by: Yash Sharma <yashrsharma44@gmail.com>

used bingo makefile commands for running copyright target

Signed-off-by: Yash Sharma <yashrsharma44@gmail.com>

reverted copyright headers

Signed-off-by: Yash Sharma <yashrsharma44@gmail.com>

renamed copyright naming

Signed-off-by: Yash Sharma <yashrsharma44@gmail.com>

renamed copyright headers

Signed-off-by: Yash Sharma <yashrsharma44@gmail.com>

added efficientgo/copyright in go mod

Signed-off-by: Yash Sharma <yashrsharma44@gmail.com>

Signed-off-by: Yash Sharma <yashrsharma44@gmail.com>

ran go mod for openn metrics

Signed-off-by: Yash Sharma <yashrsharma44@gmail.com>

* rearranged some commands

Signed-off-by: Yash Sharma <yashrsharma44@gmail.com>

* fix extra line at end

Signed-off-by: Yash Sharma <yashrsharma44@gmail.com>
* update grpc to v1.37.0

Signed-off-by: Yash Sharma <yashrsharma44@gmail.com>

* updated other dependencies

Signed-off-by: Yash Sharma <yashrsharma44@gmail.com>

* updated some issues due to err-check

Signed-off-by: Yash Sharma <yashrsharma44@gmail.com>

* Added mustEmbedUnimplementedGreeterServer for upstreaming changes from grpc examples package

Signed-off-by: Yash Sharma <yashrsharma44@gmail.com>

* renamed tocken to token

Signed-off-by: Yash Sharma <yashrsharma44@gmail.com>

* Apply suggestions from code review

Signed-off-by: Yash Sharma <yashrsharma44@gmail.com>

* make lint

Signed-off-by: Yash Sharma <yashrsharma44@gmail.com>

* make lint changes from prev projects

Signed-off-by: Yash Sharma <yashrsharma44@gmail.com>
* 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 <yashrsharma44@gmail.com>

* linting changes

Signed-off-by: Yash Sharma <yashrsharma44@gmail.com>

* move the jitter up function

Signed-off-by: Yash Sharma <yashrsharma44@gmail.com>

* renamed exponentbase2 to private

Signed-off-by: Yash Sharma <yashrsharma44@gmail.com>

* renamed the test package to retry, making it internal

Signed-off-by: Yash Sharma <yashrsharma44@gmail.com>
…bility. (grpc-ecosystem#394)

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

Fixes grpc-ecosystem#382

Signed-off-by: Bartlomiej Plotka <bwplotka@gmail.com>

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

Fixes grpc-ecosystem#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
* Added logr as logging Library provider

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

* fixed formatting
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>
@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