Skip to content

feat: remove pool.acquire and prepare spans, preserve metrics#2

Open
DavidS-ovm wants to merge 2 commits intomainfrom
remove-acquire-prepare-spans
Open

feat: remove pool.acquire and prepare spans, preserve metrics#2
DavidS-ovm wants to merge 2 commits intomainfrom
remove-acquire-prepare-spans

Conversation

@DavidS-ovm
Copy link
Member

@DavidS-ovm DavidS-ovm commented Mar 3, 2026

Summary

  • Remove span creation for pool.acquire and prepare operations to reduce trace noise
  • Preserve all metrics (db.client.operation.duration, error counts) for both operations
  • For prepare: record wall-clock duration as pgx.prepare.duration attribute (Int64, ms) on the parent query span
  • Update module path to github.com/overmindtech/otelpgx

Rationale

pool.acquire: Pool acquire duration is tracked via db.client.operation.duration metric (with pgx.operation.type=acquire) and the pgxpool.* metrics from RecordStats. 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.duration attribute surfaces this on the query span directly.

Test plan

  • TestTraceAcquire_NoSpan -- verifies no span created, only parent span present
  • TestTracePrepare_NoSpan_SetsAttribute -- verifies no prepare span, parent span has pgx.prepare.duration attribute
  • All existing tests pass

Made with Cursor

@DavidS-ovm
feat: remove pool.acquire and prepare spans, preserve metrics
545238e 
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
@DavidS-ovm
chore: rename module to github.com/overmindtech/otelpgx
65bf101 
Update module path, instrumentation scope names, benchmark import, and
README references from exaring/otelpgx to overmindtech/otelpgx.

Made-with: Cursor
@DavidS-ovm DavidS-ovm force-pushed the remove-acquire-prepare-spans branch from 2511c8f to 65bf101 Compare March 3, 2026 21:04
dylanratcliffe pushed a commit to overmindtech/cli that referenced this pull request Mar 9, 2026
@DavidS-ovm @actions-user
[ENG-2943] Switch to overmindtech/otelpgx fork (no acquire/prepare sp…
894d437 
…ans) (#4103)

## Summary

- Switch from `exaring/otelpgx` to `overmindtech/otelpgx` fork which
removes `pool.acquire` and `prepare` span creation while preserving all
metrics
- Prepare duration is now recorded as a `pgx.prepare.duration` attribute
(Int64, ms) on the parent query span
- Remove the now-unnecessary `pool.acquire` sampling rule from
`OvermindSampler`

## Linear Ticket

- **Ticket**:
[ENG-2943](https://linear.app/overmind/issue/ENG-2943/fork-otelpgx-remove-poolacquire-and-prepare-spans)
— Fork otelpgx: remove pool.acquire and prepare spans
- **Purpose**: Reduce trace noise by eliminating low-value
`pool.acquire` and `prepare` child spans, while preserving
`db.client.operation.duration` metrics for both operations
- **Related**: [ENG-2941](https://linear.app/overmind/issue/ENG-2941) —
complementary to the OTEL collector batch size fix

## Changes

| File | Change |
| --- | --- |
| `go.mod` / `go.sum` | `exaring/otelpgx v0.10.0` replaced with
`overmindtech/otelpgx` (commit `65bf101`) |
| `go/dbkit/connect.go` | Import path updated to
`github.com/overmindtech/otelpgx` |
| `go/tracing/main.go` | Removed `pool.acquire` sampling rule from
`NewOvermindSampler` (no longer needed) |

The fork itself
([overmindtech/otelpgx#2](overmindtech/otelpgx#2))
contains the functional changes to otelpgx.

## Deviations from Approved Plan

Implementation matches the approved plan -- no material deviations. All
seven parts were implemented as described:

1. Fork created at `overmindtech/otelpgx` with two commits
(functionality + rename)
2. `pool.acquire` span removed, metrics preserved, rationale documented
3. `prepare` span removed, `pgx.prepare.duration` attribute added to
parent query span
4. Unused code cleaned up from simplified methods
5. Two tests added (`TestTraceAcquire_NoSpan`,
`TestTracePrepare_NoSpan_SetsAttribute`)
6. Main repo updated (this PR)
7. Upstream PR suggestion deferred to post-validation (as planned)

Made with [Cursor](https://cursor.com)

<!-- CURSOR_SUMMARY -->
---

> [!NOTE]
> **Medium Risk**
> Moderate risk because it changes database OpenTelemetry
instrumentation and sampling rules, which can affect trace volume/shape
and observability expectations, but it is otherwise a small, localized
dependency swap.
>
> **Overview**
> Switches PostgreSQL tracing from `github.com/exaring/otelpgx` to the
`github.com/overmindtech/otelpgx` fork, updating `dbkit.Connect` to use
the new import and bumping module sums accordingly.
>
> Simplifies tracing sampling by removing the special-case sampler for
`pool.acquire` spans (and the now-unused `SpanNameMatcher` helper),
reflecting the fork’s reduced span creation. Adds a
`//nolint:staticcheck` annotation for the Kubernetes `v1.Endpoints`
adapter to suppress deprecation warnings while `EndpointSlice` migration
is pending.
>
> <sup>Written by [Cursor
Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit
2c0f40d231fdad6a06ce4080b1b834cfed6dce42. This will update automatically
on new commits. Configure
[here](https://cursor.com/dashboard?tab=bugbot).</sup>
<!-- /CURSOR_SUMMARY -->

GitOrigin-RevId: cd50535a27c2563fb4dd32592182711892177b7d
tphoney pushed a commit to overmindtech/cli that referenced this pull request Mar 9, 2026
@DavidS-ovm @tphoney
[ENG-2943] Switch to overmindtech/otelpgx fork (no acquire/prepare sp…
404c2aa 
…ans) (#4103)

## Summary

- Switch from `exaring/otelpgx` to `overmindtech/otelpgx` fork which
removes `pool.acquire` and `prepare` span creation while preserving all
metrics
- Prepare duration is now recorded as a `pgx.prepare.duration` attribute
(Int64, ms) on the parent query span
- Remove the now-unnecessary `pool.acquire` sampling rule from
`OvermindSampler`

## Linear Ticket

- **Ticket**:
[ENG-2943](https://linear.app/overmind/issue/ENG-2943/fork-otelpgx-remove-poolacquire-and-prepare-spans)
— Fork otelpgx: remove pool.acquire and prepare spans
- **Purpose**: Reduce trace noise by eliminating low-value
`pool.acquire` and `prepare` child spans, while preserving
`db.client.operation.duration` metrics for both operations
- **Related**: [ENG-2941](https://linear.app/overmind/issue/ENG-2941) —
complementary to the OTEL collector batch size fix

## Changes

| File | Change |
| --- | --- |
| `go.mod` / `go.sum` | `exaring/otelpgx v0.10.0` replaced with
`overmindtech/otelpgx` (commit `65bf101`) |
| `go/dbkit/connect.go` | Import path updated to
`github.com/overmindtech/otelpgx` |
| `go/tracing/main.go` | Removed `pool.acquire` sampling rule from
`NewOvermindSampler` (no longer needed) |

The fork itself
([overmindtech/otelpgx#2](overmindtech/otelpgx#2))
contains the functional changes to otelpgx.

## Deviations from Approved Plan

Implementation matches the approved plan -- no material deviations. All
seven parts were implemented as described:

1. Fork created at `overmindtech/otelpgx` with two commits
(functionality + rename)
2. `pool.acquire` span removed, metrics preserved, rationale documented
3. `prepare` span removed, `pgx.prepare.duration` attribute added to
parent query span
4. Unused code cleaned up from simplified methods
5. Two tests added (`TestTraceAcquire_NoSpan`,
`TestTracePrepare_NoSpan_SetsAttribute`)
6. Main repo updated (this PR)
7. Upstream PR suggestion deferred to post-validation (as planned)

Made with [Cursor](https://cursor.com)

<!-- CURSOR_SUMMARY -->
---

> [!NOTE]
> **Medium Risk**
> Moderate risk because it changes database OpenTelemetry
instrumentation and sampling rules, which can affect trace volume/shape
and observability expectations, but it is otherwise a small, localized
dependency swap.
>
> **Overview**
> Switches PostgreSQL tracing from `github.com/exaring/otelpgx` to the
`github.com/overmindtech/otelpgx` fork, updating `dbkit.Connect` to use
the new import and bumping module sums accordingly.
>
> Simplifies tracing sampling by removing the special-case sampler for
`pool.acquire` spans (and the now-unused `SpanNameMatcher` helper),
reflecting the fork’s reduced span creation. Adds a
`//nolint:staticcheck` annotation for the Kubernetes `v1.Endpoints`
adapter to suppress deprecation warnings while `EndpointSlice` migration
is pending.
>
> <sup>Written by [Cursor
Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit
2c0f40d231fdad6a06ce4080b1b834cfed6dce42. This will update automatically
on new commits. Configure
[here](https://cursor.com/dashboard?tab=bugbot).</sup>
<!-- /CURSOR_SUMMARY -->

GitOrigin-RevId: cd50535a27c2563fb4dd32592182711892177b7d
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@DavidS-ovm