Repository navigation
Conversation
github-actions
Bot
added
long review
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
AI review by Codex (OpenAI) - workflow run
The patch is correct based on static review. I found no actionable regressions in the migration, build wiring, or authorization-preview changes examined. Tests could not run because the read-only environment lacks a Go module cache.
Contributor
Go Package Import DifferencesBaseline: b911073
|
Contributor
BenchmarksBenchmark execution time: 2026-10-30 13:51:42 Comparing candidate commit f6de142 in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 3 metrics, 0 unstable metrics.
|
Contributor
Contributor
Static quality checks✅ Please find below the results from static quality gates Successful checksInfo
17 successful checks with minimal change (< 2 KiB)
|
Regression DetectorRegression Detector ResultsMetrics dashboard Baseline: b911073 Optimization Goals: ✅ No significant changes detected
|
| perf | experiment | goal | Δ mean % | Δ mean % CI | trials | links |
|---|---|---|---|---|---|---|
| ➖ | dsd_uds_10mb_3k_timestamped_contexts_cpu | % cpu utilization | +1.12 | [+0.85, +1.39] | 1 | Logs |
| ➖ | dsd_uds_client_drop_detector_cpu | % cpu utilization | +1.07 | [+0.53, +1.62] | 1 | Logs |
| ➖ | quality_gate_idle | memory utilization | +0.23 | [+0.19, +0.27] | 1 | Logs bounds checks dashboard |
| ➖ | quality_gate_security_idle | memory utilization | +0.22 | [+0.19, +0.26] | 1 | Logs bounds checks dashboard |
| ➖ | quality_gate_security_no_fs_load | memory utilization | +0.19 | [+0.13, +0.24] | 1 | Logs bounds checks dashboard |
| ➖ | quality_gate_metrics_logs | memory utilization | +0.12 | [-0.11, +0.35] | 1 | Logs bounds checks dashboard |
| ➖ | python_openmetrics | % cpu utilization | +0.08 | [-0.59, +0.74] | 1 | Logs bounds checks dashboard |
| ➖ | quality_gate_idle_all_features | memory utilization | -0.04 | [-0.11, +0.04] | 1 | Logs bounds checks dashboard |
| ➖ | quality_gate_security_mean_fs_load | memory utilization | -0.07 | [-0.11, -0.03] | 1 | Logs bounds checks dashboard |
| ➖ | quality_gate_private_action_runner | memory utilization | -0.19 | [-0.31, -0.06] | 1 | Logs bounds checks dashboard |
| ➖ | dsd_uds_10mb_3k_timestamped_contexts_memory | memory utilization | -0.44 | [-0.65, -0.23] | 1 | Logs |
| ➖ | quality_gate_logs | % cpu utilization | -1.84 | [-2.76, -0.92] | 1 | Logs bounds checks dashboard |
Bounds Checks: ✅ Passed
| perf | experiment | bounds_check_name | replicates_passed | observed_value | links |
|---|---|---|---|---|---|
| ✅ | python_openmetrics | checks_execution_time | 10/10 | 70.78 ≤ 100 | bounds checks dashboard |
| ✅ | python_openmetrics | cpu_usage | 10/10 | 1352.89 ≤ 1500 | bounds checks dashboard |
| ✅ | python_openmetrics | memory_usage | 10/10 | 4.32GiB ≤ 4.75GiB | bounds checks dashboard |
| ✅ | quality_gate_idle | intake_connections | 10/10 | 4 ≤ 5 | bounds checks dashboard |
| ✅ | quality_gate_idle | memory_usage | 10/10 | 179.25MiB ≤ 181MiB | bounds checks dashboard |
| ✅ | quality_gate_idle | total_bytes_received | 10/10 | 774.58KiB ≤ 819.20KiB | bounds checks dashboard |
| ✅ | quality_gate_idle_all_features | intake_connections | 10/10 | 2 ≤ 5 | bounds checks dashboard |
| ✅ | quality_gate_idle_all_features | memory_usage | 10/10 | 477.65MiB ≤ 542MiB | bounds checks dashboard |
| ✅ | quality_gate_idle_all_features | total_bytes_received | 10/10 | 1.18MiB ≤ 1.25MiB | bounds checks dashboard |
| ✅ | quality_gate_logs | intake_connections | 10/10 | 16 ≤ 40 | bounds checks dashboard |
| ✅ | quality_gate_logs | memory_usage | 10/10 | 221.54MiB ≤ 228MiB | bounds checks dashboard |
| ✅ | quality_gate_logs | missed_bytes | 10/10 | 0B = 0B | bounds checks dashboard |
| ✅ | quality_gate_logs | total_bytes_received | 10/10 | 264.36MiB ≤ 292MiB | bounds checks dashboard |
| ✅ | quality_gate_metrics_logs | cpu_usage | 10/10 | 374.63 ≤ 2000 | bounds checks dashboard |
| ✅ | quality_gate_metrics_logs | intake_connections | 10/10 | 19 ≤ 40 | bounds checks dashboard |
| ✅ | quality_gate_metrics_logs | memory_usage | 10/10 | 416.30MiB ≤ 455MiB | bounds checks dashboard |
| ✅ | quality_gate_metrics_logs | missed_bytes | 10/10 | 0B = 0B | bounds checks dashboard |
| ✅ | quality_gate_metrics_logs | total_bytes_received | 10/10 | 0.95GiB ≤ 1.04GiB | bounds checks dashboard |
| ✅ | quality_gate_private_action_runner | memory_usage | 10/10 | 74.80MiB ≤ 77MiB | bounds checks dashboard |
| ✅ | quality_gate_security_idle | cpu_usage | 10/10 | 28.83 ≤ 100 | bounds checks dashboard |
| ✅ | quality_gate_security_idle | memory_usage | 10/10 | 330.00MiB ≤ 357MiB | bounds checks dashboard |
| ✅ | quality_gate_security_mean_fs_load | cpu_usage | 10/10 | 42.66 ≤ 200 | bounds checks dashboard |
| ✅ | quality_gate_security_mean_fs_load | memory_usage | 10/10 | 317.03MiB ≤ 337MiB | bounds checks dashboard |
| ✅ | quality_gate_security_no_fs_load | cpu_usage | 10/10 | 23.52 ≤ 100 | bounds checks dashboard |
| ✅ | quality_gate_security_no_fs_load | memory_usage | 10/10 | 318.56MiB ≤ 348MiB | bounds checks dashboard |
Explanation
Confidence level: 90.00%
Effect size tolerance: |Δ mean %| ≥ 5.00%
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
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".
CI Pass/Fail Decision
✅ Passed. All Quality Gates passed.
- quality_gate_security_mean_fs_load, bounds check memory_usage: 10/10 replicas passed. Gate passed.
- quality_gate_security_mean_fs_load, bounds check cpu_usage: 10/10 replicas passed. Gate passed.
- quality_gate_security_no_fs_load, bounds check memory_usage: 10/10 replicas passed. Gate passed.
- quality_gate_security_no_fs_load, bounds check cpu_usage: 10/10 replicas passed. Gate passed.
- quality_gate_idle, bounds check memory_usage: 10/10 replicas passed. Gate passed.
- quality_gate_idle, bounds check total_bytes_received: 10/10 replicas passed. Gate passed.
- quality_gate_idle, bounds check intake_connections: 10/10 replicas passed. Gate passed.
- quality_gate_idle_all_features, bounds check intake_connections: 10/10 replicas passed. Gate passed.
- quality_gate_idle_all_features, bounds check total_bytes_received: 10/10 replicas passed. Gate passed.
- quality_gate_idle_all_features, bounds check memory_usage: 10/10 replicas passed. Gate passed.
- quality_gate_metrics_logs, bounds check total_bytes_received: 10/10 replicas passed. Gate passed.
- quality_gate_metrics_logs, bounds check memory_usage: 10/10 replicas passed. Gate passed.
- quality_gate_metrics_logs, bounds check cpu_usage: 10/10 replicas passed. Gate passed.
- quality_gate_metrics_logs, bounds check missed_bytes: 10/10 replicas passed. Gate passed.
- quality_gate_metrics_logs, bounds check intake_connections: 10/10 replicas passed. Gate passed.
- quality_gate_logs, bounds check memory_usage: 10/10 replicas passed. Gate passed.
- quality_gate_logs, bounds check missed_bytes: 10/10 replicas passed. Gate passed.
- quality_gate_logs, bounds check intake_connections: 10/10 replicas passed. Gate passed.
- quality_gate_logs, bounds check total_bytes_received: 10/10 replicas passed. Gate passed.
- quality_gate_security_idle, bounds check cpu_usage: 10/10 replicas passed. Gate passed.
- quality_gate_security_idle, bounds check memory_usage: 10/10 replicas passed. Gate passed.
- quality_gate_private_action_runner, bounds check memory_usage: 10/10 replicas passed. Gate passed.
dd-octo-sts
Bot
added
team/agent-devx
team/windows-products
team/ebpf-platform
team/agent-apm
rdesgroppes
reviewed
Oct 28, 2026
rdesgroppes
left a comment
rdesgroppes
left a comment
Collaborator
There was a problem hiding this comment.
First, thanks much for joining our monorepo, that's pretty encouraging to all of us!
I posted a first comment and my team, @DataDog/agent-build, will have a further look tomorrow.
For GitHub workflows, I guess @DataDog/agent-devx is probably your best contact.
rdesgroppes
reviewed
Oct 28, 2026
rdesgroppes
reviewed
Oct 28, 2026
…s license CSV Review feedback: - The fuzz jobs now extend .bazel:defs:cache:progressive (through .rshell:base) instead of defining their own cache. A generated child pipeline can't see the parent's templates, so rshell:fuzz becomes a parallel matrix job in .gitlab/test/rshell.yml. rshell.verify-fuzz-matrix, run by rshell:checks, fails when the matrix and the packages with fuzz targets drift apart. This replaces rshell.generate-fuzz-pipeline and .gitlab/rshell/fuzz.yml. - pkg/rshell/LICENSE-3rdparty.csv is dropped: the root LICENSE-3rdparty.csv, generated from the Agent's dependencies and checked by lint_licenses, already covers what rshell ships, and the copy listed the Agent's own modules. The compliance tests that checked the copy are removed. Environment: Datadog workspace Co-Authored-By: Claude Code Opus 5.5 <noreply@anthropic.com>
Hitsuji-M
approved these changes
Oct 29, 2026
Contributor
|
Looks good for |
JSGette
approved these changes
Oct 29, 2026
JSGette
reviewed
Oct 29, 2026
JSGette
reviewed
Oct 29, 2026
louis-cqrl
approved these changes
Oct 29, 2026
- dda inv fuzz gains --jobs to fuzz several targets at once (one worker each), so rshell:fuzz is one job running 8 targets at a time instead of a 33-job matrix; drop rshell.verify-fuzz-matrix. - Tolerate only Go's bare coordinator timeout under "--- FAIL: <func>", not t.Fatal output or panics that mention a deadline. - Drop the needless dda self dep sync before_script. - Pull the Bash comparison image from registry.ddbuild.io's mirror in GitLab (RSHELL_BASH_IMAGE) instead of Docker Hub, and fail rather than skip when the pull fails. - Run the landlock/seccomp tests cgo-free as Bazel targets (pure = "on", like the shipped rshell binary) so the default Bazel jobs cover them, and drop the explicit pure-mode steps from GitLab and GitHub Actions. Environment: Datadog workspace Co-Authored-By: Claude Code Sonnet 5.5 <noreply@anthropic.com>
…ll-migration Keep pkg/rshell over main's bump of github.com/DataDog/rshell to v0.0.31 (pkg/rshell is synced past it, at 6871bdf6). Environment: Datadog workspace Co-Authored-By: Claude Code Sonnet 5.5 <noreply@anthropic.com>
julesmcrt
requested changes
Oct 29, 2026
julesmcrt
left a comment
julesmcrt
left a comment
Contributor
There was a problem hiding this comment.
This will go in 7.87 ; not 7.86
Waiting for the cutoff
main added pkg/config/setup/generatedsettings_local and this branch adds pkg/rshell; each go.mod needs a replace directive pointing at the other. Environment: Datadog workspace Co-Authored-By: Claude Code Opus 5.5 <noreply@anthropic.com>
Contributor
Author
|
🤖 Reply from Matthew's coding agent (Claude Code), not Matthew himself. Re the Codex review of |
Sync DataDog/rshell@f323a145 (DataDog/rshell#696, VULN-4562, VULN-1673, VULN-4535) into pkg/rshell: MaxNestingDepth (5000) enforced by a pre-parse lexical guard in ParseScript and an iterative AST depth check in validateNode, so a deeply nested sub-5-MiB script fails with an error instead of crashing the embedding process with a Go stack overflow. pkg/rshell now matches rshell main (f323a145); #705 was already present and #706 only changes upstream's own fuzz CI. Environment: Datadog workspace Co-Authored-By: Claude Code Opus 5.5 <noreply@anthropic.com>
main bumped x/net to v0.62.0 in every module (#59052); pkg/rshell is new on this branch, so the merge left it at v0.61.0 and bazel:run-go-mod-tidy failed on it. Environment: Datadog workspace Co-Authored-By: Claude Code Opus 5.5 <noreply@anthropic.com>
GitLab's rshell:checks, rshell:bash and rshell:fuzz already run every check in .github/workflows/rshell.yml (source-file coverage, Bash comparison, compliance, fuzzing), and pull Debian from our registry mirror instead of Docker Hub. Keep GitLab as the single place they run. Environment: Datadog workspace Co-Authored-By: Claude Code Opus 5.5 <noreply@anthropic.com>
…verage The cgo-free sandbox tests re-exec themselves as restricted helpers. Under bazel coverage the helpers then failed after passing, trying to write coverage data: - the landlock helpers restrict themselves to the test paths, so they now exit directly once their checks pass; - the seccomp elevation helper drops privileges, so it no longer inherits the runner's environment and its root-owned coverage paths (as DataDog/rshell#705 did for head). Environment: Datadog workspace Co-Authored-By: Claude Code Opus 5.5 <noreply@anthropic.com>
irozzo-1A
reviewed
Oct 30, 2026
irozzo-1A
left a comment
irozzo-1A
left a comment
Contributor
There was a problem hiding this comment.
I don't see problems in agent-runtimes scope, just a nit
…endency Require pkg/rshell at v0.0.0-00010101000000-000000000000 in the root and test/new-e2e go.mod files, matching the convention for other in-repo modules resolved through replace directives. Environment: Datadog workspace Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
irozzo-1A
approved these changes
Oct 30, 2026
shimupan
approved these changes
Oct 30, 2026
shimupan
left a comment
shimupan
left a comment
Contributor
There was a problem hiding this comment.
looks good for action platform side
julesmcrt
approved these changes
Oct 31, 2026
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Moves the restricted shell (rshell) into the Agent repository as the in-tree module
github.com/DataDog/datadog-agent/pkg/rshell. Until now it was the external dependencygithub.com/DataDog/rshell.pkg/rshellis synced withDataDog/rshell@6871bdf6(rshellmainon 2026-10-28, through fix(interp): handle brace-generated invalid glob patterns without panicking rshell#704), with the import path rewritten. It is registered inmodules.yml, and every Agent module now resolves it through areplaceto./pkg/rshell.pkg/rshell/privilegedhelper(Bazel target//pkg/rshell/privilegedhelper) instead ofgithub.com/DataDog/rshell. The Renovate rule forDataDog/rshellis removed.BUILD.bazelfiles..gitlab/test/rshell.yml):rshell:checks(source-file verification, compliance, landlock/seccomp tests),rshell:bash(tests against bash on thedocker_x64image), andrshell:fuzz, aparallel: matrixjob with one entry per fuzz package. It uses the shared.bazel:defs:cache:progressivecache, andrshell:checksrunsdda inv rshell.verify-fuzz-matrixto keep the matrix in sync with the fuzz targets..github/workflows/rshell.yml(tests and per-package fuzz matrix) andrshell-analysis.yml.@DataDog/dd-agent-mcpand@DataDog/fleet-remediation.dd-agent-mcpis mapped to the FLREM Jira project.dda inv fuzztakes a--targetspackage and runs each fuzz target in it. If Go's fuzzer fails a run with a barecontext deadline exceededwhen-fuzztimeruns out, and writes no failing input, the run counts as a pass. Any run that printsFailing input written to …still fails.main(all migration-specific):errors.Newfor constant errors, unused parameters, ignored errors, naming), plus the allowlist entries they need.headpermission test starts its child with onlyPATHandTMPDIR, so under Bazel coverage as root it doesn't write into root-owned outputs.createListenertests skip when/is owned by neither root nor the caller, which happens in an unprivileged Bazel sandbox. CI runs Bazel as root and still runs them.ddareplace thegocommands in theMakefileand docs..github,.claude,hooks,.go-version,repository.datadog.yml).LICENSE-3rdparty.csvand the compliance tests that checked it are dropped; the Agent's rootLICENSE-3rdparty.csvcovers its dependencies.Motivation
Building rshell in the Agent repository removes the release and version-bump cycle between
DataDog/rshelland the Agent. Changes to the shell and to its Agent consumers can land and be tested in one PR.Describe how you validated your changes
lint_*jobs andbazel:run-gazellepass (latest run: 95/95 lint, gazelle and fuzz checks green).bazel run //:gazellemakes no further changes,dda inv linter.go --targets=./pkg/rshell/...reports 0 issues, anddda inv linter.pythonpasses.testing/fuzz.goprintsFailing input written toonly for a real crash, and a coordinator timeout prints only the error.Additional Notes
Fuzz timeouts. Upstream rshell CI already treats Go's
-fuzztimeboundary timeout as a pass. Moving the fuzz jobs ontodda inv fuzzlost that, so a random target failed on each run (ls,lsof,jq,ntfsmft) without any crashing input. The tolerance above restores upstream's behaviour, with a stricter check: upstream only looks for a_test.go:N:line, which would also hide panics in non-test code.