fix(ci): pin github actions per commit-sha #25291
Conversation
chouetz
added
changelog/no-changelog
qa/no-code-change
| @@ -26,25 +26,25 @@ jobs: | |||
| docker rmi $(docker image ls -aq) >/dev/null 2>&1 | |||
|
|
|||
| - name: Checkout datadog-agent repository | |||
| uses: actions/checkout@v4 | |||
| uses: actions/checkout@0ad4b8fadaa221de15dcec353f45205ec38ea70b # v4.1.4 | |||
There was a problem hiding this comment.
I would much rather we don't use comments with the version here since it will get out of sync if we merge a dependabot PR updating the sha
There was a problem hiding this comment.
If I believe this I think it will be synced
|
[Fast Unit Tests Report] On pipeline 33414270 (CI Visibility). The following jobs did not run any unit tests: Jobs:
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-developer-experience |
| @@ -18,7 +18,7 @@ jobs: | |||
| GH_REPO: ${{ github.repository }} | |||
| steps: | |||
| - name: Checkout datadog-agent repository | |||
| uses: actions/checkout@v4 | |||
| uses: actions/checkout@0ad4b8fadaa221de15dcec353f45205ec38ea70b # v4.1.4 | |||
There was a problem hiding this comment.
Would it make sense to have a centralized file where these version->hash maps are kept? Maybe we can also inject those as values into the build scripts? Without something like that, verification of proper hash value use becomes extremely had to maintain and review.
There was a problem hiding this comment.
I'll check if we can do this. It could remove some redundancy even if the updates are normally handled automatically by dependabot (so mitigates a little bit the maintenance complexity, wdyt?)
There was a problem hiding this comment.
Fully agree with Srdjan on this !
There was a problem hiding this comment.
I really disagree with the single file approach, we know in advance it will never be maintained and upgrades will never be made. Keeping the format easy for dependabot to do the upgrades on its own should be the main priority IMO.
There was a problem hiding this comment.
@paulcacheux Hmm - that's a fair point. Any other alternatives to make this both dependabot and human easily digestible/maintainable without using the old tags? I'm afraid with hashes approach, nobody will actually check these in updates and something might slip by.
There was a problem hiding this comment.
We still have the associated tag in the comment for human-readable format. Would you prefer having a link to the release on the corresponding github repo?
There was a problem hiding this comment.
@chouetz That may be a good compromise though a link to the repo at that tag release would be better I think since the hash can easily be compared.
Regression DetectorRegression Detector ResultsRun ID: d02895ce-66cd-4294-8b5f-eb8d471a0b9f 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 |
|---|---|---|---|---|
| ✅ | file_to_blackhole | % cpu utilization | -34.20 | [-39.00, -29.41] |
Fine details of change detection per experiment
| perf | experiment | goal | Δ mean % | Δ mean % CI |
|---|---|---|---|---|
| ➖ | tcp_syslog_to_blackhole | ingress throughput | +6.72 | [-15.18, +28.61] |
| ➖ | pycheck_1000_100byte_tags | % cpu utilization | +2.54 | [-2.33, +7.41] |
| ➖ | process_agent_standard_check | memory utilization | +0.88 | [+0.83, +0.93] |
| ➖ | basic_py_check | % cpu utilization | +0.17 | [-2.41, +2.75] |
| ➖ | uds_dogstatsd_to_api_cpu | % cpu utilization | +0.16 | [-2.66, +2.98] |
| ➖ | trace_agent_json | ingress throughput | +0.00 | [-0.01, +0.01] |
| ➖ | trace_agent_msgpack | ingress throughput | -0.01 | [-0.02, +0.00] |
| ➖ | uds_dogstatsd_to_api | ingress throughput | -0.01 | [-0.22, +0.19] |
| ➖ | tcp_dd_logs_filter_exclude | ingress throughput | -0.02 | [-0.05, +0.02] |
| ➖ | otel_to_otel_logs | ingress throughput | -0.06 | [-0.42, +0.30] |
| ➖ | process_agent_real_time_mode | memory utilization | -0.17 | [-0.22, -0.12] |
| ➖ | idle | memory utilization | -0.23 | [-0.27, -0.19] |
| ➖ | file_tree | memory utilization | -0.25 | [-0.34, -0.16] |
| ➖ | process_agent_standard_check_with_stats | memory utilization | -0.34 | [-0.39, -0.28] |
| ✅ | file_to_blackhole | % cpu utilization | -34.20 | [-39.00, -29.41] |
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".
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 |
| - tasks/system_probe.py # invoke tasks | ||
|
|
||
|
|
||
| - changed-files: |
There was a problem hiding this comment.
| - changed-files: | |
| - changed-files: |
❓ question: Isn't it missing some spaces ?
There was a problem hiding this comment.
I used the very same format as in the documentation, and I guess yaml understands both, so I think it's equal
| @@ -18,7 +18,7 @@ jobs: | |||
| GH_REPO: ${{ github.repository }} | |||
| steps: | |||
| - name: Checkout datadog-agent repository | |||
| uses: actions/checkout@v4 | |||
| uses: actions/checkout@0ad4b8fadaa221de15dcec353f45205ec38ea70b # v4.1.4 | |||
There was a problem hiding this comment.
Fully agree with Srdjan on this !
|
/merge |
|
🚂 MergeQueue Pull request added to the queue. There are 4 builds ahead! (estimated merge in less than 2h) Use |
* fix(ci): pin github actions per commit-sha * fix the labeler configuration according to bump on v5 * revert codeql to 3.24.10 as 3.25 requires a new version of cli
What does this PR do?
Pin all the github actions of the repo using commit-sha
Motivation
Having a floating version is not only a stability but also a security risk. Using a commit-sha version fix them and prevent malicious usages
Additional Notes
I bumped some versions so I'll need to fix the related jobs - on-going
Possible Drawbacks / Trade-offs
Describe how to test/QA your changes