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

prober: Remove spam of changing probe status #5051

Merged
merged 4 commits into from Jan 12, 2022

Conversation

JuozasVainauskas
Copy link
Contributor

@JuozasVainauskas JuozasVainauskas commented Jan 10, 2022

Issue: #5036

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

Changes

Remove constant spam of "changing probe status" if status of the probe was set to the same value as before

Verification

I ran make quickstart without my changes and saw that "changing probe status" messages are spammed even though probe status was set to the same value as before. Then I added my changes, ran make quickstart again and got certain that these kind of logs do not repeat if probe status does not change to different value than before.

Signed-off-by: Juozas Vainauskas <vainauskasjuozas@gmail.com>
Signed-off-by: Juozas Vainauskas <vainauskasjuozas@gmail.com>
@JuozasVainauskas JuozasVainauskas marked this pull request as ready for review January 10, 2022 06:43
@@ -23,7 +24,8 @@ type InstrumentationProbe struct {
component component.Component
logger log.Logger

status *prometheus.GaugeVec
status *prometheus.GaugeVec
formerStatus string
Copy link
Member

@GiedriusS GiedriusS Jan 10, 2022

Choose a reason for hiding this comment

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

We need a lock here since Ready/NotReady could be called from multiple goroutines. Also, it's not really former, is it? It's the current status at the time of the function call thus it looks weird. I suggest being explicit here and renaming these two variables into statusMetric and statusString or something.

Signed-off-by: Juozas Vainauskas <vainauskasjuozas@gmail.com>
@pull-request-size pull-request-size bot added size/M and removed size/S labels Jan 10, 2022
Copy link
Member

@GiedriusS GiedriusS left a comment

Choose a reason for hiding this comment

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

Please run make go-format to fix the linter issues 👍

p.mu.Lock()
defer p.mu.Unlock()
if p.statusString != ready {
level.Info(p.logger).Log("msg", "changing probe status", "status", "ready")
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
level.Info(p.logger).Log("msg", "changing probe status", "status", "ready")
level.Info(p.logger).Log("msg", "changing probe status", "status", ready)

Signed-off-by: Juozas Vainauskas <vainauskasjuozas@gmail.com>
Copy link
Member

@GiedriusS GiedriusS left a comment

Choose a reason for hiding this comment

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

👍 thx

@GiedriusS GiedriusS merged commit 629885f into thanos-io:main Jan 12, 2022
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.

None yet

3 participants