[EBPF-616] gpu: Use event consumer for processs monitoring #30755

gjulianm · 2024-11-05T09:40:26Z

What does this PR do?

This PR adds the process consumer introduced in #30559 to the GPU monitoring module. The consumer is created in the EventMonitor module and stored as a global variable in the GPU monitoring module.

Motivation

Describe how to test/QA your changes

Tested in e2e and KMT tests

Possible Drawbacks / Trade-offs

Additional Notes

cit-pr-commenter · 2024-11-05T09:51:43Z

Go Package Import Differences

Baseline: 0e63070
Comparison: 238d74f

binary	os	arch	change
system-probe	linux	amd64	+1, -0 +github.com/DataDog/datadog-agent/pkg/eventmonitor/consumers
system-probe	linux	arm64	+1, -0 +github.com/DataDog/datadog-agent/pkg/eventmonitor/consumers
system-probe	windows	amd64	+1, -0 +github.com/DataDog/datadog-agent/pkg/gpu/config

cit-pr-commenter · 2024-11-05T10:18:15Z

Regression Detector

Regression Detector Results

Metrics dashboard
Target profiles
Run ID: f7e59d07-99c2-4277-8e22-41313e8b9ab8

Baseline: 0e63070
Comparison: 238d74f
Diff

Optimization Goals: ❌ Significant changes detected

perf	experiment	goal	Δ mean %	Δ mean % CI	trials	links
✅	basic_py_check	% cpu utilization	-6.58	[-10.44, -2.73]	1	Logs

Fine details of change detection per experiment

perf	experiment	goal	Δ mean %	Δ mean % CI	trials	links
➖	uds_dogstatsd_to_api_cpu	% cpu utilization	+2.63	[+1.90, +3.36]	1	Logs
➖	file_to_blackhole_1000ms_latency	egress throughput	+0.30	[-0.18, +0.79]	1	Logs
➖	otel_to_otel_logs	ingress throughput	+0.02	[-0.66, +0.69]	1	Logs
➖	file_to_blackhole_500ms_latency	egress throughput	+0.01	[-0.24, +0.25]	1	Logs
➖	tcp_dd_logs_filter_exclude	ingress throughput	-0.00	[-0.01, +0.01]	1	Logs
➖	uds_dogstatsd_to_api	ingress throughput	-0.00	[-0.10, +0.09]	1	Logs
➖	file_to_blackhole_1000ms_latency_linear_load	egress throughput	-0.02	[-0.51, +0.46]	1	Logs
➖	file_to_blackhole_300ms_latency	egress throughput	-0.03	[-0.23, +0.17]	1	Logs
➖	file_to_blackhole_100ms_latency	egress throughput	-0.03	[-0.37, +0.30]	1	Logs
➖	file_to_blackhole_0ms_latency	egress throughput	-0.07	[-0.57, +0.42]	1	Logs
➖	quality_gate_idle	memory utilization	-0.37	[-0.42, -0.32]	1	Logs bounds checks dashboard
➖	file_tree	memory utilization	-0.75	[-0.89, -0.60]	1	Logs
➖	quality_gate_idle_all_features	memory utilization	-1.19	[-1.32, -1.07]	1	Logs bounds checks dashboard
➖	tcp_syslog_to_blackhole	ingress throughput	-1.41	[-1.49, -1.33]	1	Logs
➖	pycheck_lots_of_tags	% cpu utilization	-1.95	[-5.45, +1.55]	1	Logs
✅	basic_py_check	% cpu utilization	-6.58	[-10.44, -2.73]	1	Logs

Bounds Checks: ❌ Failed

perf	experiment	bounds_check_name	replicates_passed	links
❌	file_to_blackhole_0ms_latency	lost_bytes	7/10
❌	quality_gate_idle	memory_usage	8/10	bounds checks dashboard
❌	file_to_blackhole_100ms_latency	lost_bytes	9/10
❌	quality_gate_idle_all_features	memory_usage	9/10	bounds checks dashboard
✅	file_to_blackhole_0ms_latency	memory_usage	10/10
✅	file_to_blackhole_1000ms_latency	memory_usage	10/10
✅	file_to_blackhole_1000ms_latency_linear_load	memory_usage	10/10
✅	file_to_blackhole_100ms_latency	memory_usage	10/10
✅	file_to_blackhole_300ms_latency	memory_usage	10/10
✅	file_to_blackhole_500ms_latency	memory_usage	10/10

Explanation

Confidence level: 90.00%
Effect size tolerance: |Δ mean %| ≥ 5.00%

Performance changes are noted in the perf column of each table:

✅ = significantly better comparison variant performance
❌ = significantly worse comparison variant performance
➖ = no significant change in performance

A regression test is an A/B test of target performance in a repeatable rig, where "performance" is measured as "comparison variant minus baseline variant" for an optimization goal (e.g., ingress throughput). Due to intrinsic variability in measuring that goal, we can only estimate its mean value for each experiment; we report uncertainty in that value as a 90.00% confidence interval denoted "Δ mean % CI".

For each experiment, we decide whether a change in performance is a "regression" -- a change worth investigating further -- if all of the following criteria are true:

Its estimated |Δ mean %| ≥ 5.00%, indicating the change is big enough to merit a closer look.
Its 90.00% confidence interval "Δ mean % CI" does not contain zero, indicating that if our statistical model is accurate, there is at least a 90.00% chance there is a difference in performance between baseline and comparison variants.
Its configuration does not mark it "erratic".

agent-platform-auto-pr · 2024-11-07T10:52:35Z

Test changes on VM

Use this command from test-infra-definitions to manually test this PR changes on a VM:

inv create-vm --pipeline-id=48783107 --os-family=ubuntu

Note: This applies to commit 238d74f

gjulianm · 2024-11-08T16:00:25Z

/trigger-ci -v RUN_KMT_TESTS=on

dd-devflow · 2024-11-08T16:01:16Z

Devflow running: `/trigger-ci -v RUN_KMT_TESTS=on`

View all feedbacks in Devflow UI.

2024-11-08 16:01:16 UTC ℹ️ Gitlab pipeline started

Started pipeline #48572913

paulcacheux · 2024-11-11T16:06:36Z

cmd/system-probe/modules/eventmonitor.go

@@ -91,5 +94,13 @@ func createEventMonitorModule(_ *sysconfigtypes.Config, deps module.FactoryDepen
 		}
 	}

+	gpucfg := gpuconfig.New()
+	if gpucfg.Enabled {
+		err := createGPUProcessEventConsumer(evm)


So I'm not 100% sure, and I still have a TODO list item to refactor this, but don't you need to call evm.RegisterEventConsumer( here, like other consumers ?

That's included in the createGPUProcessEventConsumer function, more specifically here in the generic method for process-data consumers.

Also, if you could let me know how do you end up refactoring that part it'd be great, as we were thinking about refactoring the system-probe modules to allow passing dependencies such as the event consumer without resorting to globals.

Isn't the process consumer also calling it in

datadog-agent/cmd/system-probe/modules/eventmonitor.go

Line 81 in f6a1357

evm.RegisterEventConsumer(process)

? so it would be calling it twice

the goal of the refactoring would be to just clean up the need to call register twice with subtly different names..

That's a different consumer, for forwarding data to process-agent I think. This one is the one I introduced in this PR to have a generic consumer that can be used to replace pkg/process/monitor:ProcessMonitor, which turned out to be only for USM use.

gjulianm · 2024-11-12T16:30:37Z

/merge

dd-devflow · 2024-11-12T16:30:50Z

Devflow running: `/merge`

View all feedbacks in Devflow UI.

2024-11-12 16:30:50 UTC ℹ️ MergeQueue: pull request added to the queue

The median merge time in main is 23m.

gjulianm self-assigned this Nov 5, 2024

github-actions bot added component/system-probe medium review PR review might take time team/ebpf-platform labels Nov 5, 2024

gjulianm force-pushed the guillermo.julian/gpu-event-consumer branch from 4bc8747 to e5ea2ab Compare November 6, 2024 16:46

gjulianm mentioned this pull request Nov 7, 2024

[EBPF] gpu: refactor probe_test to use a suite #30846

Merged

gjulianm added changelog/no-changelog qa/done Skip QA week as QA was done before merge and regressions are covered by tests labels Nov 7, 2024

gjulianm force-pushed the guillermo.julian/gpu-event-consumer branch from e6430cd to 202abf4 Compare November 7, 2024 10:09

gjulianm added 8 commits November 8, 2024 11:44

Add event consumer to GPU monitoring

4b6d970

Use correct consumer in tests

239a6a1

Fix comment

d8edbc5

Use correct import

0207896

Fix linter

5eda681

Fix linter

a24aa62

Fix rebase

6f69fd5

Fix windows build

bdbdddd

gjulianm force-pushed the guillermo.julian/gpu-event-consumer branch from 472364f to bdbdddd Compare November 8, 2024 10:48

gjulianm added 2 commits November 8, 2024 14:02

Split linux-only config code

e3569e2

Enable eventmonitor even if only GPU is enabled

7bb60ac

github-actions bot added long review PR is complex, plan time to review it and removed medium review PR review might take time labels Nov 8, 2024

gjulianm changed the title ~~[EBPF-595] gpu: Use event consumer for processs monitoring~~ [EBPF-616] gpu: Use event consumer for processs monitoring Nov 8, 2024

gjulianm marked this pull request as ready for review November 8, 2024 16:08

gjulianm requested a review from a team as a code owner November 8, 2024 16:08

val06 approved these changes Nov 11, 2024

View reviewed changes

Merge branch 'main' into guillermo.julian/gpu-event-consumer

f6a1357

paulcacheux reviewed Nov 11, 2024

View reviewed changes

paulcacheux approved these changes Nov 11, 2024

View reviewed changes

Merge branch 'main' into guillermo.julian/gpu-event-consumer

238d74f

dd-mergequeue bot merged commit 7fdeef9 into main Nov 12, 2024
292 checks passed

dd-mergequeue bot deleted the guillermo.julian/gpu-event-consumer branch November 12, 2024 16:50

github-actions bot added this to the 7.61.0 milestone Nov 12, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[EBPF-616] gpu: Use event consumer for processs monitoring #30755

[EBPF-616] gpu: Use event consumer for processs monitoring #30755

gjulianm commented Nov 5, 2024 •

edited

Loading

cit-pr-commenter bot commented Nov 5, 2024 •

edited

Loading

cit-pr-commenter bot commented Nov 5, 2024 •

edited

Loading

Fine details of change detection per experiment

Explanation

agent-platform-auto-pr bot commented Nov 7, 2024 •

edited

Loading

gjulianm commented Nov 8, 2024

dd-devflow bot commented Nov 8, 2024

paulcacheux Nov 11, 2024

gjulianm Nov 11, 2024

gjulianm Nov 11, 2024

paulcacheux Nov 11, 2024

paulcacheux Nov 11, 2024

gjulianm Nov 11, 2024 •

edited

Loading

gjulianm commented Nov 12, 2024

dd-devflow bot commented Nov 12, 2024 •

edited

Loading

[EBPF-616] gpu: Use event consumer for processs monitoring #30755

[EBPF-616] gpu: Use event consumer for processs monitoring #30755

Conversation

gjulianm commented Nov 5, 2024 • edited Loading

What does this PR do?

Motivation

Describe how to test/QA your changes

Possible Drawbacks / Trade-offs

Additional Notes

cit-pr-commenter bot commented Nov 5, 2024 • edited Loading

Go Package Import Differences

cit-pr-commenter bot commented Nov 5, 2024 • edited Loading

Regression Detector

Regression Detector Results

Optimization Goals: ❌ Significant changes detected

Fine details of change detection per experiment

Bounds Checks: ❌ Failed

Explanation

agent-platform-auto-pr bot commented Nov 7, 2024 • edited Loading

Test changes on VM

gjulianm commented Nov 8, 2024

dd-devflow bot commented Nov 8, 2024

Devflow running: /trigger-ci -v RUN_KMT_TESTS=on

paulcacheux Nov 11, 2024

Choose a reason for hiding this comment

gjulianm Nov 11, 2024

Choose a reason for hiding this comment

gjulianm Nov 11, 2024

Choose a reason for hiding this comment

paulcacheux Nov 11, 2024

Choose a reason for hiding this comment

paulcacheux Nov 11, 2024

Choose a reason for hiding this comment

gjulianm Nov 11, 2024 • edited Loading

Choose a reason for hiding this comment

gjulianm commented Nov 12, 2024

dd-devflow bot commented Nov 12, 2024 • edited Loading

Devflow running: /merge

gjulianm commented Nov 5, 2024 •

edited

Loading

cit-pr-commenter bot commented Nov 5, 2024 •

edited

Loading

cit-pr-commenter bot commented Nov 5, 2024 •

edited

Loading

agent-platform-auto-pr bot commented Nov 7, 2024 •

edited

Loading

Devflow running: `/trigger-ci -v RUN_KMT_TESTS=on`

gjulianm Nov 11, 2024 •

edited

Loading

dd-devflow bot commented Nov 12, 2024 •

edited

Loading

Devflow running: `/merge`