Skip to content

[8.19](backport #51688) add_docker_metadata: fix data races in lazy cgroup cache init#51844

Merged
orestisfl merged 2 commits into
8.19from
mergify/bp/8.19/pr-51688
Jul 10, 2026
Merged

[8.19](backport #51688) add_docker_metadata: fix data races in lazy cgroup cache init#51844
orestisfl merged 2 commits into
8.19from
mergify/bp/8.19/pr-51688

Conversation

@mergify

@mergify mergify Bot commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Proposed commit message

add_docker_metadata: fix data races in lazy cgroup cache init

add_docker_metadata can run as a beat-level (global) processor, in
which case a single instance is shared by every pipeline client and
its Run method is invoked concurrently. Two data races existed on
that path, both reachable by default because match_pids defaults to
["process.pid", "process.parent.pid"].

lazyCgroupCacheInit guarded the cache with a plain
"if d.cgroups == nil" check-then-set. Concurrent Run calls raced on
the pointer, and two callers could each build a cache and start its
own janitor goroutine, leaking a goroutine and orphaning a cache.
Replace the field with an atomic.Pointer[common.Cache] initialized
exactly once through sync.Once, and load it everywhere it is read.

common.Cache.Get held only the read lock, but get updates the
element's last access time on access-expiry caches (the cgroup cache
is one), so concurrent Get calls mutated the same element under a
shared lock. Take the exclusive lock in Get.

Checklist

  • My code follows the style guidelines of this project
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have made corresponding change to the default configuration files
  • I have added tests that prove my fix is effective or that my feature works. Where relevant, I have used the stresstest.sh script to run them under stress conditions and race detector to verify their stability.
  • I have added an entry in ./changelog/fragments using the changelog tool.

Disruptive User Impact

None.

How to test this PR locally

Regression test

Run the new test with the stress test script:

./script/stresstest.sh --race ./libbeat/processors/add_docker_metadata '^TestMatchPIDsConcurrent$' -p 32

End-to-end reproduction

A running Docker daemon is required, otherwise Run returns early before reaching the cgroup cache.

  1. Create input files so several harvesters publish concurrently at startup:
mkdir -p /tmp/dmrepro/logs /tmp/dmrepro/out
for i in $(seq 1 24); do yes "line" | head -n 400 > /tmp/dmrepro/logs/f${i}.log; done
  1. Write /tmp/dmrepro/filebeat.yml with 24 filestream inputs (each a distinct id), an add_fields that sets process.pid and add_docker_metadata as a global processor:
Config
filebeat.inputs:
  - type: filestream
    id: gen-1
    paths: ["/tmp/dmrepro/logs/f1.log"]
  - type: filestream
    id: gen-2
    paths: ["/tmp/dmrepro/logs/f2.log"]
  - type: filestream
    id: gen-3
    paths: ["/tmp/dmrepro/logs/f3.log"]
  - type: filestream
    id: gen-4
    paths: ["/tmp/dmrepro/logs/f4.log"]
  - type: filestream
    id: gen-5
    paths: ["/tmp/dmrepro/logs/f5.log"]
  - type: filestream
    id: gen-6
    paths: ["/tmp/dmrepro/logs/f6.log"]
  - type: filestream
    id: gen-7
    paths: ["/tmp/dmrepro/logs/f7.log"]
  - type: filestream
    id: gen-8
    paths: ["/tmp/dmrepro/logs/f8.log"]
  - type: filestream
    id: gen-9
    paths: ["/tmp/dmrepro/logs/f9.log"]
  - type: filestream
    id: gen-10
    paths: ["/tmp/dmrepro/logs/f10.log"]
  - type: filestream
    id: gen-11
    paths: ["/tmp/dmrepro/logs/f11.log"]
  - type: filestream
    id: gen-12
    paths: ["/tmp/dmrepro/logs/f12.log"]
  - type: filestream
    id: gen-13
    paths: ["/tmp/dmrepro/logs/f13.log"]
  - type: filestream
    id: gen-14
    paths: ["/tmp/dmrepro/logs/f14.log"]
  - type: filestream
    id: gen-15
    paths: ["/tmp/dmrepro/logs/f15.log"]
  - type: filestream
    id: gen-16
    paths: ["/tmp/dmrepro/logs/f16.log"]
  - type: filestream
    id: gen-17
    paths: ["/tmp/dmrepro/logs/f17.log"]
  - type: filestream
    id: gen-18
    paths: ["/tmp/dmrepro/logs/f18.log"]
  - type: filestream
    id: gen-19
    paths: ["/tmp/dmrepro/logs/f19.log"]
  - type: filestream
    id: gen-20
    paths: ["/tmp/dmrepro/logs/f20.log"]
  - type: filestream
    id: gen-21
    paths: ["/tmp/dmrepro/logs/f21.log"]
  - type: filestream
    id: gen-22
    paths: ["/tmp/dmrepro/logs/f22.log"]
  - type: filestream
    id: gen-23
    paths: ["/tmp/dmrepro/logs/f23.log"]
  - type: filestream
    id: gen-24
    paths: ["/tmp/dmrepro/logs/f24.log"]

# Beat-level (global) processors: built once, shared by every input's pipeline client
processors:
  - add_fields:
      target: process
      fields:
        pid: 1
  - add_docker_metadata:
      match_source: false   # force the match_pids path (lazy cgroup cache)

output.file:
  path: "/tmp/dmrepro/out"
  filename: out

queue.mem:
  events: 4096
  flush.min_events: 1

logging.level: debug
logging.selectors: ["add_docker_metadata"]
  1. Build Filebeat with the race detector and run it.
cd filebeat && go build -race -o /tmp/dmrepro/filebeat.race . && cd -
GORACE="halt_on_error=1" /tmp/dmrepro/filebeat.race run -e --strict.perms=false \
  -c /tmp/dmrepro/filebeat.yml --path.home=/tmp/dmrepro/home

On main the run aborts within a second with a WARNING: DATA RACE (exit code 66) on the shared processor's cgroup cache:

WARNING: DATA RACE
Write at 0x... by goroutine 148:
  common.(*element).UpdateLastAccessTime()   libbeat/common/cache.go:54
  common.(*Cache).get / Get                  libbeat/common/cache.go:284 / 183
  add_docker_metadata.(*addDockerMetadata).lookupContainerIDByPID()  add_docker_metadata.go:381
  add_docker_metadata.(*addDockerMetadata).Run()                     add_docker_metadata.go:278
  processors.(*SafeProcessor).Run() -> processing.(*group).Run()     (global processor)
  pipeline.(*client).publish() <- filestream harvester
Previous read at 0x... by goroutine 155:
  common.(*element).IsExpired()              libbeat/common/cache.go:48
  ... same stack ...

This is an automatic backport of pull request #51688 done by [Mergify](https://mergify.com).

add_docker_metadata can run as a beat-level (global) processor, in
which case a single instance is shared by every pipeline client and
its Run method is invoked concurrently. Two data races existed on
that path, both reachable by default because match_pids defaults to
["process.pid", "process.parent.pid"].

lazyCgroupCacheInit guarded the cache with a plain
"if d.cgroups == nil" check-then-set. Concurrent Run calls raced on
the pointer, and two callers could each build a cache and start its
own janitor goroutine, leaking a goroutine and orphaning a cache.
Replace the field with an atomic.Pointer[common.Cache] initialized
exactly once through sync.Once, and load it everywhere it is read.

common.Cache.Get held only the read lock, but get updates the
element's last access time on access-expiry caches (the cgroup cache
is one), so concurrent Get calls mutated the same element under a
shared lock. Take the exclusive lock in Get.

Co-authored-by: Lee E. Hinman <lee.e.hinman@elastic.co>
(cherry picked from commit e1add68)

# Conflicts:
#	libbeat/processors/add_docker_metadata/add_docker_metadata.go
#	libbeat/processors/add_docker_metadata/add_docker_metadata_test.go
@mergify
mergify Bot requested a review from a team as a code owner July 10, 2026 10:05
@mergify mergify Bot added backport conflicts There is a conflict in the backported pull request labels Jul 10, 2026
@mergify
mergify Bot requested review from khushijain21 and leehinman and removed request for a team July 10, 2026 10:05
@mergify

mergify Bot commented Jul 10, 2026

Copy link
Copy Markdown
Contributor Author

Cherry-pick of e1add68 has failed:

On branch mergify/bp/8.19/pr-51688
Your branch is up to date with 'origin/8.19'.

You are currently cherry-picking commit e1add6833.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Changes to be committed:
	new file:   changelog/fragments/1783070200-fix-add-docker-metadata-cgroup-cache-races.yaml
	modified:   libbeat/common/cache.go

Unmerged paths:
  (use "git add <file>..." to mark resolution)
	both modified:   libbeat/processors/add_docker_metadata/add_docker_metadata.go
	both modified:   libbeat/processors/add_docker_metadata/add_docker_metadata_test.go

To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally

@botelastic botelastic Bot added the needs_team Indicates that the issue/PR needs a Team:* label label Jul 10, 2026
@github-actions

Copy link
Copy Markdown
Contributor

🤖 GitHub comments

Just comment with:

  • run docs-build : Re-trigger the docs validation. (use unformatted text in the comment!)
  • /test : Run the Buildkite pipeline.

@github-actions github-actions Bot added bug Team:Elastic-Agent-Data-Plane Label for the Agent Data Plane team labels Jul 10, 2026
@botelastic botelastic Bot removed the needs_team Indicates that the issue/PR needs a Team:* label label Jul 10, 2026
@infra-vault-gh-plugin-prod

Copy link
Copy Markdown

Pinging @elastic/elastic-agent-data-plane (Team:Elastic-Agent-Data-Plane)

@github-actions

Copy link
Copy Markdown
Contributor

TL;DR

The backport is failing because commit e1add6833 was cherry-picked with unresolved conflict markers left in libbeat/processors/add_docker_metadata/add_docker_metadata.go and add_docker_metadata_test.go. Both pre-commit jobs fail on check-merge-conflict, and packetbeat check/update then fails to parse the test file. Resolve the backport conflicts and push the corrected files before rerunning CI.

Remediation

  • Resolve the conflict blocks in libbeat/processors/add_docker_metadata/add_docker_metadata.go, preserving the intended 8.19 base behavior together with the atomic cgroup-cache initialization and close handling from the backported fix.
  • Resolve the import conflict and test changes in libbeat/processors/add_docker_metadata/add_docker_metadata_test.go; ensure no <<<<<<<, =======, or >>>>>>> markers remain.
  • Run pre-commit run --all-files and make -C x-pack/packetbeat check update && make check-no-changes before rerunning Buildkite.
Investigation details

Root Cause

The PR head contains unresolved cherry-pick conflict sections. In add_docker_metadata.go, the conflict is in the import/struct area at the reported lines 66-82 and again in Close at lines 250-285. The test file has an unresolved import conflict at lines 27-31.

Evidence

  • Build: https://buildkite.com/elastic/beats/builds/49187
  • Jobs: x-pack/dockerlogbeat: Run pre-commit and x-pack/packetbeat: Run pre-commit report check for merge conflicts failures, including Merge conflict string u003cu003cu003cu003cu003cu003c found in both files.
  • Job: x-pack/packetbeat: Run check/update fails at libbeat/processors/add_docker_metadata/add_docker_metadata_test.go:27:1: missing import path, which is a direct consequence of the conflict-marker import block.
  • The PR already has a Mergify comment stating that cherry-pick commit e1add6833 stopped with both files under Unmerged paths.

Verification

Not run; this workflow is read-only and only diagnoses the Buildkite failure.


What is this? | From workflow: PR Buildkite Detective

Give us feedback! React with 🚀 if perfect, 👍 if helpful, 👎 if not.

The 8.19 branch lacks the docker-retry feature that exists on main, so
the cherry-pick of #51688 conflicted on the addDockerMetadata struct and
Close(). Keep this branch's existing non-retry code and apply only the
cgroup cache race fix: make cgroups an atomic.Pointer initialized once via
sync.Once, add the closed flag, and take the atomic path in Close(). Also
add the sync/sync/atomic imports (main file) and sync/time imports (test)
that the fix now requires but that main already had.

Assisted-By: Cursor
@orestisfl
orestisfl enabled auto-merge (squash) July 10, 2026 11:40
@orestisfl
orestisfl disabled auto-merge July 10, 2026 11:40
@orestisfl
orestisfl enabled auto-merge (squash) July 10, 2026 11:40
@github-actions

Copy link
Copy Markdown
Contributor

TL;DR

All three golangci-lint matrix jobs fail in the golangci-lint step on two new diagnostics in libbeat/processors/add_docker_metadata/add_docker_metadata.go: an unchecked cid.(string) assertion at line 281 and deprecated cgroup.NewReader usage at line 55. Replace the assertion with a checked type assertion and use cgroup.NewReaderOptions, then rerun CI.

Remediation

  • In lookupContainerIDByPID, validate the cached value before returning it (for example, assign cidStr, ok := cid.(string) and handle !ok), instead of returning cid.(string) directly.
  • Change initCgroupPaths from cgroup.NewReader(rootfsMountpoint, ignoreRootCgroups) to cgroup.NewReaderOptions(cgroup.ReaderOptions{RootfsMountpoint: rootfsMountpoint, IgnoreRootCgroups: ignoreRootCgroups}).
  • Rerun the golangci-lint workflow after pushing the corrected backport.
Investigation details

Root Cause

The PR head at 5de828aa88392bb5ec1887da049b1a05c532affe contains the unchecked cached-value assertion return cid.(string), nil in lookupContainerIDByPID. It also defines initCgroupPaths with the deprecated cgroup.NewReader API. Both are reported as new issues by golangci-lint.

Evidence

  • Workflow: https://github.com/elastic/beats/actions/runs/29087857785
  • Jobs/step: lint (ubuntu-latest), lint (macos-latest), and lint (windows-latest); each fails at step golangci-lint.
  • Key log excerpt: libbeat/processors/add_docker_metadata/add_docker_metadata.go:281:12: Error return value is not checked (errcheck) followed by return cid.(string), nil.
  • Key log excerpt: libbeat/processors/add_docker_metadata/add_docker_metadata.go:55:9: SA1019: cgroup.NewReader is deprecated: use NewReaderOptions (staticcheck).
  • Each job reports 2 issues: * errcheck: 1 * staticcheck: 1.

Validation

Not run; this workflow is read-only and the diagnosis is based on the failed workflow logs and PR head contents.

Follow-up

The previous detective report on this PR covered unresolved cherry-pick conflict markers in an earlier Buildkite run. The current lint failures are materially different and remain after that issue was addressed.


What is this? | From workflow: PR Actions Detective

Give us feedback! React with 🚀 if perfect, 👍 if helpful, 👎 if not.

@orestisfl
orestisfl merged commit f9bd5a3 into 8.19 Jul 10, 2026
192 of 195 checks passed
@orestisfl
orestisfl deleted the mergify/bp/8.19/pr-51688 branch July 10, 2026 12:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport bug conflicts There is a conflict in the backported pull request Team:Elastic-Agent-Data-Plane Label for the Agent Data Plane team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant