[NPM] use config to size conn_close_flushed map

[akarpz]

What does this PR do?

Instead of hardcoding the allocated max entry count of conn_close_flushed, set it as a function of the max connections tracked. With defaults this is a no-op as the current size of the map is 1/4 the default max_connections_tracked.

Also stop inserting into conn_close_flushed in tcp_close as there is no mechanism for tcp_done to flush connections after tcp_close executes for a connection anyway

Motivation

Describe how to test/QA your changes

Possible Drawbacks / Trade-offs

Additional Notes

[agent-platform-auto-pr]

[Fast Unit Tests Report]

On pipeline 48062785 (CI Visibility). The following jobs did not run any unit tests:

Jobs:

• tests_deb-arm64-py3
• tests_deb-x64-py3
• tests_flavor_dogstatsd_deb-x64
• tests_flavor_heroku_deb-x64
• tests_flavor_iot_deb-x64
• tests_rpm-arm64-py3
• tests_rpm-x64-py3
• tests_windows-x64

If you modified Go files and expected unit tests to run in these jobs, please double check the job logs. If you think tests should have been executed reach out to #agent-devx-help

[agent-platform-auto-pr]

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=48062785 --os-family=ubuntu

Note: This applies to commit e05e8d9

[cit-pr-commenter]

Regression Detector

Regression Detector Results

Run ID: bca89d71-7464-42d5-a334-5cacb44e258c Metrics dashboard Target profiles

Baseline: a0e73e3
Comparison: e05e8d9

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	+2.47	[-0.09, +5.02]	1	Logs
➖	file_tree	memory utilization	+1.90	[+1.74, +2.06]	1	Logs
➖	idle_all_features	memory utilization	+0.91	[+0.78, +1.04]	1	Logs bounds checks dashboard
➖	tcp_syslog_to_blackhole	ingress throughput	+0.10	[+0.04, +0.15]	1	Logs
➖	file_to_blackhole_300ms_latency	egress throughput	+0.10	[-0.08, +0.28]	1	Logs
➖	file_to_blackhole_500ms_latency	egress throughput	+0.05	[-0.20, +0.29]	1	Logs
➖	basic_py_check	% cpu utilization	+0.03	[-2.70, +2.76]	1	Logs
➖	uds_dogstatsd_to_api	ingress throughput	+0.01	[-0.10, +0.13]	1	Logs
➖	tcp_dd_logs_filter_exclude	ingress throughput	-0.00	[-0.01, +0.01]	1	Logs
➖	file_to_blackhole_100ms_latency	egress throughput	-0.01	[-0.23, +0.21]	1	Logs
➖	file_to_blackhole_0ms_latency	egress throughput	-0.01	[-0.36, +0.33]	1	Logs
➖	quality_gate_idle	memory utilization	-0.25	[-0.30, -0.19]	1	Logs bounds checks dashboard
➖	file_to_blackhole_1000ms_latency	egress throughput	-0.26	[-0.75, +0.23]	1	Logs
➖	idle	memory utilization	-0.32	[-0.38, -0.26]	1	Logs bounds checks dashboard
➖	uds_dogstatsd_to_api_cpu	% cpu utilization	-0.44	[-1.16, +0.28]	1	Logs
➖	otel_to_otel_logs	ingress throughput	-0.90	[-1.70, -0.10]	1	Logs
➖	quality_gate_idle_all_features	memory utilization	-3.39	[-3.51, -3.26]	1	Logs bounds checks dashboard

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
✅	idle_all_features	memory_usage	10/10
✅	quality_gate_idle	memory_usage	10/10
✅	quality_gate_idle_all_features	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:

1. Its estimated |Δ mean %| ≥ 5.00%, indicating the change is big enough to merit a closer look.

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

3. Its configuration does not mark it "erratic".

[usamasaqib]

I am not sure I follow the reasoning for suppressing E2BIG errors.
Error suppression should only be used for errors we cannot do anything about, or are expected due to ABI usage. E2BIG does not fall into this category imo.

[hmahmood]

+1, I think we should not suppress E2BIG

[akarpz]

basically this map is used as a cache for closed connection flushes. if the cache fills up, we will not clean up closed connections (and some other maps will leak). I am fine with focusing on making the cache not fill up in the first place instead, but as a backstop I think it would have been useful to allow this codepath to flush connection closures.

edit: didn't realize this was only for telemetry, I'll change it

[hmahmood]

Wouldn't the tcp_done probe race with this one here to clean up the connection? We will still flush an empty connection in cleanup_conn whether a "real" connection is flushed here.

[akarpz]

consider a few cases:

• tcp_done first, then tcp_close: conn_close_flushed map is cleaned up (NEW), no double flush happens
• tcp_close first, then tcp_done: tcp_done relies on fetching the PID from the ongoing_connect_pid map, which is cleaned up in tcp_close. So in this case, tcp_done will bail early and never set an entry in the conn_close_flushed map or flush any connection.
• tcp_done only: entry in conn_close_flushed leaks, cleaned by map cleaner. connection flushed, and entry in ongoing_connect_pid deleted too.
• tcp_close only: no entry created in conn_close_flushed (NEW), ongoing_connect_pid entry deleted
• both probes execute concurrently. I'm not sure much changes here with this change. Before we could have flushed a connection twice in an edge case, now that is still possible.

[hmahmood]

The last case is what I don't think works anymore. With the concurrent update to conn_close_flushed by both probes we had a mutex of sorts going; that is not present anymore, and so you could call cleanup_conn twice while before you wouldn't. Or am I misreading this?

[akarpz]

hm okay I think you're right.. we can't get rid of this. I'm going to close this PR and open another one with just the config changes for this map and one other I see filling up consistently