-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
Restart Docker target if stopped. #5280
Conversation
Summary: The Docker target would not reconnect to a container if it restarted and kept the same ID. This should introduces a `startIfNotRunning` method that will be called to restart scraping the container's logs. Fixes grafana#5259
func (t *Target) startIfNotRunning() { | ||
if t.running.CAS(false, true) { | ||
ctx, cancel := context.WithCancel(context.Background()) | ||
t.cancel = cancel |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the one bit that would bother me. However, I claim it's thread safe as long as it's threadsafe to access t.cancel
concurrently. It will never be nil
for Stop
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, two non-blocking suggestions
t, ok := tg.targets[id] | ||
if ok { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is an opportunity to use the idiom
if t, ok := tg.targets[id]; ok {
level.Debug(tg.logger).Log("msg", "container target already exists", "container", id)
t.startIfNotRunning()
return nil
}
What this PR does / why we need it:
The Docker target would not reconnect to a container if it restarted and
kept the same ID. This should introduces a
startIfNotRunning
methodthat will be called to restart scraping the container's logs.
Which issue(s) this PR fixes:
Fixes #5259
Special notes for your reviewer:
Checklist
CHANGELOG.md
about the changes.