Skip to content

Refactor reporter to use event tree#175

Merged
nsavoire merged 5 commits intomainfrom
nsavoire/update_upstream_tree
Aug 18, 2025
Merged

Refactor reporter to use event tree#175
nsavoire merged 5 commits intomainfrom
nsavoire/update_upstream_tree

Conversation

@nsavoire
Copy link
Copy Markdown
Collaborator

@nsavoire nsavoire commented Aug 4, 2025

Store trace events in a tree (similar to open-telemetry/opentelemetry-ebpf-profiler#528):
[ContainerId, Service] -> [Origin] (CPU,OffCPU,...) -> [Hash,Comm,Pid,Tid]

This makes emitting profile by service easier since events are already split by service.
This moves service determination from reporting time to trace event collection time, meaning that changes to process metadata (eg. exe name) will not be taken into account for already collected traces.

Also:

  • Extract pprof generation code to its own package.
  • Remove executablePath and apmServiceName from TraceAndMetaKey since they were not used.
  • Use a unique dummy mapping for all interpreted frames.
  • Use "kernel" as source file for kernel frames.
  • Use following logic to determine process executable path in decreasing priority order:
    • Use /proc/<pid>/exe in addProcessMetadata if successful
    • Use TraceEventMeta.ExecutablePath if not empty
    • "kernel" if root frame is kernel
    • TraceAndMetaKey.Comm: since it's retrieved by eBPF it can be non empty even if TraceAndMetaKey.ProcessName is empty

@nsavoire nsavoire requested a review from Gandem August 4, 2025 14:23
@nsavoire nsavoire force-pushed the nsavoire/upstream_update branch from 7580433 to cc345f5 Compare August 7, 2025 09:20
@nsavoire nsavoire force-pushed the nsavoire/update_upstream_tree branch 3 times, most recently from 4e49c33 to c288005 Compare August 8, 2025 07:49
@nsavoire nsavoire changed the title Test using event tree Refactor reporter to use event tree Aug 8, 2025
@nsavoire nsavoire marked this pull request as ready for review August 8, 2025 09:51
@nsavoire nsavoire requested a review from a team as a code owner August 8, 2025 09:51
@nsavoire nsavoire force-pushed the nsavoire/upstream_update branch from 7c45b8e to 0903004 Compare August 8, 2025 13:22
@nsavoire nsavoire force-pushed the nsavoire/update_upstream_tree branch from a3ab067 to 7fe648d Compare August 8, 2025 13:23
Comment thread reporter/datadog_reporter.go Outdated
totalSampleCount += stats.totalSampleCount
totalPIDsWithNoProcessMetadata += stats.pidWithNoMetadata
tags := createTagsForProfile(r.tags, profileSeq, e.service, e.inferredService)
for e, perServiceEvents := range reportedEvents {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nit: rename e to s since it's the service entity attributes

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Done

Copy link
Copy Markdown
Member

@Gandem Gandem left a comment

Choose a reason for hiding this comment

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

Thanks!

@nsavoire nsavoire force-pushed the nsavoire/upstream_update branch from 0903004 to 3705ec0 Compare August 18, 2025 14:24
@nsavoire nsavoire force-pushed the nsavoire/update_upstream_tree branch from 7fe648d to 107eac4 Compare August 18, 2025 14:27
Base automatically changed from nsavoire/upstream_update to main August 18, 2025 14:45
@nsavoire nsavoire force-pushed the nsavoire/update_upstream_tree branch from 107eac4 to e35e964 Compare August 18, 2025 14:54
@nsavoire nsavoire merged commit 450e1a1 into main Aug 18, 2025
8 checks passed
@nsavoire nsavoire deleted the nsavoire/update_upstream_tree branch August 18, 2025 15:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants