Send process name as process.executable.name (/proc/PID/comm) #298

christos68k · 2025-01-06T20:37:32Z

Summary

This PR adds support for sending process name as process.executable.name, abiding by current OTel semantics for that key. Instead of Name in /prod/PID/status as semconv dictates, I'm using /proc/PID/comm which contains the same value and is simpler to parse.

Regarding these semantics, I also started a discussion in semconv WG to improve clarity as process.executable.name is misleading/confusing/wrong depending on interpretation so we may end up changing the key in the future.

christos68k · 2025-01-07T22:04:13Z

processmanager/processinfo.go

@@ -641,6 +646,19 @@ func (pm *ProcessManager) CleanupPIDs() {
 	}
 }

+// NameForPID returns the process name for given PID.


Depending on how we go about solving #278, we may move these (process name, executable path) somewhere else. For now, I left them as processInfo attributes.

florianl · 2025-01-08T07:46:40Z

host/host.go

@@ -48,7 +48,8 @@ type Frame struct {

 type Trace struct {
 	Comm             string
-	Executable       string
+	ProcessName      string


With this change there are two fields with roughly the same information. Comm originates from the eBPF space and is fetched with the helper bpf_get_current_comm() and ProcessName is fetched from procfs via /prod/%d/comm.
To some degree this is duplicate information - ignoring for the moment all the events that can change these values and cause differences in them.
Going forward, I think there should be either Comm or ProcessName but not both. If both fields should be kept, what is the point they bring in that makes them valuable?

It's not the same information: Comm retrieved in the kernel is the thread name and /proc/PID/comm is the process name, they're sent to the backend under different keys.

The value they bring is more flexible aggregations: executable, process, thread. Why pick one or the other?

Both values are kind of metadata to me and I'm wondering if there is something in OTel that enriches metadata based on a PID. The value from bpf_get_current_comm() might not be available to a OTel metadata enricher, but the rest should be. So overall, I'm thinking more about what components are the key functionality of profiling vs what enrichment can be added by other components.

The reason we want to do this particular enrichment here that depends on process lifetime is because we're in full control of the metadata (process name, executable name) that is thrown away on process exit and can guarantee their availability (after we fix #278).

processmanager/processinfo.go

Co-authored-by: Tim Rühsen <[email protected]>

* Bump golangci-lint to 1.63.4 (open-telemetry#311) Author: Tim Rühsen <[email protected]> * CI: reduce boiler plate of environment (open-telemetry#312) * Consistent format of the eBPF code (open-telemetry#310) * Off CPU profiling (open-telemetry#196) * CI: Check for differences in the eBPF binary blobs (open-telemetry#305) * README: drop make target docker-image as pre requirement (open-telemetry#304) Signed-off-by: Florian Lehner <[email protected]> * Setup collector receiver (open-telemetry#160) Author: Damien Mathieu <[email protected]> Co-authored-by: Tim Rühsen <[email protected]> * Send process name as process.executable.name (/proc/PID/comm) (open-telemetry#298) Author: Christos Kalkanis <[email protected]> Co-authored-by: Tim Rühsen <[email protected]> * Remove host metadata collector (open-telemetry#299) Author: Damien Mathieu <[email protected]> * apmint: return error if Attach() fails (open-telemetry#271) * docker: build and push docker builder image (open-telemetry#297) Signed-off-by: Florian Lehner <[email protected]> * Update golang.org/x/net to v0.33.0 (Fix CVE-2024-45338) (open-telemetry#294) Author: Tim Rühsen <[email protected]> * metrics: update package documentation (open-telemetry#295) Signed-off-by: Florian Lehner <[email protected]> * Fix pdata function table order (open-telemetry#286) Author: Tolya Korniltsev <[email protected]> * Fix cross-compilation (open-telemetry#282) (open-telemetry#290) Author: Christos Kalkanis <[email protected]> * Use clang-17 for building tracers (open-telemetry#270) Author: Mathieu Bressolle-Chataigner <[email protected]> Co-authored-by: Co-authored-by: Christos Kalkanis <[email protected]> * Make the CollAgentAddr config optional in controller (open-telemetry#279) Author: Damien Mathieu <[email protected]> Co-authored-by: Christos Kalkanis <[email protected]> Co-authored-by: Florian Lehner <[email protected]> * Remove unused type libpf.TraceAndCounts (open-telemetry#283) Author: Tim Rühsen <[email protected]> * Collector Reporter (open-telemetry#208) Author: Damien Mathieu <[email protected]> Co-authored-by: Tim Rühsen <[email protected]> Co-authored-by: Florian Lehner <[email protected]> Co-authored-by: Christos Kalkanis <[email protected]> * proc: handle nvidia kernel modules (open-telemetry#274) * README: provide new version of devfiler (open-telemetry#275) Author: Florian Lehner <[email protected]> * Extract reporter data generation, and use pdata (open-telemetry#245) Author: Damien Mathieu <[email protected]> Co-authored-by: Florian Lehner <[email protected]> Co-authored-by: Christos Kalkanis <[email protected]> * apmint: warn user about changes in the APM service (open-telemetry#269) Author: Florian Lehner <[email protected]> Co-authored-by: Joel Höner <[email protected]> * coredump: no need for full dump with bundled files (open-telemetry#213) Author: Timo Teräs <[email protected]> * Send executable path with every stack trace (open-telemetry#253) * Send thread name as ThreadNameKey not ProcessCommandKey (open-telemetry#265) Author: Christos Kalkanis <[email protected]> * proc: Fix parsing Kernel Module lines where refcount is - (open-telemetry#263) Author: Frederic Branczyk <[email protected]> * reporter: extend lifetime for cached elements (open-telemetry#260) Author: Florian Lehner <[email protected]> * Reporter: allow extending samples with extra attributes (open-telemetry#237) Author: Joel Höner <[email protected]> Co-authored-by: Tim Rühsen <[email protected]>

Send process name as process.executable.name (/proc/PID/comm)

68a989e

christos68k requested review from a team as code owners January 6, 2025 20:37

christos68k marked this pull request as draft January 6, 2025 20:37

christos68k marked this pull request as ready for review January 7, 2025 22:02

christos68k commented Jan 7, 2025

View reviewed changes

florianl reviewed Jan 8, 2025

View reviewed changes

rockdaboot reviewed Jan 8, 2025

View reviewed changes

processmanager/processinfo.go Outdated Show resolved Hide resolved

Update processmanager/processinfo.go

215d5a8

Co-authored-by: Tim Rühsen <[email protected]>

florianl approved these changes Jan 9, 2025

View reviewed changes

rockdaboot approved these changes Jan 9, 2025

View reviewed changes

christos68k merged commit e113282 into main Jan 9, 2025
23 checks passed

christos68k deleted the ck/process-name branch January 9, 2025 14:48

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Send process name as process.executable.name (/proc/PID/comm) #298

Send process name as process.executable.name (/proc/PID/comm) #298

christos68k commented Jan 6, 2025 •

edited

Loading

christos68k Jan 7, 2025 •

edited

Loading

florianl Jan 8, 2025

christos68k Jan 8, 2025 •

edited

Loading

florianl Jan 9, 2025 •

edited

Loading

christos68k Jan 9, 2025

Send process name as process.executable.name (/proc/PID/comm) #298

Send process name as process.executable.name (/proc/PID/comm) #298

Conversation

christos68k commented Jan 6, 2025 • edited Loading

Summary

christos68k Jan 7, 2025 • edited Loading

Choose a reason for hiding this comment

florianl Jan 8, 2025

Choose a reason for hiding this comment

christos68k Jan 8, 2025 • edited Loading

Choose a reason for hiding this comment

florianl Jan 9, 2025 • edited Loading

Choose a reason for hiding this comment

christos68k Jan 9, 2025

Choose a reason for hiding this comment

christos68k commented Jan 6, 2025 •

edited

Loading

christos68k Jan 7, 2025 •

edited

Loading

christos68k Jan 8, 2025 •

edited

Loading

florianl Jan 9, 2025 •

edited

Loading