Skip to content
This repository has been archived by the owner on Jan 21, 2025. It is now read-only.

Add root frame with executable path when available (PROF-9924) #12

Merged
merged 3 commits into from
Jun 12, 2024

Conversation

Gandem
Copy link
Member

@Gandem Gandem commented Jun 11, 2024

Objective / Motivation

Add a root frame with the path of the executable when the information is available.

How this is done

This is done in three steps.

First, we need to provide the reporter with the PID <-> Executable path name mapping. This is done by:

  • Adding a GetExecutablePath() on the process object, that either returns the main executable for a core dump, or reads /proc/pid/exe for live processes)
  • On SynchronizeProcess, the process manager will call ProcessMetadata indicating which pid has which exec name.
  • On the reporter we add an LRU that contains the PID <-> Executable mapping, that is updated by ProcessMetadata calls.

Then we need to have the PID for the sample we're currently producing. This is done by:

  • Updating the tracehandler so that it includes the PID in ReportCountForTrace
  • Updating ReportCountForTrace so that it includes the PID of the frame, this PID will be added to the traceInfo structure.

Now we have the PID for each trace, and the PID <-> executable path mapping 🎉 🌮 We use this when producing the pprof in the datadog reporter:

  • If the latest frame is a kernel frame, we add a dummy location / function named "kernel"
  • Else, we try to find the exec path associated with the PID. If we do, we add a dummy location / function with the name of the executable path. Else we do nothing.

This is an example profile link that showcases this working locally.

image

(Note that the java program present in that image is indeed launched via ld hence why we see ld there)

@Gandem Gandem force-pushed the nayef/file-path-frame branch 5 times, most recently from 37a7f30 to 2a0ffef Compare June 11, 2024 15:54
@Gandem Gandem changed the title Add additional frame with executable path when available Add additional frame with executable path when available (PROF-9924) Jun 11, 2024
@Gandem Gandem changed the title Add additional frame with executable path when available (PROF-9924) Add root frame with executable path when available (PROF-9924) Jun 11, 2024
@Gandem Gandem force-pushed the nayef/file-path-frame branch from 2a0ffef to d0a485a Compare June 11, 2024 16:17
@Gandem Gandem requested review from nsavoire and r1viollet June 11, 2024 16:19
@Gandem Gandem marked this pull request as ready for review June 11, 2024 16:20
@Gandem Gandem force-pushed the nayef/file-path-frame branch from d0a485a to 5adaf07 Compare June 11, 2024 16:29
r1viollet
r1viollet previously approved these changes Jun 12, 2024
Copy link
Collaborator

@r1viollet r1viollet left a comment

Choose a reason for hiding this comment

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

LGTM
I will be happy to try this out!

Copy link
Collaborator

@r1viollet r1viollet left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Collaborator

@nsavoire nsavoire left a comment

Choose a reason for hiding this comment

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

LGTM !

@Gandem Gandem merged commit d21ac0c into main Jun 12, 2024
5 checks passed
@Gandem Gandem deleted the nayef/file-path-frame branch June 12, 2024 13:17
Gandem added a commit that referenced this pull request Jun 26, 2024
* Add additional frame with executable path when available

* datadog reporter: set different function name and executable name for root frame

* reporter: rename pid label to process_id

also add the label in the datadog reported as well
Gandem added a commit that referenced this pull request Jun 27, 2024
* Add additional frame with executable path when available

* datadog reporter: set different function name and executable name for root frame

* reporter: rename pid label to process_id

also add the label in the datadog reported as well
Gandem added a commit that referenced this pull request Jun 28, 2024
* Add additional frame with executable path when available

* datadog reporter: set different function name and executable name for root frame

* reporter: rename pid label to process_id

also add the label in the datadog reported as well
Gandem added a commit that referenced this pull request Jun 28, 2024
* Add additional frame with executable path when available

* datadog reporter: set different function name and executable name for root frame

* reporter: rename pid label to process_id

also add the label in the datadog reported as well
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants