-
Notifications
You must be signed in to change notification settings - Fork 64
[*] deprecate --direct-os-stats flag
#1024
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
Conversation
Pull Request Test Coverage Report for Build 19362920012Details
💛 - Coveralls |
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.
Pull Request Overview
This PR deprecates the --direct-os-stats command-line flag in favor of automatic detection of same-host scenarios. When pgwatch and PostgreSQL run on the same host, the system now automatically attempts to fetch OS-level metrics (like psutil* and cpu_load) directly from the OS rather than through PL/Python wrappers.
Key changes:
- Automatic same-host detection via unix socket check and system identifier comparison
- Removal of manual
--direct-os-statsflag requirement with deprecation warning - Updated documentation to reflect automatic behavior
Reviewed Changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/sources/conn.go | Added IsClientOnSameHost() helper method to check if client and server are on same host |
| internal/reaper/reaper.go | Modified to call IsDirectlyFetchableMetric() with automatic detection instead of checking DirectOSStats flag; moved timer initialization before metric fetch |
| internal/reaper/metric_test.go | Updated unit test to mock same-host detection query and pass SourceConn to IsDirectlyFetchableMetric() |
| internal/reaper/file.go | Modified IsDirectlyFetchableMetric() to accept SourceConn parameter and check same-host status automatically |
| internal/metrics/cmdopts.go | Marked DirectOSStats field as hidden to deprecate the flag |
| internal/db/conn.go | Updated comments in IsClientOnSameHost() to clarify the two-option detection approach |
| docs/tutorial/preparing_databases.md | Updated documentation to explain automatic same-host detection behavior instead of manual flag |
| docs/reference/cli_env.md | Removed --direct-os-stats flag documentation |
| cmd/pgwatch/main_integration_test.go | Added integration test to verify non-direct OS stats behavior when not on same host |
| cmd/pgwatch/main.go | Added deprecation warning when --direct-os-stats flag is used |
| cmd/pgwatch/doc.go | Removed --direct-os-stats from auto-generated documentation |
automatically when pgwatch runs on the same host as PostgreSQL. - `IsDirectlyFetchableMetric()` now auto-detects same-host - Removed `r.Metrics.DirectOSStats` flag checks from reaper - Added `SourceConn.IsClientOnSameHost()` helper method
- Mention that pgwatch now auto-detects this.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
8f87542 to
a02bd8a
Compare
IsDirectlyFetchableMetric()to auto-detect same-host viamd.IsClientOnSameHost().r.Metrics.DirectOSStatsflag checks.SourceConn.IsClientOnSameHost()helper method.--direct-os-statsfield as hidden and log a deprecation warning if command option is specified.--direct-os-statsand explain automatic detection behaviour.For now, I am having problems making the integration test detect that the pg container and the pgwatch instance are on the same host.
Closes: #1022