refactor(go/adbc): refactor logging instrumentation into OTel tracing - part 1/3 - #4655
Conversation
birschick-bq
changed the title
|
Is there some more meaningful way to split this up than part 1/2/3? Can we break it into parts that can each have meaningful descriptions? It's not clear to me how these changes were grouped |
Yes. I'll work on it. |
|
Ok, thanks. Let me know if that's too much of a hassle. I wish the stacked PRs feature worked for us...I gave it a try, but it means you/others need push access to this repo which is a non-starter (even for committers I'd rather we not push dev branches to this repo). Hopefully that gets fixed. |
Without a re-write here are the individual stacked PRs for part 2 and 3 on my repo @lidavidm - Let me know if this is satisfactory. Thanks. |
|
@davidhcoe - if you have time to review, this refactors your logging instrumentation into OTel tracing. |
| for _, loc := range endpoint.Location { | ||
| uris = append(uris, loc.Uri) | ||
| } | ||
| attrs = append(attrs, attribute.StringSlice("locations", uris)) |
There was a problem hiding this comment.
(1) Is this actually useful to record?
(2) While it doesn't apply to gRPC, some implementations may be putting things like presigned URLs into "locations" (I've seen this before)
There was a problem hiding this comment.
This is a port of work that @davidhcoe did in
PR for
func endpointLogAttrs(endpointIndex, numEndpoints int, endpoint *flight.FlightEndpoint) []any {
lidavidm
changed the title
| } | ||
| } | ||
|
|
||
| func (h *EndSpanHelper) WithError(err error) *EndSpanHelper { |
There was a problem hiding this comment.
nit: it would be good to document what these do and the helper overall
There was a problem hiding this comment.
@lidavidm - I will add docs for the helper and methods. Thanks.
3 participants
This pull request introduces OpenTelemetry tracing to the FlightSQL driver, enhancing observability for record reading and endpoint streaming operations. The main changes involve adding tracing hooks, attributes, and error recording to critical code paths, as well as refactoring logging and context management to support tracing. Additionally, new utility functions for collecting and attaching response metadata to traces are implemented.
Tracing and Observability Enhancements:
record_reader.go, including span creation, event recording, and error tracking innewRecordReaderand endpoint goroutines. This enables detailed tracing of FlightSQL record reading operations. [1] [2] [3] [4]flightsql_tracing.gofor collecting response metadata, building trace attributes for endpoints, and summarizing stream progress as OpenTelemetry attributes.Refactoring for Tracing Integration:
logging.gowith tracing attribute builders, and removed now-redundant logging functions. [1] [2]record_reader.goto usecontext.CancelCauseFuncfor improved error propagation and tracing. [1] [2]Internal API and Dependency Updates:
recordReaderConfigstruct to include a tracing configuration parameter, enabling tracing to be passed through to record readers.These changes collectively provide fine-grained tracing and error visibility for FlightSQL operations, making it easier to monitor, debug, and analyze the driver's behavior in production environments.
Refactors slog instrumentation into OTel tracing.
flightsql_tracing.goto provide tracing wrappers originally provided inlogging.godriverbaseto handle improved trace handlingPart 1 of a multi-part change to refactor logging instrumentation into OTel tracing.
Stacked PRs in my personal forked repo ...
Part 2
Part 3