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

Upstream changes for PR #3361 - Added request logging for gRPC and HTTP in the server side #3862

Merged
merged 5 commits into from
Mar 4, 2021

Conversation

yashrsharma44
Copy link
Contributor

I had deleted my thanos fork for some reason, and so I was not able to push the changes in the same branch.
Created another PR to reflect the changes for #3361

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

Added request logging for grpc

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

changed the logging option to log start and finish of a call

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

added flags for enabling/disabling request logging for grpc

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

nitpicks due to make docs

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

added a changelog

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

nitpicks

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

configure option for logging

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

changelog nitpick

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

added changelog

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

more nitpicks

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

renamed requestLoggingDecision to reqLogDecision and some nitcpicks

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

Added the check of reusing the request-id if present

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

make docs nitpick

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

Added the default level of debug and error codes for logging

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

Add the levels for logging

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

Added a YAML flag for request logging config

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

Changed the setup signature to accept a reqlog param to be shared among all the components of Thanos. Changed the signature of decision for logging, and pushed the new yaml config among all the middleware

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

Updated the grpc middleware package to include the latest signature of the decider

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

Added new config files for YAML parsing

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

changelog nitpick

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

linting nitpicks

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

Added deprecation warning and added a function to reroute logging through one of the flags

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

docs nitpicks

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

bug fix for reverse logging condition

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

fixed some bugs for evaluating the options and added a warning for deprecating flag of log.request.decision

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

removed gophercloud from go.mod

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

self addressed comments

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

* Added a dummy variable which was used by the, to be deprecated flag of request logging
* Added a warning that request logging is disabled for tools
* Removed a flag which is to be deprecated in the next release

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

* Added a fail-first approach if the request logging is incorrectly enabled.

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

Renaming of functions

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

Removed indentation and simplified else conditions

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

break down the yaml struct for grpc and http into its individual configs

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

make docs nits

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

modify changelog

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

modified the signature of setup function to the original one

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

removed the message for the flags

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

make docs nits

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

Added auto generation scripts for req logging

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

removed request logging from compactor

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

remove verbose warn messages

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

changed pass by value to pass by reference

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

removed occurence of os.Exit

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

make docs nits

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

revert compact.go to master

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

rename ReqlogConfig to RequestConfig

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

Added a validation check so that all the configs are checked before a component starts

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

nits

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

Modify the message for request.logging flag for having a default value as null

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

remove a line from the flags

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

remove a deceptive comment

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

changed the var name to small caps

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

change errors.Errorf to Wrapf for providing context to error message

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

move changelog entry to unreleased tag

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

changed request.logging to `request.logging` in the flags

Signed-off-by: Yash Sharma <[email protected]>
@yashrsharma44 yashrsharma44 force-pushed the grpc-request-logging branch 2 times, most recently from 9a074b9 to a9b0cc7 Compare March 4, 2021 10:13
Signed-off-by: Yash Sharma <[email protected]>
@yashrsharma44 yashrsharma44 force-pushed the grpc-request-logging branch from a9b0cc7 to 53d1625 Compare March 4, 2021 10:18
@bwplotka bwplotka requested review from kakkoyun and bwplotka March 4, 2021 11:08
Copy link
Member

@bwplotka bwplotka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some review during Contributor Hours 🤗

Good work!

CHANGELOG.md Outdated Show resolved Hide resolved
cmd/thanos/query.go Outdated Show resolved Hide resolved
cmd/thanos/query.go Outdated Show resolved Hide resolved
cmd/thanos/query.go Outdated Show resolved Hide resolved
cmd/thanos/query.go Outdated Show resolved Hide resolved
pkg/logging/grpc.go Outdated Show resolved Hide resolved
pkg/logging/grpc.go Outdated Show resolved Hide resolved
pkg/logging/grpc.go Show resolved Hide resolved
pkg/logging/grpc.go Show resolved Hide resolved
pkg/logging/http.go Outdated Show resolved Hide resolved
Copy link
Member

@kakkoyun kakkoyun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 🚀 Thanks for the great work 🎉
Let's merge it and test it and address the issues until we cut the next RC.

@kakkoyun kakkoyun merged commit befb025 into thanos-io:main Mar 4, 2021
andrejbranch pushed a commit to andrejbranch/thanos that referenced this pull request Mar 11, 2021
…PC and HTTP in the server side (thanos-io#3862)

* Added request logging for grpc

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

Added request logging for grpc

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

changed the logging option to log start and finish of a call

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

added flags for enabling/disabling request logging for grpc

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

nitpicks due to make docs

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

added a changelog

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

nitpicks

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

configure option for logging

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

changelog nitpick

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

added changelog

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

more nitpicks

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

renamed requestLoggingDecision to reqLogDecision and some nitcpicks

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

Added the check of reusing the request-id if present

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

make docs nitpick

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

Added the default level of debug and error codes for logging

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

Add the levels for logging

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

Added a YAML flag for request logging config

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

Changed the setup signature to accept a reqlog param to be shared among all the components of Thanos. Changed the signature of decision for logging, and pushed the new yaml config among all the middleware

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

Updated the grpc middleware package to include the latest signature of the decider

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

Added new config files for YAML parsing

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

changelog nitpick

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

linting nitpicks

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

Added deprecation warning and added a function to reroute logging through one of the flags

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

docs nitpicks

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

bug fix for reverse logging condition

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

fixed some bugs for evaluating the options and added a warning for deprecating flag of log.request.decision

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

removed gophercloud from go.mod

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

self addressed comments

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

* Added a dummy variable which was used by the, to be deprecated flag of request logging
* Added a warning that request logging is disabled for tools
* Removed a flag which is to be deprecated in the next release

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

* Added a fail-first approach if the request logging is incorrectly enabled.

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

Renaming of functions

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

Removed indentation and simplified else conditions

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

break down the yaml struct for grpc and http into its individual configs

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

make docs nits

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

modify changelog

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

modified the signature of setup function to the original one

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

removed the message for the flags

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

make docs nits

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

Added auto generation scripts for req logging

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

removed request logging from compactor

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

remove verbose warn messages

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

changed pass by value to pass by reference

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

removed occurence of os.Exit

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

make docs nits

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

revert compact.go to master

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

rename ReqlogConfig to RequestConfig

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

Added a validation check so that all the configs are checked before a component starts

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

nits

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

Modify the message for request.logging flag for having a default value as null

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

remove a line from the flags

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

remove a deceptive comment

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

changed the var name to small caps

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

change errors.Errorf to Wrapf for providing context to error message

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

move changelog entry to unreleased tag

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

changed request.logging to `request.logging` in the flags

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

* added changelog entry

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

* changed changelog

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

* renamed decidehttpflag and decidegrpcflag

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

* addressed reviewers comments

Signed-off-by: Yash Sharma <[email protected]>
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.

3 participants