[usm] service discovery, fetch only target environment variables from /proc #29574
Conversation
yuri-lipnesh
added
team/usm
Regression DetectorRegression Detector ResultsRun ID: 08edd294-d1eb-4fc3-8110-41a2c936e7a9 Metrics dashboard Target profiles Baseline: 10d974e Performance changes are noted in the perf column of each table:
No significant changes in experiment optimization goalsConfidence level: 90.00% There were no significant changes in experiment optimization goals at this confidence level and effect size tolerance.
|
| perf | experiment | goal | Δ mean % | Δ mean % CI | trials | links |
|---|---|---|---|---|---|---|
| ➖ | uds_dogstatsd_to_api_cpu | % cpu utilization | +3.15 | [+2.41, +3.90] | 1 | Logs |
| ➖ | tcp_syslog_to_blackhole | ingress throughput | +2.16 | [+2.06, +2.26] | 1 | Logs |
| ➖ | file_tree | memory utilization | +1.81 | [+1.71, +1.92] | 1 | Logs |
| ➖ | otel_to_otel_logs | ingress throughput | +1.30 | [+0.48, +2.11] | 1 | Logs |
| ➖ | idle | memory utilization | +0.57 | [+0.51, +0.62] | 1 | Logs |
| ➖ | pycheck_lots_of_tags | % cpu utilization | +0.20 | [-2.29, +2.69] | 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.08] | 1 | Logs |
| ➖ | basic_py_check | % cpu utilization | -2.29 | [-4.97, +0.39] | 1 | Logs |
Bounds Checks
| perf | experiment | bounds_check_name | replicates_passed |
|---|---|---|---|
| ✅ | 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".
Test changes on VMUse this command from test-infra-definitions to manually test this PR changes on a VM: inv create-vm --pipeline-id=46302915 --os-family=ubuntuNote: This applies to commit 45cc90a |
… target environment variables
Go Package Import DifferencesBaseline: c5bcbc1
|
pkg/collector/corechecks/servicediscovery/targetenvs/envscanner.go
Outdated
Show resolved
Hide resolved
…sed field in EnvReader struct.
…ironmentVariables.Set()
…ped env variable search
| if _, ok := targets[name]; !ok { | ||
| return "", false | ||
| } | ||
|
|
||
| val, ok := ev.vars[name] | ||
| return val, ok |
There was a problem hiding this comment.
imho, vars should reflect only the allowed environment variables
no need to store (and waste memory) unneeded envs + currently you always pay for 2 map accesses per "supported" environment variable instead of only one map access
There was a problem hiding this comment.
If vars is guaranteed to have only supported environment variables, then no need to check targets first
There was a problem hiding this comment.
This is where unnecessary environment variables are filtered out and ultimately only the requested variables are included in the list.
| }, nil | ||
| } | ||
|
|
||
| // finish closes an open file. |
…parate function used for testing only.
… some cosmetic changes
…iables.GetDefault to satisfy props.PropertyGetter needed in spring.go
….GetDefault() to satisfy linter.
What does this PR do?
Creates a new function to read /proc/pid/environ in chunks and save only the target environment variables.
Motivation
A process may have a very long list of environment variables, and some of them may contain sensitive data.
Extracting the entire list of all environment variables into memory may be expensive and unsafe.
Reading the environment file chunks consumes less runtime memory.
Extracting only the variables of interest prevents sensitive data from being loaded into memory and potentially leaked.
Describe how to test/QA your changes
New unit tests compare results of new function to fetch environment variables to results of previously implemented function.
Possible Drawbacks / Trade-offs
Additional Notes
Benchmark comparison with previously implemented env. search function which read entire file into memory.
runtime improved by 20%
bytes allocation improved 2.2 times
number of allocations increased 2 times