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

Remove tenant header logs in store gateway #6552

Merged
merged 2 commits into from
Jul 26, 2023

Conversation

yeya24
Copy link
Contributor

@yeya24 yeya24 commented Jul 21, 2023

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

Changes

It is very verbose to have a separate log line to log tenant for every requests in store gateway, especially for Cortex which uses store gateway as a library. It doesn't propagate Thanos header at all so no need to have the warning log for missing headers.

Verification

Signed-off-by: Ben Ye <benye@amazon.com>
@sonatype-lift
Copy link

sonatype-lift bot commented Jul 21, 2023

Sonatype Lift is retiring

Sonatype Lift will be retiring on Sep 12, 2023, with its analysis stopping on Aug 12, 2023. We understand that this news may come as a disappointment, and Sonatype is committed to helping you transition off it seamlessly. If you’d like to retain your data, please export your issues from the web console.
We are extremely grateful and thank you for your support over the years.

📖 Read about the impacts and timeline

Signed-off-by: Ben Ye <benye@amazon.com>
Copy link
Member

@saswatamcode saswatamcode left a comment

Choose a reason for hiding this comment

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

LGTM!
We might need to add this back in the future, if we start to rely on tenant labels heavily here. cc: @jacobbaungard

@yeya24
Copy link
Contributor Author

yeya24 commented Jul 24, 2023

We might need to add this back in the future, if we start to rely on tenant labels heavily here

I think we can add tenant header along side with other logs. But warning log sounds unnecessary to me. Same for the debugging log for the tenant id itself. It really doesn't provide much information overall.
If really needed, can we add that log in querier instead? Store Gateway should be fine as long as we know the gRPC metadata propagation works as expected.

@saswatamcode
Copy link
Member

saswatamcode commented Jul 25, 2023

If really needed, can we add that log in querier instead?

Probably. Right now we aren't using this tenant information in StoreGW.

@GiedriusS GiedriusS merged commit 510c054 into thanos-io:main Jul 26, 2023
GiedriusS pushed a commit to vinted/thanos that referenced this pull request Jul 27, 2023
* remove tenant header logs which are too verbose

Signed-off-by: Ben Ye <benye@amazon.com>

* keep the debug level log

Signed-off-by: Ben Ye <benye@amazon.com>

---------

Signed-off-by: Ben Ye <benye@amazon.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants