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

Add support for docker containers restart in loki.source.docker #742

Merged
merged 5 commits into from
May 8, 2024

Conversation

wildum
Copy link
Contributor

@wildum wildum commented May 2, 2024

PR Description

The loki.source.docker does not attempt to re-collect logs from a docker container target that has exited. The only way to continue collecting the logs from the target is to remove the target from the component and add it again.

This means that if a target is terminated and restarted within a discovery.docker interval, it will not be removed from the loki.source.docker component and the logs won't be collected anymore.

This fix adds a retry strategy that attempts to collect logs from the current running targets every 5 seconds. If the task is already running it does not do anything. If a task was stopped it will be restarted.

Which issue(s) this PR fixes

Fixes #278

PR Checklist

  • CHANGELOG.md updated
  • Tests updated

@wildum wildum added the backport-to-agent PR should be backported to the agent repo. label May 2, 2024
Copy link
Contributor

@thampiotr thampiotr left a comment

Choose a reason for hiding this comment

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

Got one concern about the resource leak, but it looks okay otherwise!

Copy link
Contributor

@thampiotr thampiotr left a comment

Choose a reason for hiding this comment

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

One comment, otherwise LGTM

internal/component/loki/source/docker/runner.go Outdated Show resolved Hide resolved
@wildum wildum force-pushed the fix-loki-source-docker-logs-restart branch from 160f1c7 to ddd8332 Compare May 7, 2024 14:58
@wildum wildum merged commit 5d777a5 into main May 8, 2024
19 checks passed
@wildum wildum deleted the fix-loki-source-docker-logs-restart branch May 8, 2024 15:59
github-actions bot pushed a commit that referenced this pull request May 8, 2024
* Attempt to restart docker target for loki source at a fixed interval to support containers restart

* add test

* add missing target cleanup

* improve test readability

* rename config option

(cherry picked from commit 5d777a5)
wildum added a commit that referenced this pull request May 8, 2024
#803)

* Attempt to restart docker target for loki source at a fixed interval to support containers restart

* add test

* add missing target cleanup

* improve test readability

* rename config option

(cherry picked from commit 5d777a5)

Co-authored-by: William Dumont <william.dumont@grafana.com>
@wildum wildum mentioned this pull request May 16, 2024
2 tasks
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 8, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
backport release/v1.1 backport-to-agent PR should be backported to the agent repo. frozen-due-to-age
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Logs are not forwarded to Loki after target docker container is restarted
2 participants