feat(dbm): add 'agent integration setup postgres' CLI command - #51751
feat(dbm): add 'agent integration setup postgres' CLI command#51751sangeetashivaji wants to merge 11 commits into
Conversation
Implements RFC 2026-06-02. The command handles the full DB-side setup for Datadog Database Monitoring on Postgres (self-hosted, RDS, Aurora, Cloud SQL): creates the datadog user, grants, schema, helper functions, and configures server settings via ALTER SYSTEM or provider-specific MANUAL_STEP instructions. Follows a Detect → Plan → Apply model with --dry-run, --output json, and idempotent re-runs. Architecture is explicitly designed so MySQL and SQL Server can be added later without restructuring the CLI or output layers. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
All contributors have signed the CLA ✍️ ✅ |
|
🎯 Code Coverage (details) 🔗 Commit SHA: 42d6686 | Docs | Datadog PR Page | Give us feedback! |
Go Package Import DifferencesBaseline: 596b9de
|
Files inventory check summaryFile checks results against ancestor 596b9de6: Results for datadog-agent_7.83.0~devel.git.114.42d6686.pipeline.124630501-1_amd64.deb:No change detected |
Static quality checks✅ Please find below the results from static quality gates Successful checksInfo
17 successful checks with minimal change (< 2 KiB)
|
…p postgres' Renames the CLI entry point from `agent db setup postgres` to `agent integration setup postgres`, relocating the command tree from cmd/agent/subcommands/db/ to cmd/agent/subcommands/integrations/ and the logic layer from pkg/cli/subcommands/db/ to pkg/cli/subcommands/integrations/. Adds postgres_setup.py helper under cmd/agent/dist/setup/. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
sangeetashivaji
changed the title
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 18e7c0ff42
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if err != nil { | ||
| return fmt.Errorf("unable to find embedded Python: %w", err) | ||
| } | ||
| scriptPath = filepath.Join(rootDir, "dist", "setup", "postgres_setup.py") |
There was a problem hiding this comment.
For the normal embedded-Python path, rootDir is the install root found via requirements-agent-release.txt (for example /opt/datadog-agent), while agent dist assets are installed under bin/agent/dist (omnibus copies bin/agent into the install dir and Bazel prefixes these assets with bin/agent/dist). This probes /opt/datadog-agent/dist/setup/postgres_setup.py, so installed agents will fail with "setup script not found" even when the script is packaged; use the same bin/agent/dist layout as the rest of the command or reuse findSetupScript().
Useful? React with 👍 / 👎.
| """ | ||
| DBM Postgres setup — Detect → Plan → Apply using psycopg2 (bundled with the agent). |
There was a problem hiding this comment.
Adding this script under cmd/agent/dist/setup is not enough to make it available in built agents: tasks/agent.py refreshes bin/agent/dist by copying only checks/, utils/, and config.py, and cmd/agent/dist/BUILD.bazel likewise includes only those Python assets. As a result agent integration setup postgres cannot find the script after dda inv agent.build or Bazel packaging unless DD_DBM_SETUP_SCRIPT points back to the source tree; add the setup directory to both asset-copying/package filegroups.
Useful? React with 👍 / 👎.
| instruction = ( | ||
| f"Set {name} = '{desired}'\n" | ||
| f" → RDS Console → Parameter Groups → your group → save → reboot instance" | ||
| ) | ||
| return [{"kind": "MANUAL_STEP", "description": f"[AWS Parameter Group] {name} = '{desired}'", |
There was a problem hiding this comment.
On RDS/Aurora reruns where the parameter group already has the desired value, this still returns a MANUAL_STEP for every setting except Aurora shared_preload_libraries, because there is no current == desired skip like the Cloud SQL/Azure planners have. That makes manual_steps stay true and the CLI exits 2 with "complete the manual steps" even after setup is already correct, breaking the promised idempotent verification flow for managed AWS Postgres.
Useful? React with 👍 / 👎.
sangeetashivaji
added
the
qa/done
…tgres - Auto-format postgres_setup.py with ruff (formatting only, no logic change) - Add release note for the new `agent integration setup postgres` command Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Addresses two P1 review comments: 1. Add cmd/agent/dist/setup/ to tasks/agent.py asset copy (python builds) and to cmd/agent/dist/BUILD.bazel Bazel filegroup so postgres_setup.py is packaged at bin/agent/dist/setup/ in built/installed agents. 2. Replace the hardcoded rootDir+dist path in the embedded-Python branch with findSetupScript(), which already probes the correct bin/agent/dist layout used by both omnibus and Bazel packaging. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
I have read the CLA Document and I hereby sign the CLA |
nathan-b
left a comment
nathan-b
left a comment
There was a problem hiding this comment.
Considering that a possible failure mode is "silently misconfigures production databases", I'd like to see some tests.
| showCmd.Flags().BoolVarP(&cliParams.versionOnly, "show-version-only", "q", false, "only display version information") | ||
| integrationCmd.AddCommand(showCmd) | ||
|
|
||
| integrationCmd.AddCommand(newSetupCommand(globalParams, cliParams)) |
There was a problem hiding this comment.
You should make a version of this command that works without the python build tag, because setup.go is gated on go:build python. Without that tag set, this is a build break.
There was a problem hiding this comment.
The non-python build uses command_nopython.go that returns a nil for this command so there would be no build break.
And a non-python version of the command as of the recent change doesn't make sense since the whole logic now lives in the python integration
| new_value = f"{current},{desired}" if current else desired | ||
| return [ | ||
| { | ||
| "kind": "ALTER_SYSTEM", | ||
| "description": f"ALTER SYSTEM SET shared_preload_libraries = '{new_value}'", | ||
| "sql": f"ALTER SYSTEM SET shared_preload_libraries = '{new_value}'", |
There was a problem hiding this comment.
Do we need to sanitize current to avoid SQL injection? (or, less maliciously, an errant ' in the input breaking the format)
|
|
||
| if result.Outcome == "failure" { | ||
| fmt.Fprintln(os.Stderr, "\nSetup did not complete — check the output above for details.") | ||
| os.Exit(1) |
There was a problem hiding this comment.
Use return and let cobra handle exiting the process (like you do on the previous lines)
| cur.execute("SELECT 1 FROM pg_roles WHERE rolname = 'cloudsqladmin'") | ||
| if cur.fetchone(): | ||
| return "cloud_sql" | ||
| except Exception: |
There was a problem hiding this comment.
What happens if this gets a legitimate exception, maybe a network blip or ACCESS DENIED? The overly-broad except Exception: is going to fallthrough and return self_hosted and then you get a weird error from your ALTER SYSTEM command
| databases := params.databases | ||
| if len(databases) == 0 && !params.allDatabases { | ||
| // Parse the default database from the URI (everything after the last '/'). | ||
| if idx := strings.LastIndex(uri, "/"); idx >= 0 { |
Regression DetectorRegression Detector ResultsMetrics dashboard Baseline: 596b9de Optimization Goals: ✅ No significant changes detected
|
| perf | experiment | goal | Δ mean % | Δ mean % CI | trials | links |
|---|---|---|---|---|---|---|
| ➖ | quality_gate_logs | % cpu utilization | +0.91 | [-0.07, +1.90] | 1 | Logs bounds checks dashboard |
| ➖ | quality_gate_idle | memory utilization | +0.53 | [+0.48, +0.58] | 1 | Logs bounds checks dashboard |
| ➖ | quality_gate_idle_all_features | memory utilization | +0.39 | [+0.32, +0.46] | 1 | Logs bounds checks dashboard |
| ➖ | quality_gate_security_no_fs_load | memory utilization | +0.33 | [+0.23, +0.42] | 1 | Logs bounds checks dashboard |
| ➖ | quality_gate_security_idle | memory utilization | +0.14 | [+0.08, +0.20] | 1 | Logs bounds checks dashboard |
| ➖ | quality_gate_metrics_logs | memory utilization | +0.10 | [-0.15, +0.35] | 1 | Logs bounds checks dashboard |
| ➖ | quality_gate_security_mean_fs_load | memory utilization | -0.03 | [-0.06, +0.01] | 1 | Logs bounds checks dashboard |
Bounds Checks: ✅ Passed
| perf | experiment | bounds_check_name | replicates_passed | observed_value | links |
|---|---|---|---|---|---|
| ✅ | quality_gate_idle | intake_connections | 10/10 | 3 ≤ 4 | bounds checks dashboard |
| ✅ | quality_gate_idle | memory_usage | 10/10 | 147.43MiB ≤ 154MiB | bounds checks dashboard |
| ✅ | quality_gate_idle | total_bytes_received | 10/10 | 734.00KiB ≤ 819.20KiB | bounds checks dashboard |
| ✅ | quality_gate_idle_all_features | intake_connections | 10/10 | 3 ≤ 4 | bounds checks dashboard |
| ✅ | quality_gate_idle_all_features | memory_usage | 10/10 | 492.68MiB ≤ 495MiB | bounds checks dashboard |
| ✅ | quality_gate_idle_all_features | total_bytes_received | 10/10 | 1.12MiB ≤ 1.25MiB | bounds checks dashboard |
| ✅ | quality_gate_logs | intake_connections | 10/10 | 4 ≤ 6 | bounds checks dashboard |
| ✅ | quality_gate_logs | memory_usage | 10/10 | 187.49MiB ≤ 195MiB | bounds checks dashboard |
| ✅ | quality_gate_logs | missed_bytes | 10/10 | 0B = 0B | bounds checks dashboard |
| ✅ | quality_gate_logs | total_bytes_received | 10/10 | 264.17MiB ≤ 292MiB | bounds checks dashboard |
| ✅ | quality_gate_metrics_logs | cpu_usage | 10/10 | 348.49 ≤ 2000 | bounds checks dashboard |
| ✅ | quality_gate_metrics_logs | intake_connections | 10/10 | 3 ≤ 6 | bounds checks dashboard |
| ✅ | quality_gate_metrics_logs | memory_usage | 10/10 | 399.52MiB ≤ 430MiB | 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.93GiB ≤ 1.04GiB | bounds checks dashboard |
| ✅ | quality_gate_security_idle | cpu_usage | 10/10 | 31.54 ≤ 40 | bounds checks dashboard |
| ✅ | quality_gate_security_idle | memory_usage | 10/10 | 299.13MiB ≤ 330MiB | bounds checks dashboard |
| ✅ | quality_gate_security_mean_fs_load | cpu_usage | 10/10 | 65.73 ≤ 80 | bounds checks dashboard |
| ✅ | quality_gate_security_mean_fs_load | memory_usage | 10/10 | 274.91MiB ≤ 310MiB | bounds checks dashboard |
| ✅ | quality_gate_security_no_fs_load | cpu_usage | 10/10 | 22.34 ≤ 40 | bounds checks dashboard |
| ✅ | quality_gate_security_no_fs_load | memory_usage | 10/10 | 286.29MiB ≤ 320MiB | 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".
Replicate Execution Details
We run multiple replicates for each experiment/variant. However, we allow replicates to be automatically retried if there are any failures, up to 8 times, at which point the replicate is marked dead and we are unable to run analysis for the entire experiment. We call each of these attempts at running replicates a replicate execution. This section lists all replicate executions that failed due to the target crashing or being oom killed.
Note: In the below tables we bucket failures by experiment, variant, and failure type. For each of these buckets we list out the replicate indexes that failed with an annotation signifying how many times said replicate failed with the given failure mode. In the below example the baseline variant of the experiment named experiment_with_failures had two replicates that failed by oom kills. Replicate 0, which failed 8 executions, and replicate 1 which failed 6 executions, all with the same failure mode.
| Experiment | Variant | Replicates | Failure | Logs | Debug Dashboard |
|---|---|---|---|---|---|
| experiment_with_failures | baseline | 0 (x8) 1 (x6) | Oom killed | Debug Dashboard |
The debug dashboard links will take you to a debugging dashboard specifically designed to investigate replicate execution failures.
❌ Retried Profiling Replicate Execution Failures (ddprof)
Note: Profiling replicas may still be executing. See the debug dashboard for up to date status.
| Experiment | Variant | Replicates | Failure | Debug Dashboard |
|---|---|---|---|---|
| quality_gate_idle_all_features | baseline | 10 | Oom killed | Debug Dashboard |
| quality_gate_idle_all_features | comparison | 10 | Oom killed | Debug Dashboard |
| quality_gate_logs | baseline | 10 | Oom killed | Debug Dashboard |
| quality_gate_logs | comparison | 10 | Oom killed | Debug Dashboard |
| quality_gate_metrics_logs | baseline | 10 | Oom killed | Debug Dashboard |
| quality_gate_metrics_logs | comparison | 10 | Oom killed | Debug Dashboard |
| quality_gate_security_idle | baseline | 10 | Crashed (exit code: 134) | Debug Dashboard |
CI Pass/Fail Decision
✅ Passed. All Quality Gates 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_idle, bounds check intake_connections: 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_security_idle, bounds check memory_usage: 10/10 replicas passed. Gate passed.
- quality_gate_security_idle, bounds check cpu_usage: 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 intake_connections: 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 total_bytes_received: 10/10 replicas passed. Gate passed.
- quality_gate_metrics_logs, bounds check cpu_usage: 10/10 replicas passed. Gate 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_logs, bounds check missed_bytes: 10/10 replicas passed. Gate passed.
- quality_gate_logs, bounds check memory_usage: 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.
…oring it Replaces the mirrored _configure_connection logic in postgres_setup.py with a direct import of datadog_checks.postgres.connection_pool.configure_connection, which was extracted as a standalone function in a companion integrations-core change. The setup script now genuinely reuses the integration's connection layer (autocommit, SQL_ASCII text decoding, CommenterCursor) rather than maintaining a copy. The fallback for dev / --use-sys-python without the integration installed drops to a minimal psycopg.ClientCursor config. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bc5c2ea77c
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| instruction = ( | ||
| f"Set {name} = '{desired}'\n → RDS Console → Parameter Groups → your group → save → reboot instance" | ||
| ) |
There was a problem hiding this comment.
On RDS/Aurora reruns after the user has already applied the parameter-group changes, current can already equal desired, but this path still emits a MANUAL_STEP unconditionally. Because _build_result treats any manual step as manual_steps, the command continues to exit 2 and tells users to make manual changes forever instead of verifying success.
Useful? React with 👍 / 👎.
| if idx := strings.LastIndex(uri, "/"); idx >= 0 { | ||
| db := uri[idx+1:] | ||
| if q := strings.Index(db, "?"); q >= 0 { | ||
| db = db[:q] |
There was a problem hiding this comment.
When --databases is omitted, this uses the last slash anywhere in the connection URI, so valid URIs with slash-containing query parameters such as ?sslrootcert=/etc/ssl/ca.pem or Unix-socket ?host=/var/run/postgresql derive ca.pem/postgresql as the database name. The Python runner then overrides dbname with that bogus value and the setup fails or targets the wrong database; parse the URI path (or ask Postgres for current_database()) instead.
Useful? React with 👍 / 👎.
| srcs = glob( | ||
| ["**"], |
There was a problem hiding this comment.
The Bazel review guideline for this repo forbids recursive glob(["**"]) patterns because they silently skip files under future subpackages and hurt incremental/remote-cache behavior. This package currently only ships postgres_setup.py, so listing that file explicitly keeps the package hermetic and avoids the recursive-glob trap.
Useful? React with 👍 / 👎.
Move the DBM Postgres setup logic into integrations-core (datadog_checks.postgres.setup) and run it via `python -m` so the agent command is only the tunnel: it parses flags, prompts, renders output, and owns exit codes. Drops the shipped cmd/agent/dist/setup script and its asset wiring, so setup ships with the integration wheel instead of agent dist assets. Parse the default database from the connection URI path with url.Parse so slash-containing query parameters (sslrootcert, host) are not mistaken for the database name, and return an error on failure instead of calling os.Exit. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Covers defaultDatabaseFromURI's query-param edge cases and callPython's JSON success/failure/non-JSON parsing paths, addressing the test-coverage request from PR review 4480467506. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Run gazelle to add the missing setup.go/setup_test.go entries to cmd/agent/subcommands/integrations/BUILD.bazel and generate the new pkg/cli/subcommands/integrations/setup/BUILD.bazel. Also fix golangci-lint findings: gofmt alignment in output.go and two fmt.Errorf calls replaced with errors.New (perfsprint) in setup.go. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
This pull request has been automatically marked as stale because it has not had activity in the past 15 days. It will be closed in 30 days if no further activity occurs. If this pull request is still relevant, adding a comment or pushing new commits will keep it open. Also, you can always reopen the pull request if you missed the window. Thank you for your contributions! |
3 participants
Summary
datadog-agent integration setup postgres <connection-uri>which idempotently handles the full DB-side setup for Datadog Database Monitoring on Postgres (self-hosted, RDS, Aurora, Cloud SQL, Azure)Architecture (updated after review)
Per review feedback, the setup logic now lives in the Postgres integration
and the agent is only the tunnel:
datadog_checks.postgres.setupin integrations-core(Add DBM Postgres setup module and extract configure_connection integrations-core#24052). Reuses the integration's connection layer
(
configure_connection), database navigation (AUTODISCOVERY_QUERY), andparse_shared_preload_libraries. The canonicaldatadog.*helper-function SQLlives there next to the test compose resources (single source of truth).
output, and owns exit codes, then runs
python -m datadog_checks.postgres.setupvia the embedded interpreter (JSON over stdin/stdout). This mirrors the existing
integration install/showsubprocess pattern.cmd/agent/subcommands/integrations/setup.gopkg/cli/subcommands/integrations/setup/Review items addressed
cmd/agent/dist/setup/and reverted the
tasks/agent.py/cmd/agent/dist/BUILD.bazelwiring, so thetwo "script not packaged / not found" findings no longer apply — setup ships
with the integration wheel.
integrations-core/.../03_setup.shisresolved: the canonical SQL now lives only in the integration.
manual step once the value already matches.
url.Parse,so slash-containing query parameters (
sslrootcert,host) are not mistaken forthe database name.
os.Exiton failure replaced with a returned error (cobra handles exit).go:build python: theintegrationcommand tree is already gated — under!python,command_nopython.goreturnsnil, so there is no build break.Test plan
datadog-agent integration setup postgres --help--dry-runagainst a localpostgres:15container — verify plan output--update-password— all rows[SKIP], exits zero--output json--all-databases🤖 Generated with Claude Code