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

Added request logging for grpc

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

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

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

added flags for enabling/disabling request logging for grpc

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

nitpicks due to make docs

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

added a changelog

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

nitpicks

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

configure option for logging

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

changelog nitpick

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

added changelog

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

more nitpicks

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

renamed requestLoggingDecision to reqLogDecision and some nitcpicks

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

Added the check of reusing the request-id if present

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

make docs nitpick

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

Added the default level of debug and error codes for logging

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

Add the levels for logging

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

Added a YAML flag for request logging config

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

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

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

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

Added new config files for YAML parsing

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

changelog nitpick

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

linting nitpicks

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

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

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

docs nitpicks

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

bug fix for reverse logging condition

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

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

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

removed gophercloud from go.mod

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

self addressed comments

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

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

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

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

Renaming of functions

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

Removed indentation and simplified else conditions

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

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

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

make docs nits

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

modify changelog

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

modified the signature of setup function to the original one

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

removed the message for the flags

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

make docs nits

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

Added auto generation scripts for req logging

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

removed request logging from compactor

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

remove verbose warn messages

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

changed pass by value to pass by reference

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

removed occurence of os.Exit

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

make docs nits

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

revert compact.go to master

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

rename ReqlogConfig to RequestConfig

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

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

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

nits

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

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

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

remove a line from the flags

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

remove a deceptive comment

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

changed the var name to small caps

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

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

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

move changelog entry to unreleased tag

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

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

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

Added request logging for grpc

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

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

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

added flags for enabling/disabling request logging for grpc

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

nitpicks due to make docs

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

added a changelog

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

nitpicks

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

configure option for logging

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

changelog nitpick

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

added changelog

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

more nitpicks

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

renamed requestLoggingDecision to reqLogDecision and some nitcpicks

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

Added the check of reusing the request-id if present

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

make docs nitpick

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

Added the default level of debug and error codes for logging

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

Add the levels for logging

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

Added a YAML flag for request logging config

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

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

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

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

Added new config files for YAML parsing

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

changelog nitpick

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

linting nitpicks

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

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

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

docs nitpicks

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

bug fix for reverse logging condition

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

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

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

removed gophercloud from go.mod

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

self addressed comments

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

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

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

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

Renaming of functions

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

Removed indentation and simplified else conditions

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

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

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

make docs nits

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

modify changelog

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

modified the signature of setup function to the original one

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

removed the message for the flags

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

make docs nits

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

Added auto generation scripts for req logging

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

removed request logging from compactor

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

remove verbose warn messages

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

changed pass by value to pass by reference

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

removed occurence of os.Exit

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

make docs nits

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

revert compact.go to master

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

rename ReqlogConfig to RequestConfig

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

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

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

nits

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

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

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

remove a line from the flags

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

remove a deceptive comment

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

changed the var name to small caps

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

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

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

move changelog entry to unreleased tag

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

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

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

* added changelog entry

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

* changed changelog

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

* renamed decidehttpflag and decidegrpcflag

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

* addressed reviewers comments

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