feat: remove pool.acquire and prepare spans, preserve metrics#76
Open
DavidS-ovm wants to merge 1 commit into
Open
feat: remove pool.acquire and prepare spans, preserve metrics#76DavidS-ovm wants to merge 1 commit into
DavidS-ovm wants to merge 1 commit into
Conversation
3 tasks
e00d7db to
1521356
Compare
dylanratcliffe
pushed a commit
to overmindtech/cli
that referenced
this pull request
May 14, 2026
## Summary - Switch `github.com/overmindtech/otelpgx` import back to `github.com/exaring/otelpgx` and add a `go.mod` `replace` directive pointing at our fork's new branch (`remove-acquire-prepare-spans-upstream`). - Same span-removal patch we've been running in production since March, just consumed via `replace` instead of relying on the fork's module-rename commit. - Once upstream [exaring/otelpgx#76](exaring/otelpgx#76) merges and a release is tagged, we drop the `replace` and bump the require — no other workspace code changes. ## Linear Ticket Fixes: [ENG-4119](https://linear.app/overmind/issue/ENG-4119/consume-otelpgx-fork-via-gomod-replace-instead-of-import-path-rename) — Consume otelpgx fork via go.mod replace instead of import-path rename - **Purpose**: Decouple workspace consumption from the rename commit so the same span patch can land cleanly upstream and we can drop the fork the moment a tagged release is available. - **No reviewer requested**: Pure plumbing cleanup. Behaviour is unchanged from what's already in production via [#4103](https://github.com/overmindtech/workspace/pull/4103); the `replace` directive will be removed entirely once upstream releases. ## Changes | File | Change | | --- | --- | | `go/dbkit/connect.go` | Import path back to `github.com/exaring/otelpgx` | | `go.mod` | `require github.com/exaring/otelpgx v0.10.0` + new `replace` directive (with explanatory comment) pointing at `github.com/overmindtech/otelpgx v0.0.0-20260504155636-e00d7db88073` | | `go.sum` | `go mod tidy` refresh | 3 files, 8 insertions, 4 deletions. Verified locally with `go build ./...` and `go test ./go/dbkit/...`. Made with [Cursor](https://cursor.com) <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Low Risk** > Low risk plumbing change: switches `otelpgx` consumption back to the upstream module path while pinning it to an internal fork via `replace`, so runtime behavior should remain the same aside from potential dependency-resolution issues. > > **Overview** > Updates the `otelpgx` dependency wiring to use the upstream import path (`github.com/exaring/otelpgx`) while temporarily sourcing it from an Overmind fork via a new `go.mod` `replace` (to keep the span-removal patch applied). > > Refreshes `go.sum` accordingly and adjusts the only in-repo usage (`go/dbkit/connect.go`) to import `exaring/otelpgx`. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 63daef169d0cd8cd776991b66203b70cdb5dd611. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> Co-authored-by: Cursor <cursoragent@cursor.com> GitOrigin-RevId: 96bc81f19695b4e95caa418b32bc92db3ba17912
carabasdaniel
pushed a commit
to overmindtech/cli
that referenced
this pull request
May 14, 2026
## Summary - Switch `github.com/overmindtech/otelpgx` import back to `github.com/exaring/otelpgx` and add a `go.mod` `replace` directive pointing at our fork's new branch (`remove-acquire-prepare-spans-upstream`). - Same span-removal patch we've been running in production since March, just consumed via `replace` instead of relying on the fork's module-rename commit. - Once upstream [exaring/otelpgx#76](exaring/otelpgx#76) merges and a release is tagged, we drop the `replace` and bump the require — no other workspace code changes. ## Linear Ticket Fixes: [ENG-4119](https://linear.app/overmind/issue/ENG-4119/consume-otelpgx-fork-via-gomod-replace-instead-of-import-path-rename) — Consume otelpgx fork via go.mod replace instead of import-path rename - **Purpose**: Decouple workspace consumption from the rename commit so the same span patch can land cleanly upstream and we can drop the fork the moment a tagged release is available. - **No reviewer requested**: Pure plumbing cleanup. Behaviour is unchanged from what's already in production via [#4103](overmindtech/workspace#4103); the `replace` directive will be removed entirely once upstream releases. ## Changes | File | Change | | --- | --- | | `go/dbkit/connect.go` | Import path back to `github.com/exaring/otelpgx` | | `go.mod` | `require github.com/exaring/otelpgx v0.10.0` + new `replace` directive (with explanatory comment) pointing at `github.com/overmindtech/otelpgx v0.0.0-20260504155636-e00d7db88073` | | `go.sum` | `go mod tidy` refresh | 3 files, 8 insertions, 4 deletions. Verified locally with `go build ./...` and `go test ./go/dbkit/...`. Made with [Cursor](https://cursor.com) <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Low Risk** > Low risk plumbing change: switches `otelpgx` consumption back to the upstream module path while pinning it to an internal fork via `replace`, so runtime behavior should remain the same aside from potential dependency-resolution issues. > > **Overview** > Updates the `otelpgx` dependency wiring to use the upstream import path (`github.com/exaring/otelpgx`) while temporarily sourcing it from an Overmind fork via a new `go.mod` `replace` (to keep the span-removal patch applied). > > Refreshes `go.sum` accordingly and adjusts the only in-repo usage (`go/dbkit/connect.go`) to import `exaring/otelpgx`. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 63daef169d0cd8cd776991b66203b70cdb5dd611. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> Co-authored-by: Cursor <cursoragent@cursor.com> GitOrigin-RevId: 96bc81f19695b4e95caa418b32bc92db3ba17912
Remove span creation for pool.acquire and prepare operations to reduce trace noise. Metrics (db.client.operation.duration, error counts) are preserved for both operations. For prepare, the wall-clock duration is recorded as a pgx.prepare.duration attribute (Int64, milliseconds) on the parent query span instead of creating a separate child span. Pool acquire duration is better tracked through aggregate metrics (pgxpool.* from RecordStats) since contention is an environmental concern, not useful as individual trace spans. Made-with: Cursor Co-authored-by: Cursor <cursoragent@cursor.com>
1521356 to
9a74fca
Compare
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.
Summary
pool.acquireandprepareoperations to reduce trace noisedb.client.operation.duration, error counts) for both operationspgx.prepare.durationattribute (Int64, ms) on the parent query spanRationale
pool.acquire: Pool acquire duration is tracked via
db.client.operation.durationmetric (withpgx.operation.type=acquire) and thepgxpool.*metrics fromRecordStats. As a span in a trace it adds noise without actionable signal — pool contention is an environmental concern (database bottleneck) better diagnosed from aggregate metrics than individual traces.prepare: The prepare round-trip time is useful context on the parent query span, but not worth a dedicated child span. The
pgx.prepare.durationattribute surfaces this on the query span directly.Production usage
Overmind has been running this patch in production since March 2026 across multiple internal services that use
pgxpoolfor PostgreSQL access. We've observed the expected reduction in trace noise (no morepool.acquire/preparechild spans cluttering query traces) with no regressions in pool behaviour, error visibility, or the underlyingdb.client.operation.duration/pgxpool.*metrics.