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 constant spam of "changing probe status" #5036

Closed
GiedriusS opened this issue Jan 6, 2022 · 8 comments
Closed

Remove constant spam of "changing probe status" #5036

GiedriusS opened this issue Jan 6, 2022 · 8 comments

Comments

@GiedriusS
Copy link
Member

With Sidecar now changing its readiness status depending on whether it can establish a connection with Prometheus, there is spam of:

{"caller":"intrumentation.go:48","level":"info","msg":"changing probe status","status":"ready","ts":"2022-01-06T16:37:27.595400008Z"}

We shouldn't print this message if the status is set to the same value as the previous one.

@AvineshTripathi
Copy link

Hey @GiedriusS very new here is this something I can go for without knowing the codebase

@GiedriusS
Copy link
Member Author

Yep.

level.Info(p.logger).Log("msg", "changing probe status", "status", "ready")
here we need to introduce a new variable in InstrumentationProbe called ready or something. Like here:
ready atomic.Uint32
healthy atomic.Uint32
. And then check in Ready() if we weren't ready before. If true then print. Same with NotReady().

@AvineshTripathi
Copy link

okay i ll go for this please assign me

@AvineshTripathi
Copy link

Hey @GiedriusS I have done some changes but not sure if that is working can you tell how do I check it

@GiedriusS
Copy link
Member Author

Run the quickstart script or in some other way, or manually a pair of Thanos Query+Sidecar (& Prometheus). If everything is ok then you won't see a constant spam as in the original description.

@JuozasVainauskas
Copy link
Contributor

@AvineshTripathi are You still working on this and planning to finish it or can I open a PR with my fix for this issue?

@AvineshTripathi
Copy link

Hey @JuozasVainauskas I have done the changes but not able to set up in my local device so I think you should go for the PR i ll unaaisgn myself
P.S. sorry for the trouble

@GiedriusS
Copy link
Member Author

Fixed in #5051

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

3 participants