-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Implements file reading for peer tags to synchronize cross repo usage #25600
Implements file reading for peer tags to synchronize cross repo usage #25600
Conversation
Part of larger changes to come where common tag names are sourced from a single source of truth across DD repos The planned source of truth will generate static files which can be read by the agent code and synchronized across all DD repos involving peer tags
Probably going to shift away from the csv style but for now a place holder until @majorgreys decides on the final expected format to read values from |
Regression DetectorRegression Detector ResultsRun ID: 93cb6abb-3864-4870-8ca4-6628907c0c63 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 |
---|---|---|---|---|
➖ | tcp_syslog_to_blackhole | ingress throughput | +7.41 | [-14.80, +29.63] |
➖ | uds_dogstatsd_to_api_cpu | % cpu utilization | +2.37 | [-0.69, +5.43] |
➖ | basic_py_check | % cpu utilization | +1.76 | [-1.14, +4.65] |
➖ | file_tree | memory utilization | +1.02 | [+0.92, +1.12] |
➖ | pycheck_1000_100byte_tags | % cpu utilization | +0.96 | [-3.70, +5.61] |
➖ | tcp_dd_logs_filter_exclude | ingress throughput | +0.07 | [+0.02, +0.11] |
➖ | otel_to_otel_logs | ingress throughput | +0.02 | [-0.34, +0.39] |
➖ | uds_dogstatsd_to_api | ingress throughput | +0.00 | [-0.20, +0.20] |
➖ | trace_agent_json | ingress throughput | +0.00 | [-0.01, +0.01] |
➖ | trace_agent_msgpack | ingress throughput | -0.00 | [-0.00, +0.00] |
➖ | idle | memory utilization | -0.12 | [-0.16, -0.08] |
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=35476282 --os-family=ubuntu |
Goal is to have values stored in ini file which is now parsed as expected
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Go Package Import DifferencesBaseline: 344ecd7
|
Due to mismatches between remapping on backend and previous list, several tags are inserted and removed to ensure the final list is the same as before This may change in the future in order to ensure all behavior is consistent Tests are also added to ensure similar behavior
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #25600 +/- ##
===========================================
+ Coverage 45.08% 54.56% +9.47%
===========================================
Files 2340 456 -1884
Lines 269884 39061 -230823
===========================================
- Hits 121683 21312 -100371
+ Misses 138581 16776 -121805
+ Partials 9620 973 -8647
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Final format style is determined to be INI. PR is finished and ready for review but note there are a few mismatches between the backend and agent aggregation. To avoid any breaking changes, some tags are manually added and removed but that may change in the future once discussed with the stats team |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you do a quick sanity check on whether OTel collector can still build with this change? Basically
- get a pseudo version of pkg/trace & comp/otelcol/otlp/components/statsprocessor
- use it in https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/main/exporter/datadogexporter/go.mod#L25
- run
make otelcontribcol
Ran it locally by cloning the otel repo, replacing the packages with the local new versions, and ran the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approval for OTel-owned files
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM for Serverless
/merge |
🚂 MergeQueue Pull request added to the queue. This build is going to start soon! (estimated merge in less than 26m) Use |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM for agent-shared-components
This comment has been minimized.
This comment has been minimized.
BenchmarksBenchmark execution time: 2024-05-30 15:55:45 Comparing candidate commit 1f5cc9f in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 2 metrics, 1 unstable metrics. |
Serverless Benchmark Results
tl;drUse these benchmarks as an insight tool during development.
What is this benchmarking?The The benchmark is run using a large variety of lambda request payloads. In the charts below, there is one row for each event payload type. How do I interpret these charts?The charts below comes from The benchstat docs explain how to interpret these charts.
I need more helpFirst off, do not worry if the benchmarks are failing. They are not tests. The intention is for them to be a tool for you to use during development. If you would like a hand interpreting the results come chat with us in Benchmark stats
|
/merge |
🚂 MergeQueue Pull request added to the queue. There are 3 builds ahead! (estimated merge in less than 2h) Use |
What does this PR do?
Make the
defaultPeerTags
list read from a file so that file can be synchronized with other repos that use peer tagsPart of a larger set of changes to come where common tag names are sourced from a single source of truth across DD repos
The planned source of truth will generate static files which can be read by the agent code and synchronized across all DD repos involving peer tags
How to test
Ensure that service map behavior is unchanged namely the features related to
peer.*
tags. Specifically, the connections between services in the service map remain unchangedRun
TestPreparePeerTags
inconcentrator_test.go