process-agent: use the new statsd component #21108

iksaif · 2023-11-27T12:35:03Z

What does this PR do?

Use use the new statsd component

For now we use the shared factory. We might want to change the component later to provide a pre-configured client later, but this requires more changes.

Motivation

To be sure all agents use STATSD_URL and the same options.

Additional Notes

N/A

Possible Drawbacks / Trade-offs

N/A

Describe how to test/QA your changes

Validate that statsd metrics are still being sent by the process agent
When started and pointed to a validate core agent it should at least datadog.process.agent with a value of 1

Reviewer's Checklist

For now we use the shared factory. We might want to change the component later to provide a pre-configured client later, but this requires more changes.

cit-pr-commenter · 2023-11-27T12:47:51Z

Go Package Import Differences

Baseline: cfb86ca
Comparison: b3e8b1c

binary	os	arch	change
process-agent	linux	amd64	+1, -0 +github.com/DataDog/datadog-agent/comp/dogstatsd/statsd
process-agent	linux	arm64	+1, -0 +github.com/DataDog/datadog-agent/comp/dogstatsd/statsd
process-agent	windows	amd64	+1, -0 +github.com/DataDog/datadog-agent/comp/dogstatsd/statsd
process-agent	darwin	amd64	+1, -0 +github.com/DataDog/datadog-agent/comp/dogstatsd/statsd
process-agent	darwin	arm64	+1, -0 +github.com/DataDog/datadog-agent/comp/dogstatsd/statsd
heroku-process-agent	linux	amd64	+1, -0 +github.com/DataDog/datadog-agent/comp/dogstatsd/statsd
system-probe	linux	amd64	+1, -0 +github.com/DataDog/datadog-agent/comp/dogstatsd/statsd
system-probe	linux	arm64	+1, -0 +github.com/DataDog/datadog-agent/comp/dogstatsd/statsd
system-probe	windows	amd64	+1, -0 +github.com/DataDog/datadog-agent/comp/dogstatsd/statsd

pr-commenter · 2023-11-27T15:14:27Z

Bloop Bleep... Dogbot Here

Regression Detector Results

Run ID: a4ae8baf-23e7-4b67-97b9-b400b5714fdb
Baseline: bd4a9fb
Comparison: 16c9072
Total CPUs: 7

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

✅ denotes better comparison variant performance
❌ denotes worse comparison variant performance

Explanation

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".

No interesting changes in experiment optimization goals with confidence ≥ 90.00% and |Δ mean %| ≥ 5.00%.

Declared stable experiments that are now erratic

An experiment is erratic (i.e., not stable) if its coefficient of variation is at least 0.10.

perf	experiment	goal	Δ mean %	Δ mean % CI	confidence
✅	otel_to_otel_logs	ingress throughput	+0.48	[-0.21, +1.16]	74.28%

Experiments with missing or malformed data

idle

Usually, this warning means that there is no usable optimization goal data for that experiment, which could be a result of misconfiguration.

Fine details of change detection per experiment

perf	experiment	goal	Δ mean %	Δ mean % CI	confidence
✅	otel_to_otel_logs	ingress throughput	+0.48	[-0.21, +1.16]	74.28%
❌	trace_agent_msgpack	ingress throughput	+0.00	[-0.00, +0.00]	0.00%
❌	uds_dogstatsd_to_api	ingress throughput	+0.00	[-0.00, +0.00]	0.00%
❌	dogstatsd_string_interner_8MiB_1k	ingress throughput	+0.00	[-0.00, +0.00]	0.00%
❌	dogstatsd_string_interner_64MiB_1k	ingress throughput	+0.00	[-0.00, +0.00]	0.00%
❌	dogstatsd_string_interner_8MiB_10k	ingress throughput	+0.00	[-0.00, +0.00]	0.00%
❌	dogstatsd_string_interner_8MiB_50k	ingress throughput	+0.00	[-0.00, +0.00]	0.00%
❌	trace_agent_json	ingress throughput	+0.00	[-0.01, +0.01]	0.00%
❌	dogstatsd_string_interner_64MiB_100	ingress throughput	+0.00	[-0.00, +0.00]	0.00%
❌	dogstatsd_string_interner_8MiB_100	ingress throughput	+0.00	[-0.00, +0.00]	0.00%
❌	tcp_dd_logs_filter_exclude	ingress throughput	+0.00	[-0.00, +0.00]	0.00%
❌	dogstatsd_string_interner_128MiB_1k	ingress throughput	+0.00	[-0.00, +0.00]	0.00%
❌	dogstatsd_string_interner_128MiB_100	ingress throughput	+0.00	[-0.00, +0.00]	0.00%
❌	dogstatsd_string_interner_8MiB_100k	ingress throughput	-0.00	[-0.04, +0.04]	1.76%
❌	file_tree	egress throughput	-0.02	[-1.82, +1.78]	1.37%
❌	file_to_blackhole	egress throughput	-0.07	[-6.23, +6.09]	1.60%
❌	tcp_syslog_to_blackhole	ingress throughput	-0.09	[-0.16, -0.03]	98.58%

brycekahle · 2023-11-27T19:04:52Z

cmd/system-probe/subcommands/run/command.go

@@ -90,7 +92,7 @@ func Commands(globalParams *command.GlobalParams) []*cobra.Command {
 }

 // run starts the main loop.
-func run(log log.Component, config config.Component, telemetry telemetry.Component, sysprobeconfig sysprobeconfig.Component, rcclient rcclient.Component, cliParams *cliParams) error {
+func run(log log.Component, statsd compstatsd.Component, telemetry telemetry.Component, sysprobeconfig sysprobeconfig.Component, rcclient rcclient.Component, cliParams *cliParams) error {


config.Component is still required.

do you know how to make the linter happier when modifying this function then? Or do we just ignore it ?

You can use _ config.Component instead of config config.Component

pkg/process/statsd/statsd.go

brycekahle

Please re-add the config.Component argument which was removed

iksaif · 2023-11-27T19:30:33Z

Please re-add the config.Component argument which was removed

It was removed because unused and creating linter errors.

brycekahle · 2023-11-27T20:00:28Z

Please re-add the config.Component argument which was removed

It was removed because unused and creating linter errors.

Please name the argument _ to satisfy the linter. It is necessary to ensure the datadog config is read and parsed.

…-process-agent

iksaif · 2023-12-04T08:47:24Z

The latest commits address your comments, the failing test seems unrelated

kkhor-datadog

Mind elaborating on the QA instructions a little? For the process agent a good metric to check would be datadog.process.agent:

datadog-agent/pkg/process/runner/submitter.go

Line 277 in 38b4788

statsd.Client.Gauge("datadog.process.agent", 1, tags, 1) //nolint:errcheck

iksaif · 2023-12-05T14:31:33Z

Mind elaborating on the QA instructions a little? For the process agent a good metric to check would be datadog.process.agent:

datadog-agent/pkg/process/runner/submitter.go

Line 277 in 38b4788

statsd.Client.Gauge("datadog.process.agent", 1, tags, 1) //nolint:errcheck

Good suggestion, is that better? (I've never done QA myself so I'm not sure which amount of details we need to put there)

…-process-agent

iksaif · 2023-12-07T13:11:31Z

re-merged with main, resolved conflicts, and explained why we need a NoopClient{}

…-process-agent

process-agent: use the new statsd component

89f0c40

For now we use the shared factory. We might want to change the component later to provide a pre-configured client later, but this requires more changes.

iksaif requested review from a team as code owners November 27, 2023 12:35

github-actions bot added the component/system-probe label Nov 27, 2023

iksaif added changelog/no-changelog team/processes and removed component/system-probe labels Nov 27, 2023

iksaif added this to the 7.51.0 milestone Nov 27, 2023

Make the linter happy

278dcf6

github-actions bot added the component/system-probe label Nov 27, 2023

brycekahle reviewed Nov 27, 2023

View reviewed changes

pkg/process/statsd/statsd.go Show resolved Hide resolved

brycekahle requested changes Nov 27, 2023

View reviewed changes

iksaif added 2 commits November 28, 2023 08:54

Merge remote-tracking branch 'origin/main' into corentin.chary/statsd…

afe86b1

…-process-agent

system-probe: add config.Component to run()

334078f

kkhor-datadog reviewed Dec 4, 2023

View reviewed changes

kkhor-datadog approved these changes Dec 5, 2023

View reviewed changes

iksaif requested review from brycekahle and kkhor-datadog December 5, 2023 15:46

kkhor-datadog approved these changes Dec 6, 2023

View reviewed changes

Merge remote-tracking branch 'origin/main' into corentin.chary/statsd…

dcf2650

…-process-agent

brycekahle approved these changes Dec 7, 2023

View reviewed changes

Merge remote-tracking branch 'origin/main' into corentin.chary/statsd…

16c9072

…-process-agent

Merge remote-tracking branch 'origin/main' into corentin.chary/statsd…

b3e8b1c

…-process-agent

iksaif merged commit 939b135 into main Dec 12, 2023
147 of 149 checks passed

iksaif deleted the corentin.chary/statsd-process-agent branch December 12, 2023 14:48

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

process-agent: use the new statsd component #21108

process-agent: use the new statsd component #21108

iksaif commented Nov 27, 2023 •

edited

Loading

cit-pr-commenter bot commented Nov 27, 2023 •

edited

Loading

pr-commenter bot commented Nov 27, 2023 •

edited

Loading

Explanation

Declared stable experiments that are now erratic

Fine details of change detection per experiment

brycekahle Nov 27, 2023

iksaif Nov 27, 2023

paulcacheux Nov 27, 2023

brycekahle left a comment

iksaif commented Nov 27, 2023

brycekahle commented Nov 27, 2023

iksaif commented Dec 4, 2023

kkhor-datadog left a comment

iksaif commented Dec 5, 2023

iksaif commented Dec 7, 2023

process-agent: use the new statsd component #21108

process-agent: use the new statsd component #21108

Conversation

iksaif commented Nov 27, 2023 • edited Loading

What does this PR do?

Motivation

Additional Notes

Possible Drawbacks / Trade-offs

Describe how to test/QA your changes

Reviewer's Checklist

cit-pr-commenter bot commented Nov 27, 2023 • edited Loading

Go Package Import Differences

pr-commenter bot commented Nov 27, 2023 • edited Loading

Bloop Bleep... Dogbot Here

Regression Detector Results

Explanation

Declared stable experiments that are now erratic

Experiments with missing or malformed data

Fine details of change detection per experiment

brycekahle Nov 27, 2023

Choose a reason for hiding this comment

iksaif Nov 27, 2023

Choose a reason for hiding this comment

paulcacheux Nov 27, 2023

Choose a reason for hiding this comment

brycekahle left a comment

Choose a reason for hiding this comment

iksaif commented Nov 27, 2023

brycekahle commented Nov 27, 2023

iksaif commented Dec 4, 2023

kkhor-datadog left a comment

Choose a reason for hiding this comment

iksaif commented Dec 5, 2023

iksaif commented Dec 7, 2023

iksaif commented Nov 27, 2023 •

edited

Loading

cit-pr-commenter bot commented Nov 27, 2023 •

edited

Loading

pr-commenter bot commented Nov 27, 2023 •

edited

Loading