-
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
[otel-agent] report and build with proper versioning #29334
Conversation
@@ -69,6 +69,7 @@ require ( | |||
github.com/DataDog/datadog-agent/comp/otelcol/configstore/def v0.56.0-rc.3 | |||
github.com/DataDog/datadog-agent/comp/otelcol/configstore/impl v0.56.0-rc.3 | |||
github.com/DataDog/datadog-agent/comp/otelcol/ddflareextension/def v0.0.0-00010101000000-000000000000 | |||
github.com/DataDog/datadog-agent/pkg/version v0.0.0-00010101000000-000000000000 |
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.
we import v0.57.0 here and here; does it make sense to make this consistent across the repo?
github.com/DataDog/datadog-agent/pkg/version v0.0.0-00010101000000-000000000000 | |
github.com/DataDog/datadog-agent/pkg/version v0.57.0 |
comp/otelcol/converter/impl/go.mod
Outdated
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.
recommend adding the replace for
github.com/DataDog/datadog-agent/pkg/version => ../../../../../pkg/version
on line 36
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.
Agreed. Yup.
5045eb2
to
96b0177
Compare
Test changes on VMUse this command from test-infra-definitions to manually test this PR changes on a VM: inv create-vm --pipeline-id=45302381 --os-family=ubuntu Note: This applies to commit e51aece |
Regression DetectorRegression Detector ResultsRun ID: 1f5ecf6a-4223-4d35-bf3e-1267a55434cd Metrics dashboard Target profiles Baseline: 2588d20 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 |
---|---|---|---|---|---|---|
➖ | idle_all_features | memory utilization | +0.99 | [+0.91, +1.06] | 1 | Logs |
➖ | pycheck_lots_of_tags | % cpu utilization | +0.72 | [-1.87, +3.31] | 1 | Logs |
➖ | basic_py_check | % cpu utilization | +0.52 | [-2.21, +3.25] | 1 | Logs |
➖ | uds_dogstatsd_to_api_cpu | % cpu utilization | +0.28 | [-0.45, +1.02] | 1 | Logs |
➖ | idle | memory utilization | +0.19 | [+0.15, +0.23] | 1 | Logs |
➖ | otel_to_otel_logs | ingress throughput | +0.06 | [-0.75, +0.86] | 1 | Logs |
➖ | tcp_dd_logs_filter_exclude | ingress throughput | +0.00 | [-0.01, +0.01] | 1 | Logs |
➖ | uds_dogstatsd_to_api | ingress throughput | -0.02 | [-0.11, +0.06] | 1 | Logs |
➖ | tcp_syslog_to_blackhole | ingress throughput | -0.06 | [-0.11, -0.01] | 1 | Logs |
➖ | file_tree | memory utilization | -0.22 | [-0.31, -0.13] | 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".
another thought: for BYOC, this requires users to set a distribution version number, right? Will it cause issues if they either don't specify a distribution version number or have a number different than the underlying components? |
96b0177
to
d285f2f
Compare
d285f2f
to
e51aece
Compare
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 DevX Infra owned file !
/merge |
🚂 MergeQueue: waiting for PR to be ready This merge request is not mergeable yet, because of pending checks/missing approvals. It will be added to the queue as soon as checks pass and/or get approvals. Use |
🚂 MergeQueue: pull request added to the queue The median merge time in Use |
What does this PR do?
This PR attempts to introduce proper versioning for the
otel-agent
. The goal is thatotel-agent
andagent
are aligned with the respectivepkg/version
versioning pattern. We also still need to understand what OTel Collector version is being used (currently reported in BuildInfo asv0.104.0
) and submitted in themetadata
as the extension version).Motivation
Proper version tracking for the
otel-agent
and the OTel Collector internals moving forward.Additional Notes
We may want to add additional fields to the OTel metadata payload. We may need to discern between:
otel-agent
versionextension
versioncollector component
versionPossible Drawbacks / Trade-offs
Describe how to test/QA your changes
otel-agent version
andagent version
commands should reflect the same version.