ProcessManager: Extract containerID with the rest of process metadata by christos68k · Pull Request #577 · open-telemetry/opentelemetry-ebpf-profiler

christos68k · 2025-07-03T00:49:00Z

Summary

This is an alternate solution to #548, see here and here for more context.

It includes all the commits that @florianl did in #548 (which is now closed), we can review and merge #577 standalone.

This is a follow up to #535. While the tracer part of the project reports the cgroup v2 path for each sample, the reporter is expected to report the container ID. The container ID then can be used to associate the sample to more detailed resource information. In the context of OTel collector, the container ID then can be used like this: ``` k8sattributes: auth_type: "serviceAccount" passthrough: false filter: node_from_env_var: KUBERNETES_NODE_NAME extract: metadata: - k8s.pod.name - k8s.pod.uid - k8s.deployment.name - k8s.namespace.name - k8s.node.name - k8s.pod.start_time - service.namespace - service.name - service.version - service.instance.id labels: - tag_name: app.label.component key: app.kubernetes.io/component from: pod otel_annotations: true pod_association: - sources: - from: resource_attribute name: container.id ``` As the cgroupv2 path contains further information, that could be beneficial for other reporters, the extraction of the container ID happens in the reporter. Signed-off-by: Florian Lehner <florian.lehner@elastic.co>

Signed-off-by: Florian Lehner <florian.lehner@elastic.co>

florianl · 2025-07-03T06:37:33Z

Repeating my #548 (comment) here as well:

While it seems tempting to move process data extraction to processmanager, it is not the purpose of processmanager in the first place. Therefore, I suggest:

merge reporter: extract container.ID from cgroupv2 path #548 to get k8s compatability in OTel
introduce a generic hook in processmanager, so it can be configured to extract any process data and not limited to just container ID

florianl

I think, @fabled summarized the situation well in #548 (comment) and so I think, this option is a good interim solution until there is a plugin mechanism to extract such information by configuration.

christos68k · 2025-07-03T13:00:28Z

I think, @fabled summarized the situation well in #548 (comment) and so I think, this option is a good interim solution until there is a plugin mechanism to extract such information by configuration.

OK let's close #548, review #577 (I still need to do another pass and test some more) and I'll merge after (I'll also add you as a co-author to the squashed commit).

florianl · 2025-07-03T13:16:07Z

Closing in favor of #577

christos68k · 2025-07-03T13:43:34Z

Closing in favor of #577

This is #577 😄

florianl · 2025-07-03T13:44:46Z

This is #577 😄

sry 🙏

Anthony-Tafoya · 2025-07-07T03:01:24Z

Hello, I understand this is still in review. However, tried to run the profiler from the commit here and my container.id started to show up as empty for all entries. Thought it would be relevant to drop this here:

florianl · 2025-07-07T05:39:52Z

@Anthony-Tafoya can you provide the full cgroupv2 path of one of your containers? You can fetch it from /proc/<PID>/cgroup.

Anthony-Tafoya · 2025-07-07T19:06:51Z

After taking a further look, this works fine. I was testing on a docker compose setup (so I would not have to deploy every small yaml change). Before I was seeing the login session’s cgroup like /user.slice/user‑1000.slice/session‑1.scope and /../... On my kubernetes cluster this works as expected (grabs the containerID). Apologies for the confusion

rockdaboot · 2025-07-09T07:31:27Z

+	}
+
+	for _, tc := range tests {
+		tc := tc


Suggested change

tc := tc

rockdaboot · 2025-07-09T07:35:33Z

+	for _, tc := range tests {
+		tc := tc
+		t.Run(tc.expectedContainerID, func(t *testing.T) {
+			reader := bytes.NewReader([]byte(tc.line))


Suggested change

reader := bytes.NewReader([]byte(tc.line))

reader := strings.NewReader(tc.line)

rockdaboot · 2025-07-09T07:53:13Z

 	Set(pre host.FileID, post libpf.FileID)
 }
+
+func parseContainerID(cgroupFile io.Reader) string {


Since the tests only contain cgroupV2 strings, should we document this function to support cgroupV2 only? Otherwise, we should add cgroupV1 test cases.
IMO, we don't need to support for v1.

rockdaboot · 2025-07-09T08:05:24Z

+	scanner.Buffer(buf, 8192)
+	var pathParts []string
+	for scanner.Scan() {
+		line := scanner.Text()


Skip regex processing and heap allocations for the very common case of "0::/".

Suggested change

line := scanner.Text()

b := scanner.Bytes()

if bytes.Equal(b, []byte("0::/")) {

continue // Skip a common case

}

line := string(b)

rockdaboot

LGTM, just some minor comments

florianl and others added 8 commits June 25, 2025 10:57

fixup: add small LRU to cache regex result

7d0c039

Signed-off-by: Florian Lehner <florian.lehner@elastic.co>

fixup: merge reporter caches into a single cache

aa21cdb

Signed-off-by: Florian Lehner <florian.lehner@elastic.co>

fixup: function comment

877544e

Signed-off-by: Florian Lehner <florian.lehner@elastic.co>

fixup: update regex

daa9dc6

Signed-off-by: Florian Lehner <florian.lehner@elastic.co>

fixup: simplify test

55426a6

Signed-off-by: Florian Lehner <florian.lehner@elastic.co>

fixup: just log instead of return

7d42302

Signed-off-by: Florian Lehner <florian.lehner@elastic.co>

processmanager: fetch containerID with the rest of process metadata

3b0a7a2

christos68k requested review from a team as code owners July 3, 2025 00:49

christos68k mentioned this pull request Jul 3, 2025

reporter: extract container.ID from cgroupv2 path #548

Closed

florianl approved these changes Jul 3, 2025

View reviewed changes

florianl closed this Jul 3, 2025

florianl mentioned this pull request Jul 3, 2025

k8s discovery #581

Closed

christos68k reopened this Jul 3, 2025

christos68k enabled auto-merge (squash) July 3, 2025 20:02

christos68k self-assigned this Jul 3, 2025

florianl self-requested a review July 7, 2025 07:08

christos68k requested review from fabled and rockdaboot July 7, 2025 19:18

Add containerID tests

4535523

florianl reviewed Jul 8, 2025

View reviewed changes

Comment thread processmanager/helpers.go Outdated

florianl self-requested a review July 8, 2025 06:06

florianl approved these changes Jul 9, 2025

View reviewed changes

rockdaboot reviewed Jul 9, 2025

View reviewed changes

Comment thread processmanager/helpers_test.go

}

for _, tc := range tests {

tc := tc

Copy link
Copy Markdown

Contributor

rockdaboot Jul 9, 2025

Choose a reason for hiding this comment

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

Suggested change

tc := tc

rockdaboot reviewed Jul 9, 2025

View reviewed changes

rockdaboot approved these changes Jul 9, 2025

View reviewed changes

christos68k merged commit dba0c0e into main Jul 9, 2025
27 checks passed

christos68k deleted the ck/cid branch July 9, 2025 08:05

This was referenced Aug 14, 2025

fix golabels tests parca-dev/opentelemetry-ebpf-profiler#92

Closed

fix golabels test parca-dev/opentelemetry-ebpf-profiler#97

Closed

	reader := bytes.NewReader([]byte(tc.line))
	reader := strings.NewReader(tc.line)

-		line := scanner.Text()
+		b := scanner.Bytes()
+		if bytes.Equal(b, []byte("0::/")) {
+			continue // Skip a common case
+		}
+		line := string(b)

Conversation

christos68k commented Jul 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Uh oh!

florianl commented Jul 3, 2025

Uh oh!

florianl left a comment

Choose a reason for hiding this comment

Uh oh!

christos68k commented Jul 3, 2025

Uh oh!

florianl commented Jul 3, 2025

Uh oh!

christos68k commented Jul 3, 2025

Uh oh!

florianl commented Jul 3, 2025

Uh oh!

Anthony-Tafoya commented Jul 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

florianl commented Jul 7, 2025

Uh oh!

Anthony-Tafoya commented Jul 7, 2025

Uh oh!

Uh oh!

rockdaboot Jul 9, 2025

Choose a reason for hiding this comment

Uh oh!

rockdaboot Jul 9, 2025

Choose a reason for hiding this comment

Uh oh!

rockdaboot Jul 9, 2025

Choose a reason for hiding this comment

Uh oh!

rockdaboot Jul 9, 2025

Choose a reason for hiding this comment

Uh oh!

rockdaboot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

christos68k commented Jul 3, 2025 •

edited

Loading

Anthony-Tafoya commented Jul 7, 2025 •

edited

Loading