Start dropping python2 support #30035

chouquette · 2024-10-11T07:28:35Z

What does this PR do?

Start dropping python2 as we don't ship it with the agent anymore.
It handles:

the CI job parameters
rtloader
omnibus
The MSI package

Motivation

We don't need to clutter the code with various parameters or branches for something we don't support anymore.

There are most likely some missing pieces and a lot of remaining references to python2, but this serves as a good starting point

Describe how to test/QA your changes

I'm not sure this needs dedicated QA as long as the CI still passes since we're only removing code, but since the change is large, I can definitely convinced otherwise

Possible Drawbacks / Trade-offs

Additional Notes

This is probably better review commit by commit. The changes should be very close to atomic, and things can be split out in multiple PRs if we chose to.

Associated PRs:

It's now always set to 3

We are breaking backward compatibility quite a bit here

cit-pr-commenter · 2024-10-11T07:57:37Z

Regression Detector

Regression Detector Results

Run ID: c2fcdb13-bc86-4415-9f99-6d4bd78ab9e4 Metrics dashboard Target profiles

Baseline: ade743b
Comparison: 80ee5b4

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

No significant changes in experiment optimization goals

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

There were no significant changes in experiment optimization goals at this confidence level and effect size tolerance.

Fine details of change detection per experiment

perf	experiment	goal	Δ mean %	Δ mean % CI	trials	links
➖	pycheck_lots_of_tags	% cpu utilization	+1.33	[-1.18, +3.84]	1	Logs
➖	file_tree	memory utilization	+1.00	[+0.87, +1.12]	1	Logs
➖	file_to_blackhole_500ms_latency	egress throughput	+0.16	[-0.08, +0.40]	1	Logs
➖	file_to_blackhole_300ms_latency	egress throughput	+0.07	[-0.11, +0.25]	1	Logs
➖	file_to_blackhole_100ms_latency	egress throughput	+0.01	[-0.21, +0.23]	1	Logs
➖	file_to_blackhole_0ms_latency	egress throughput	+0.01	[-0.34, +0.35]	1	Logs
➖	tcp_dd_logs_filter_exclude	ingress throughput	-0.00	[-0.01, +0.01]	1	Logs
➖	uds_dogstatsd_to_api	ingress throughput	-0.01	[-0.10, +0.09]	1	Logs
➖	uds_dogstatsd_to_api_cpu	% cpu utilization	-0.09	[-0.81, +0.63]	1	Logs
➖	otel_to_otel_logs	ingress throughput	-0.12	[-0.93, +0.69]	1	Logs
➖	idle_all_features	memory utilization	-0.25	[-0.33, -0.16]	1	Logs
➖	file_to_blackhole_1000ms_latency	egress throughput	-0.53	[-1.01, -0.05]	1	Logs
➖	idle	memory utilization	-0.65	[-0.69, -0.61]	1	Logs
➖	tcp_syslog_to_blackhole	ingress throughput	-0.74	[-0.78, -0.69]	1	Logs
➖	basic_py_check	% cpu utilization	-1.44	[-4.09, +1.20]	1	Logs

Bounds Checks

perf	experiment	bounds_check_name	replicates_passed
✅	file_to_blackhole_0ms_latency	memory_usage	10/10
✅	file_to_blackhole_1000ms_latency	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
✅	idle	memory_usage	10/10

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

CelianR

Good job! Added minor suggestions

CelianR · 2024-10-11T08:31:05Z

rtloader/common/builtins/datadog_agent.c

    Py_END_ALLOW_THREADS

-    if (v == NULL) {
+        if (v == NULL)


I think this should be de-dented right?

Suggested change

if (v == NULL)

if (v == NULL)

CelianR · 2024-10-11T08:32:17Z

tasks/agent.py

+    if python_runtimes:
+        print(
+            color_message(
+                '--python-runtimes is deprecated and will always be 3. Please remove this parametes from your scripts',


Suggested change

'--python-runtimes is deprecated and will always be 3. Please remove this parametes from your scripts',

'Warning: --python-runtimes is deprecated and will always be 3. Please remove this parameter from your scripts',

CelianR · 2024-10-11T08:33:00Z

tasks/github_tasks.py

+    if python_runtimes:
+        print(
+            color_message(
+                '--python-runtimes is deprecated and will always be 3. Please remove this parameter from your scripts',


Suggested change

'--python-runtimes is deprecated and will always be 3. Please remove this parameter from your scripts',

'Warning: --python-runtimes is deprecated and will always be 3. Please remove this parameter from your scripts',

CelianR · 2024-10-11T08:33:17Z

tasks/gotest.py

+    if python_runtimes:
+        print(
+            color_message(
+                '--python-runtimes is deprecated and will always be 3. Please remove this parameter from your scripts',


Suggested change

'--python-runtimes is deprecated and will always be 3. Please remove this parameter from your scripts',

'Warning: --python-runtimes is deprecated and will always be 3. Please remove this parameter from your scripts',

CelianR · 2024-10-11T08:33:42Z

tasks/msi.py

+    if python_runtimes:
+        print(
+            color_message(
+                '--python-runtimes is deprecated and will always be 3. Please remove this parameter from your scripts',


Suggested change

'--python-runtimes is deprecated and will always be 3. Please remove this parameter from your scripts',

'Warning: --python-runtimes is deprecated and will always be 3. Please remove this parameter from your scripts',

CelianR · 2024-10-11T08:34:34Z

tasks/trace_agent.py

+    if python_runtimes:
+        print(
+            color_message(
+                '--python-runtimes is deprecated and will always be 3. Please remove this parametes from your scripts',


Suggested change

'--python-runtimes is deprecated and will always be 3. Please remove this parametes from your scripts',

'Warning: --python-runtimes is deprecated and will always be 3. Please remove this parameter from your scripts',

CelianR · 2024-10-11T08:34:43Z

tasks/system_probe.py

+    if python_runtimes:
+        print(
+            color_message(
+                '--python-runtimes is deprecated and will always be 3. Please remove this parametes from your scripts',


Suggested change

'--python-runtimes is deprecated and will always be 3. Please remove this parametes from your scripts',

'Warning: --python-runtimes is deprecated and will always be 3. Please remove this parameter from your scripts',

CelianR · 2024-10-11T08:35:01Z

tasks/system_probe.py

+    if python_runtimes:
+        print(
+            color_message(
+                '--python-runtimes is deprecated and will always be 3. Please remove this parametes from your scripts',


Suggested change

'--python-runtimes is deprecated and will always be 3. Please remove this parametes from your scripts',

'Warning: --python-runtimes is deprecated and will always be 3. Please remove this parameter from your scripts',

CelianR · 2024-10-11T08:35:14Z

tasks/rtloader.py

+    if python_runtimes:
+        print(
+            color_message(
+                '--python-runtimes is deprecated and will always be 3. Please remove this parametes from your scripts',


Suggested change

'--python-runtimes is deprecated and will always be 3. Please remove this parametes from your scripts',

'Warning: --python-runtimes is deprecated and will always be 3. Please remove this parameter from your scripts',

CelianR · 2024-10-11T08:35:40Z

tasks/process_agent.py

+    if python_runtimes:
+        print(
+            color_message(
+                '--python-runtimes is deprecated and will always be 3. Please remove this parameter from your scripts',


Suggested change

'--python-runtimes is deprecated and will always be 3. Please remove this parameter from your scripts',

'Warning: --python-runtimes is deprecated and will always be 3. Please remove this parameter from your scripts',

julien-lebot

There's some diff issue & the PR is missing the changes to the Python compression & decompression in the MSI installer.

julien-lebot · 2024-10-11T08:35:19Z

tools/windows/DatadogAgentInstaller/WixSetup/Datadog Agent/AgentInstaller.cs

-        public event XDocumentFormatedDlgt WixSourceFormated;
-    }
-}
+using Datadog.AgentCustomActions;


Could you undo the line ending changes in this file so that it shows what actually changed? Right now the diff shows that the entire file changed which makes it very difficult to review.

gjulianm

I have a suggestion for the tasks code

gjulianm · 2024-10-11T09:12:01Z

tasks/system_probe.py

@@ -618,6 +618,13 @@ def build(
    """
    Build the system-probe
    """
+    if python_runtimes:


One suggestion, wouldn't it be better to extract these calls to a single warn_deprecation_if_used(python_runtimes) common function to be used everywhere? It'd also make it easier to track later which parameters are still active)

chouquette added 11 commits October 10, 2024 12:32

CI: don't pass python runtime to invoke tasks

052ad0a

It's now always set to 3

tasks: rtloader: drop python version selection

04293e3

rtloader: drop python2 support

3e36f31

tasks: deprecate --python-runtimes parameter

b51375e

omnibus: drop python2 support

5ff8bd8

MSI: drop python2 support

e317d38

win: drop python2 support

f029352

doc: remove mention of python2

2f0a061

pkg/collector: drop python2 support

6cd267d

bump rtloader soversion

027fec3

We are breaking backward compatibility quite a bit here

target an omnibus software wip branch

80ee5b4

chouquette added qa/no-code-change Skip QA week as there is no code change in Agent code team/agent-delivery labels Oct 11, 2024

github-actions bot added the component/system-probe label Oct 11, 2024

chouquette marked this pull request as ready for review October 11, 2024 08:22

chouquette requested review from a team as code owners October 11, 2024 08:22

chouquette requested a review from a team as a code owner October 11, 2024 08:22

pducolin approved these changes Oct 11, 2024

View reviewed changes

CelianR approved these changes Oct 11, 2024

View reviewed changes

julien-lebot requested changes Oct 11, 2024

View reviewed changes

gjulianm reviewed Oct 11, 2024

View reviewed changes

pgimalac approved these changes Oct 11, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Start dropping python2 support #30035

Start dropping python2 support #30035

chouquette commented Oct 11, 2024 •

edited

Loading

cit-pr-commenter bot commented Oct 11, 2024

Fine details of change detection per experiment

Explanation

CelianR left a comment

CelianR Oct 11, 2024

CelianR Oct 11, 2024

CelianR Oct 11, 2024

CelianR Oct 11, 2024

CelianR Oct 11, 2024

CelianR Oct 11, 2024

CelianR Oct 11, 2024

CelianR Oct 11, 2024

CelianR Oct 11, 2024

CelianR Oct 11, 2024

julien-lebot left a comment

julien-lebot Oct 11, 2024

gjulianm left a comment •

edited

Loading

gjulianm Oct 11, 2024

	'--python-runtimes is deprecated and will always be 3. Please remove this parametes from your scripts',
	'Warning: --python-runtimes is deprecated and will always be 3. Please remove this parameter from your scripts',

Start dropping python2 support #30035

Are you sure you want to change the base?

Start dropping python2 support #30035

Conversation

chouquette commented Oct 11, 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 Oct 11, 2024

Regression Detector

Regression Detector Results

No significant changes in experiment optimization goals

Fine details of change detection per experiment

Bounds Checks

Explanation

CelianR left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

julien-lebot left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gjulianm left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chouquette commented Oct 11, 2024 •

edited

Loading

gjulianm left a comment •

edited

Loading