Skip to content

Conversation

flowchartsman
Copy link
Contributor

@flowchartsman flowchartsman commented Apr 26, 2025

Was reading #30 and thought the tracing idea sounded interesting, so I thought I'd do a quick WIP of the idea to play around with the technique and maybe use it in my own projects. Interested in feedback.

Still fiddling with the ergonomics, but I like the abstraction of tracerSpan combined with defer, since it helps it get out of the way. Also not too sure what should go in AcquireStartData beyond the start time. It might be more useful only to track AcquireEnd. Maybe @jackc cares to weigh in, since you've had three years of experience with the technique now.

Ended up going with the big interface, but that's hardly solid, though splitting it up would require picking a required subset, much as pgx does with QueryTracer, and I'm not sure what subset that should be. The BaseTracer type also allows the caller to implement any subset of tracer methods and that might need adjustment if you want to ensure that implementations support both methods for each tracing type, but these are basic implementation details and easy enough to change with more granular interfaces and a new span type for each one.

Probably also want to add some benchmarking.

Happy to keep going if there's interest.

- Might need something similar for tracing resource lifecycle stuff, since there's no context integration into Release() et al.
@flowchartsman
Copy link
Contributor Author

Some selected go test -v output (forgive the line numbers, I got a little carried away with t.Helper())

=== RUN   TestPoolAcquireReusesResources
   puddle/tracing.go:67: got new resource after 21.958µs. blocked for 0s, init took: 21.583µs
   puddle/tracing.go:67: got pooled resource after 250ns. blocked for 0s, resource is 128.375µs old and has been idle for 416ns
--- PASS: TestPoolAcquireReusesResources (0.00s)
=== RUN   TestPoolAcquireContextCanceledDuringCreate
    puddle/tracing.go:52: Acquire error after 100.701ms: context canceled
--- PASS: TestPoolAcquireContextCanceledDuringCreate (0.10s)

this change reduces the need to have two separate calls fo trace.acquireEnd for each instance (warm vs. new resource)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant