Skip to content

Conversation

@0xgouda
Copy link
Collaborator

@0xgouda 0xgouda commented Nov 13, 2025

  • Modified IsDirectlyFetchableMetric() to auto-detect same-host via md.IsClientOnSameHost().
  • Removed r.Metrics.DirectOSStats flag checks.
  • Added SourceConn.IsClientOnSameHost() helper method.
  • Marked --direct-os-stats field as hidden and log a deprecation warning if command option is specified.
  • Updated docs to remove --direct-os-stats and explain automatic detection behaviour.
  • Added a Unit Test
  • Added an Integration Test for non-direct OS access of psutil metrics.

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

@0xgouda 0xgouda linked an issue Nov 13, 2025 that may be closed by this pull request
9 tasks
@coveralls
Copy link

coveralls commented Nov 13, 2025

Pull Request Test Coverage Report for Build 19362920012

Details

  • 4 of 14 (28.57%) changed or added relevant lines in 5 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.07%) to 73.688%

Changes Missing Coverage Covered Lines Changed/Added Lines %
cmd/pgwatch/main.go 1 3 33.33%
internal/reaper/reaper.go 0 4 0.0%
internal/sources/conn.go 0 4 0.0%
Totals Coverage Status
Change from base Build 19360779365: -0.07%
Covered Lines: 3680
Relevant Lines: 4994

💛 - Coveralls

@pashagolub pashagolub requested a review from Copilot November 14, 2025 09:56
@pashagolub pashagolub self-assigned this Nov 14, 2025
@pashagolub pashagolub added refactoring Something done as it should've been done from the start config Configuration store related sources What sources and in what way to monitor labels Nov 14, 2025
Copilot finished reviewing on behalf of pashagolub November 14, 2025 10:00
Copy link

Copilot AI left a 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-stats flag 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

pashagolub and others added 9 commits November 14, 2025 13:16
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>
@0xgouda 0xgouda force-pushed the deprecate-direct-os-stats branch from 8f87542 to a02bd8a Compare November 14, 2025 11:16
@0xgouda 0xgouda marked this pull request as ready for review November 14, 2025 11:18
@0xgouda 0xgouda requested a review from pashagolub November 14, 2025 11:19
@pashagolub pashagolub merged commit 5ed6968 into master Nov 17, 2025
9 checks passed
@pashagolub pashagolub deleted the deprecate-direct-os-stats branch November 17, 2025 16:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

config Configuration store related refactoring Something done as it should've been done from the start sources What sources and in what way to monitor

Projects

None yet

Development

Successfully merging this pull request may close these issues.

deprecate --direct-os-stats flag

4 participants