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

Fix event time calculation on docker events #24318

Merged
merged 5 commits into from
Mar 3, 2021

Conversation

ChrsMark
Copy link
Member

@ChrsMark ChrsMark commented Mar 3, 2021

What does this PR do?

This PR fixes how lastValidTimestamp is calculated based on the event's time we get from docker API.

Docker events are reported like:

{"status":"exec_die","id":"d960ece07fde79828ba6f228a1930ff36c2bf54d363d6ed833f31eee4c0e0912","from":"busybox","Type":"container","Action":"exec_die","Actor":{"ID":"d960ece07fde79828ba6f228a1930ff36c2bf54d363d6ed833f31eee4c0e0912","Attributes":{"execID":"820942d4d8831ed11efca865f5b7ef776244f24b05a6fb6f108b8050707cfb76","exitCode":"0","image":"busybox","name":"test42"}},"scope":"local","time":1614780249,"timeNano":1614780249008388154}

time and timeNano report the same timestamp but with different accuracy.

In this, we should only use Time or TimeNano but not together, otherwise it will result in a timestamp points to the future which will lead to not valid restarts of the watcher at

Since: lastValidTimestamp.Format(time.RFC3339Nano),
. --since flag will point to the future making it unable to catch any events from that time on.

Why is it important?

To be able to recover from restarts like when docker daemon is getting restarted.

How to manually test this PR

  1. Start filebeat with docker autodiscover enabled:
filebeat.autodiscover:
  providers:
    - type: docker
      hints.enabled: true
      hints.default_config:
        type: container
        paths:
          - /var/log/containers/*-${data.container.id}.log 

NOTE: verify that log's path is valid on your system. If not check if /var/lib/docker/containers/*/*${data.container.id}*.log is the valid path.
2. run a dummy container: docker run --restart=always --name test42 -d busybox sh -c "while true; do $(echo date); sleep 1; done"
3. Verify logs flow in ES
3. Restart the docker daemon with sudo systemctl restart docker
4. Verify that logs keep flowing in ES after the restart

Logs

2021-03-03T14:12:36.729Z	DEBUG	[docker]	docker/watcher.go:278	Got a new docker event: {kill d960ece07fde79828ba6f228a1930ff36c2bf54d363d6ed833f31eee4c0e0912 busybox container kill {d960ece07fde79828ba6f228a1930ff36c2bf54d363d6ed833f31eee4c0e0912 map[image:busybox name:test42 signal:15]} local 1614780756 1614780756729496625}


2021-03-03T14:12:46.822Z	DEBUG	[docker]	docker/watcher.go:278	Got a new docker event: {kill d960ece07fde79828ba6f228a1930ff36c2bf54d363d6ed833f31eee4c0e0912 busybox container kill {d960ece07fde79828ba6f228a1930ff36c2bf54d363d6ed833f31eee4c0e0912 map[image:busybox name:test42 signal:9]} local 1614780766 1614780766821876317}
2021-03-03T14:12:47.022Z	DEBUG	[docker]	docker/watcher.go:278	Got a new docker event: {die d960ece07fde79828ba6f228a1930ff36c2bf54d363d6ed833f31eee4c0e0912 busybox container die {d960ece07fde79828ba6f228a1930ff36c2bf54d363d6ed833f31eee4c0e0912 map[exitCode:137 image:busybox name:test42]} local 1614780767 1614780767022110976}
2021-03-03T14:12:47.483Z	DEBUG	[docker]	docker/watcher.go:278	Got a new docker event: {stop d960ece07fde79828ba6f228a1930ff36c2bf54d363d6ed833f31eee4c0e0912 busybox container stop {d960ece07fde79828ba6f228a1930ff36c2bf54d363d6ed833f31eee4c0e0912 map[image:busybox name:test42]} local 1614780767 1614780767482947600}
2021-03-03T14:12:47.495Z	ERROR	[docker]	docker/watcher.go:299	Error watching for docker events: unexpected EOF
2021-03-03T14:12:48.495Z	DEBUG	[docker]	docker/watcher.go:264	Fetching events since 2021-03-03 14:12:47.4829476 +0000 UTC
2021-03-03T14:12:48.959Z	DEBUG	[docker]	docker/watcher.go:278	Got a new docker event: {start d960ece07fde79828ba6f228a1930ff36c2bf54d363d6ed833f31eee4c0e0912 busybox container start {d960ece07fde79828ba6f228a1930ff36c2bf54d363d6ed833f31eee4c0e0912 map[image:busybox name:test42]} local 1614780768 1614780768842421505}
2021-03-03T14:12:48.959Z	DEBUG	[docker]	docker/watcher.go:370	List containers
2021-03-03T14:12:48.963Z	DEBUG	[docker]	docker/docker.go:237	Container d960ece07fde79828ba6f228a1930ff36c2bf54d363d6ed833f31eee4c0e0912 is restarting, aborting pending stop

Signed-off-by: chrismark <chrismarkou92@gmail.com>
@ChrsMark ChrsMark added bug containers Related to containers use case labels Mar 3, 2021
@ChrsMark ChrsMark requested a review from jsoriano March 3, 2021 14:28
@ChrsMark ChrsMark self-assigned this Mar 3, 2021
@botelastic botelastic bot added the needs_team Indicates that the issue/PR needs a Team:* label label Mar 3, 2021
Signed-off-by: chrismark <chrismarkou92@gmail.com>
Copy link
Member

@jsoriano jsoriano left a comment

Choose a reason for hiding this comment

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

Good catch, thanks for the fix!

CHANGELOG.next.asciidoc Outdated Show resolved Hide resolved
@@ -276,7 +276,7 @@ func (w *watcher) watch() {
select {
case event := <-events:
w.log.Debugf("Got a new docker event: %v", event)
lastValidTimestamp = time.Unix(event.Time, event.TimeNano)
lastValidTimestamp = time.Unix(0, event.TimeNano)
Copy link
Member

Choose a reason for hiding this comment

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

Good one 👍

@jsoriano jsoriano added the Team:Integrations Label for the Integrations team label Mar 3, 2021
@elasticmachine
Copy link
Collaborator

Pinging @elastic/integrations (Team:Integrations)

@botelastic botelastic bot removed the needs_team Indicates that the issue/PR needs a Team:* label label Mar 3, 2021
ChrsMark and others added 2 commits March 3, 2021 17:30
Co-authored-by: Jaime Soriano Pastor <jaime.soriano@elastic.co>
Co-authored-by: Jaime Soriano Pastor <jaime.soriano@elastic.co>
@elasticmachine
Copy link
Collaborator

elasticmachine commented Mar 3, 2021

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview

Expand to view the summary

Build stats

  • Build Cause: Pull request #24318 updated

  • Start Time: 2021-03-03T15:53:04.475+0000

  • Duration: 53 min 32 sec

  • Commit: 2b43301

Test stats 🧪

Test Results
Failed 0
Passed 46636
Skipped 4939
Total 51575

Trends 🧪

Image of Build Times

Image of Tests

💚 Flaky test report

Tests succeeded.

Expand to view the summary

Test stats 🧪

Test Results
Failed 0
Passed 46636
Skipped 4939
Total 51575

Signed-off-by: chrismark <chrismarkou92@gmail.com>
@ChrsMark ChrsMark added test-plan Add this PR to be manual test plan needs_backport PR is waiting to be backported to other branches. v7.12.0 v7.13.0 labels Mar 3, 2021
@ChrsMark ChrsMark merged commit 8b673e6 into elastic:master Mar 3, 2021
ChrsMark added a commit to ChrsMark/beats that referenced this pull request Mar 3, 2021
@ChrsMark ChrsMark removed the needs_backport PR is waiting to be backported to other branches. label Mar 3, 2021
ChrsMark added a commit to ChrsMark/beats that referenced this pull request Mar 3, 2021
ChrsMark added a commit to ChrsMark/beats that referenced this pull request Mar 3, 2021
ChrsMark added a commit that referenced this pull request Mar 3, 2021
@andresrc andresrc added the test-plan-added This PR has been added to the test plan label Apr 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug containers Related to containers use case Team:Integrations Label for the Integrations team test-plan Add this PR to be manual test plan test-plan-added This PR has been added to the test plan v7.11.2 v7.12.0 v7.13.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants