fix: handle bare container IDs from CRI client in matchContainerID#3157
Merged
brancz merged 2 commits intoparca-dev:mainfrom Mar 16, 2026
Merged
Conversation
|
|
The CRI client fast path (cri_client.go:convertSandboxToPod) sets
ContainerStatus.ContainerID to the bare runtime ID from the CRI API,
which is a 64-char hex string without the "containerd://" prefix that
the kubelet adds for the Kubernetes API.
The containerIDPattern regex required this prefix, causing every
CRI-sourced container lookup to fail.
Change regex from `.+://([0-9a-f]{64})` to `^(?:.+://)?([0-9a-f]{64})$`:
- Make the runtime:// prefix optional
- Add ^ and $ anchors (stricter than original - no substring matching)
- Handles both Kubernetes API format and CRI API format
Fixes parca-dev#3156
Co-Authored-By: Claude Opus 4.6 <[email protected]>
The getKubernetesPodMetadata function iterates all containers on a node to find the one matching a profiled PID. Non-matching containers are expected - logging each at Error level produces N_processes * M_containers error logs per profiling cycle. Downgrade to Debugf to match the level already used in addPodContainerLabels for the same matchContainerID error. Co-Authored-By: Claude Opus 4.6 <[email protected]>
michaelfindlater
force-pushed
the
fix/cri-container-id-prefix
branch
from
2e576a1 to
d8f6e6b
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
containerIDPatternregex accept both prefixed (containerd://hex) and bare (hex) 64-char container IDslog.Errortolog.DebugfingetKubernetesPodMetadatato match the level used inaddPodContainerLabelsfor the same errormatchContainerIDcovering both formats and edge casesProblem
The CRI client fast path (
cri_client.go:convertSandboxToPod) setsContainerStatus.ContainerIDto the bare runtime ID from the CRI API (container.Id), which is a 64-char hex string without thecontainerd://prefix. The kubelet normally adds this prefix when constructingContainerStatusfor the Kubernetes API, but the CRI client bypasses the kubelet.The
containerIDPatternregex (.+://([0-9a-f]{64})) requires the://prefix, so every CRI-sourced container lookup fails. Combined withlog.Errorlevel logging, this generates N_processes * M_containers error logs per profiling cycle - over 10,000 logs/hour across our fleet of 134 agents.Fix
Commit 1 - Regex:
.+://([0-9a-f]{64})to^(?:.+://)?([0-9a-f]{64})$^...$anchors prevent substring matching (stricter than original)(?:.+://)?makes the prefix optionalCommit 2 - Log level:
log.Errortolog.DebugfingetKubernetesPodMetadataaddPodContainerLabelsfor the same errorTest plan
matchContainerIDcovering prefixed IDs, bare IDs, and invalid inputs (empty, short, non-hex, 65 chars, cgroup paths, trailing chars)Fixes #3156