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

Logging: Simplify and adjust request logging configuration #4961

Open
matej-g opened this issue Dec 16, 2021 · 9 comments
Open

Logging: Simplify and adjust request logging configuration #4961

matej-g opened this issue Dec 16, 2021 · 9 comments

Comments

@matej-g
Copy link
Collaborator

matej-g commented Dec 16, 2021

As seems to stem from the discussion in #4934, request logging implementation and configuration seem to have a couple of confusing features, which might be a bit misleading for the users.

I'll try to summarize some of my and @philipgough's observations below:

  • There is no simple way to determine if the request logging for a component has taken effect - perhaps it would make sense to log a line with this information (e.g. "logging for XY has been enabled")
  • A configuration to log requests for particular server (HTTP, gRPC) on a component requires to specify a port, but it is not clear which port the user should specify and why this is required. Perhaps specifying the port is not necessary?
  • It is possible to configure a log level for the requests logs, but the logs still do not seem to appear until log.level=debug is used. I think normal user expectation would be to set the level and see the logs being logged at that level, without the need to set log.level=debug.
  • URL path configuration seems to be working on the basis of exact match, per the comment in Document request logging configuration #4934 (comment), but this is insufficient if URL parameters are passed as well.
@matej-g
Copy link
Collaborator Author

matej-g commented Dec 16, 2021

There's already #4956 to address the log level portion of this issue.

@wiardvanrij
Copy link
Member

LGTM, though it should get some more actionable items IMO :)

@stale
Copy link

stale bot commented Apr 17, 2022

Hello 👋 Looks like there was no activity on this issue for the last two months.
Do you mind updating us on the status? Is this still reproducible or needed? If yes, just comment on this PR or push a commit. Thanks! 🤗
If there will be no activity in the next two weeks, this issue will be closed (we can always reopen an issue if we need!). Alternatively, use remind command if you wish to be reminded at some point in future.

@B0go
Copy link
Contributor

B0go commented Apr 27, 2022

Hello, I am interested in doing my first contribution to the Thanos project and I think this issue may be a good one to start! I have a good knowledge on Thanos as an user and I still need to study the code and configure the dev environment... so it may take some time

@matej-g
Copy link
Collaborator Author

matej-g commented May 13, 2022

Hey @B0go, sorry for the late reply! Please go ahead, feel free to take your time to familiarize yourself with the code, and don't hesitate to ask if something's not clear 🙂

@stale
Copy link

stale bot commented Jul 31, 2022

Hello 👋 Looks like there was no activity on this issue for the last two months.
Do you mind updating us on the status? Is this still reproducible or needed? If yes, just comment on this PR or push a commit. Thanks! 🤗
If there will be no activity in the next two weeks, this issue will be closed (we can always reopen an issue if we need!). Alternatively, use remind command if you wish to be reminded at some point in future.

@stale stale bot added the stale label Jul 31, 2022
@GiedriusS GiedriusS removed the stale label Aug 23, 2022
@stale
Copy link

stale bot commented Nov 13, 2022

Hello 👋 Looks like there was no activity on this issue for the last two months.
Do you mind updating us on the status? Is this still reproducible or needed? If yes, just comment on this PR or push a commit. Thanks! 🤗
If there will be no activity in the next two weeks, this issue will be closed (we can always reopen an issue if we need!). Alternatively, use remind command if you wish to be reminded at some point in future.

@stale stale bot added the stale label Nov 13, 2022
@theluckiesthuman
Copy link

Hey @matej-g. I want to contribute to Thanos. As no one is working on this issue, I am working on it.

@stale stale bot removed the stale label Dec 14, 2022
@ReticentFacade
Copy link

@matej-g, I'm willing to work on this issue. Could you please share the steps to reproduce?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants