Skip to content

Commit f3dbc6b

Browse files
refactor(go/adbc): refactor logging instrumentation into OTel tracing - part 1/3 (#4655)
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:** * Added OpenTelemetry tracing support to `record_reader.go`, including span creation, event recording, and error tracking in `newRecordReader` and endpoint goroutines. This enables detailed tracing of FlightSQL record reading operations. * Introduced new tracing utility functions and types in `flightsql_tracing.go` for collecting response metadata, building trace attributes for endpoints, and summarizing stream progress as OpenTelemetry attributes. **Refactoring for Tracing Integration:** * Replaced the logger-based endpoint and stream progress attribute builders in `logging.go` with tracing attribute builders, and removed now-redundant logging functions. * Updated context and cancellation handling in `record_reader.go` to use `context.CancelCauseFunc` for improved error propagation and tracing. **Internal API and Dependency Updates:** * Added OpenTelemetry and internal tracing imports to relevant files to support the new tracing features. * Extended the `recordReaderConfig` struct 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. - update utilities to instrument duration for a span from a given start time. - update utilities to separate already recorded error to avoid duplicate events in the span - adds a `flightsql_tracing.go` to provide tracing wrappers originally provided in `logging.go` - some updates in `driverbase` to handle improved trace handling - some initial instrumentation in flightsql connection and reader Part 1 of a multi-part change to refactor logging instrumentation into OTel tracing. --------- Co-authored-by: Bruce Irschick (Bit Quill Technologies Inc) <v-birschick@microsoft.com>
1 parent ddfbd4a commit f3dbc6b

10 files changed

Lines changed: 750 additions & 135 deletions

File tree

go/adbc/driver/flightsql/flightsql_connection.go

Lines changed: 149 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,8 @@ import (
3939
flightproto "github.com/apache/arrow-go/v18/arrow/flight/gen/flight"
4040
"github.com/apache/arrow-go/v18/arrow/ipc"
4141
"github.com/bluele/gcache"
42+
"go.opentelemetry.io/otel/attribute"
43+
"go.opentelemetry.io/otel/trace"
4244
"google.golang.org/grpc"
4345
grpccodes "google.golang.org/grpc/codes"
4446
"google.golang.org/grpc/metadata"
@@ -232,6 +234,153 @@ var adbcToFlightSQLInfo = map[adbc.InfoCode]flightsql.SqlInfo{
232234
adbc.InfoVendorSubstraitMaxVersion: flightsql.SqlInfoFlightSqlServerSubstraitMaxVersion,
233235
}
234236

237+
func doGetWithResponseMetadata(ctx context.Context, client *flightsql.Client, ticket *flight.Ticket, opts ...grpc.CallOption) (*flight.Reader, error) {
238+
var header, trailer metadata.MD
239+
callOpts := append(append([]grpc.CallOption{}, opts...), grpc.Header(&header), grpc.Trailer(&trailer))
240+
reader, err := client.DoGet(ctx, ticket, callOpts...)
241+
if err != nil {
242+
captureResponseMetadata(ctx, metadata.Join(header, trailer))
243+
}
244+
return reader, err
245+
}
246+
247+
func doGetWithTracer(ctx context.Context, cl *flightsql.Client, endpoint *flight.FlightEndpoint, clientCache gcache.Cache, tracing adbc.OTelTracing, opts ...grpc.CallOption) (rdr *flight.Reader, err error) {
248+
const spanName = "FlightSQL.Connection.DoGet"
249+
startTime := time.Now()
250+
ctx, span := internal.StartSpan(ctx, spanName, tracing)
251+
errorRecorded := false
252+
defer func() {
253+
internal.NewEndSpanHelper(span).
254+
WithStartTime(startTime).
255+
WithError(err).
256+
WithRecordedError(errorRecorded).
257+
EndSpan()
258+
}()
259+
260+
streamOpts := make([]grpc.CallOption, 0, len(opts))
261+
for _, opt := range opts {
262+
switch opt.(type) {
263+
case grpc.HeaderCallOption, *grpc.HeaderCallOption, grpc.TrailerCallOption, *grpc.TrailerCallOption:
264+
continue
265+
default:
266+
streamOpts = append(streamOpts, opt)
267+
}
268+
}
269+
270+
if len(endpoint.Location) == 0 {
271+
span.AddEvent("flight.location.attempt", trace.WithAttributes(
272+
attribute.String("flight.location.source", "default_client"),
273+
))
274+
start := time.Now()
275+
rdr, err = doGetWithResponseMetadata(ctx, cl, endpoint.Ticket, streamOpts...)
276+
attrs := []attribute.KeyValue{
277+
attribute.Float64("duration_s", time.Since(start).Seconds()),
278+
attribute.String("flight.location.source", "default_client"),
279+
}
280+
if err != nil {
281+
attrs = append(attrs, attribute.String("flight.stage", "do_get"))
282+
span.RecordError(err, trace.WithAttributes(attrs...), trace.WithStackTrace(true))
283+
errorRecorded = true
284+
} else {
285+
span.AddEvent("flight.location.selected", trace.WithAttributes(attrs...))
286+
}
287+
return rdr, err
288+
}
289+
290+
var (
291+
cc interface{}
292+
hasFallback bool
293+
attemptErrors []string
294+
)
295+
296+
for _, loc := range endpoint.Location {
297+
if loc.Uri == flight.LocationReuseConnection {
298+
hasFallback = true
299+
continue
300+
}
301+
302+
start := time.Now()
303+
span.AddEvent("flight.location.attempt", trace.WithAttributes(
304+
attribute.String("flight.location", loc.Uri),
305+
attribute.String("flight.location.source", "endpoint"),
306+
))
307+
cc, err = clientCache.Get(loc.Uri)
308+
if err != nil {
309+
attemptErrors = append(attemptErrors, fmt.Sprintf("clientCache.Get(%q): %s", loc.Uri, err.Error()))
310+
span.AddEvent("flight.location.failed", trace.WithAttributes(
311+
attribute.String("flight.stage", "client_cache_get"),
312+
attribute.String("flight.location", loc.Uri),
313+
attribute.Float64("duration_s", time.Since(start).Seconds()),
314+
attribute.String("error.message", err.Error()),
315+
))
316+
continue
317+
}
318+
319+
conn := cc.(*flightsql.Client)
320+
rdr, err = doGetWithResponseMetadata(ctx, conn, endpoint.Ticket, streamOpts...)
321+
if err != nil {
322+
attemptErrors = append(attemptErrors, fmt.Sprintf("DoGet(%q): %s", loc.Uri, err.Error()))
323+
span.AddEvent("flight.location.failed", trace.WithAttributes(
324+
attribute.String("flight.stage", "do_get"),
325+
attribute.String("flight.location", loc.Uri),
326+
attribute.Float64("duration_s", time.Since(start).Seconds()),
327+
attribute.String("error.message", err.Error()),
328+
))
329+
continue
330+
}
331+
332+
span.AddEvent("flight.location.selected", trace.WithAttributes(
333+
attribute.String("flight.location", loc.Uri),
334+
attribute.String("flight.location.source", "endpoint"),
335+
attribute.Float64("duration_s", time.Since(start).Seconds()),
336+
))
337+
return
338+
}
339+
340+
if hasFallback {
341+
start := time.Now()
342+
span.AddEvent("flight.location.attempt", trace.WithAttributes(
343+
attribute.String("flight.location.source", "fallback"),
344+
))
345+
rdr, err = doGetWithResponseMetadata(ctx, cl, endpoint.Ticket, streamOpts...)
346+
if err != nil {
347+
attemptErrors = append(attemptErrors, fmt.Sprintf("DoGet(fallback to default client): %s", err.Error()))
348+
span.AddEvent("flight.location.failed", trace.WithAttributes(
349+
attribute.String("flight.stage", "do_get"),
350+
attribute.String("flight.location.source", "fallback"),
351+
attribute.Float64("duration_s", time.Since(start).Seconds()),
352+
attribute.String("error.message", err.Error()),
353+
))
354+
err = fmt.Errorf("all DoGet attempts failed: %s; final: %w", strings.Join(attemptErrors, "; "), err)
355+
span.RecordError(err, trace.WithAttributes(
356+
attribute.String("flight.stage", "all_locations_failed"),
357+
attribute.Int("flight.location.attempt_count", len(attemptErrors)),
358+
), trace.WithStackTrace(true))
359+
errorRecorded = true
360+
return nil, err
361+
}
362+
span.AddEvent("flight.location.selected", trace.WithAttributes(
363+
attribute.String("flight.location.source", "fallback"),
364+
attribute.Float64("duration_s", time.Since(start).Seconds()),
365+
))
366+
return rdr, nil
367+
}
368+
369+
if err != nil && len(attemptErrors) > 1 {
370+
err = fmt.Errorf("all %d DoGet location(s) failed: %s; final: %w",
371+
len(attemptErrors), strings.Join(attemptErrors, "; "), err)
372+
}
373+
if err != nil {
374+
span.RecordError(err, trace.WithAttributes(
375+
attribute.String("flight.stage", "all_locations_failed"),
376+
attribute.Int("flight.location.attempt_count", len(attemptErrors)),
377+
), trace.WithStackTrace(true))
378+
errorRecorded = true
379+
}
380+
381+
return nil, err
382+
}
383+
235384
// doGetWithLogger performs DoGet against an endpoint's locations, logging each
236385
// attempt and joining all per-location failures into the returned error so the
237386
// caller can see every location that was tried. logger may be nil.

go/adbc/driver/flightsql/flightsql_database.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -513,7 +513,11 @@ func (d *databaseImpl) Open(ctx context.Context) (_ adbc.Connection, err error)
513513
d,
514514
trace.WithAttributes(traceHeaderAttrsWithPrefix(d.hdrs, traceRequestMetadataPrefix)...),
515515
)
516-
defer internal.EndSpanWithError(span, &err)
516+
defer func() {
517+
internal.NewEndSpanHelper(span).
518+
WithError(err).
519+
EndSpan()
520+
}()
517521

518522
authMiddle := &bearerAuthMiddleware{hdrs: d.hdrs.Copy(), logger: safeLogger(d.Logger)}
519523
var cookies flight.CookieMiddleware

go/adbc/driver/flightsql/flightsql_statement.go

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -538,7 +538,11 @@ func (s *statement) ExecuteQuery(ctx context.Context) (rdr array.RecordReader, n
538538
s.cnxn,
539539
trace.WithAttributes(traceHeaderAttrsWithPrefix(s.hdrs, traceRequestMetadataPrefix)...),
540540
)
541-
defer internal.EndSpanWithError(span, &err)
541+
defer func() {
542+
internal.NewEndSpanHelper(span).
543+
WithError(err).
544+
EndSpan()
545+
}()
542546

543547
// Handle bulk ingest
544548
if s.targetTable != "" {
@@ -618,7 +622,11 @@ func (s *statement) ExecuteUpdate(ctx context.Context) (n int64, err error) {
618622
s.cnxn,
619623
trace.WithAttributes(traceHeaderAttrsWithPrefix(s.hdrs, traceRequestMetadataPrefix)...),
620624
)
621-
defer internal.EndSpanWithError(span, &err)
625+
defer func() {
626+
internal.NewEndSpanHelper(span).
627+
WithError(err).
628+
EndSpan()
629+
}()
622630

623631
// Handle bulk ingest
624632
if s.targetTable != "" {
@@ -672,7 +680,11 @@ func (s *statement) Prepare(ctx context.Context) (err error) {
672680
s.cnxn,
673681
trace.WithAttributes(traceHeaderAttrsWithPrefix(s.hdrs, traceRequestMetadataPrefix)...),
674682
)
675-
defer internal.EndSpanWithError(span, &err)
683+
defer func() {
684+
internal.NewEndSpanHelper(span).
685+
WithError(err).
686+
EndSpan()
687+
}()
676688

677689
startTime := time.Now()
678690
s.log.InfoContext(ctx, "FlightSQL Prepare start", s.queryAttrs()...)
Lines changed: 179 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,179 @@
1+
// Licensed to the Apache Software Foundation (ASF) under one
2+
// or more contributor license agreements. See the NOTICE file
3+
// distributed with this work for additional information
4+
// regarding copyright ownership. The ASF licenses this file
5+
// to you under the Apache License, Version 2.0 (the
6+
// "License"); you may not use this file except in compliance
7+
// with the License. You may obtain a copy of the License at
8+
//
9+
// http://www.apache.org/licenses/LICENSE-2.0
10+
//
11+
// Unless required by applicable law or agreed to in writing,
12+
// software distributed under the License is distributed on an
13+
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
14+
// KIND, either express or implied. See the License for the
15+
// specific language governing permissions and limitations
16+
// under the License.
17+
18+
package flightsql
19+
20+
import (
21+
"context"
22+
"encoding/hex"
23+
"fmt"
24+
"sync"
25+
"time"
26+
27+
"github.com/apache/arrow-go/v18/arrow/flight"
28+
"go.opentelemetry.io/otel/attribute"
29+
"google.golang.org/grpc"
30+
"google.golang.org/grpc/metadata"
31+
)
32+
33+
type responseMetadataKey struct{}
34+
35+
type responseMetadataCollector struct {
36+
mutex sync.RWMutex
37+
value metadata.MD
38+
}
39+
40+
func withResponseMetadata(ctx context.Context) (context.Context, *responseMetadataCollector) {
41+
collector := &responseMetadataCollector{}
42+
return context.WithValue(ctx, responseMetadataKey{}, collector), collector
43+
}
44+
45+
func captureResponseMetadata(ctx context.Context, value metadata.MD) {
46+
collector, ok := responseMetadataFromContext(ctx)
47+
if !ok {
48+
return
49+
}
50+
collector.mutex.Lock()
51+
collector.value = value.Copy()
52+
defer collector.mutex.Unlock()
53+
}
54+
55+
func responseMetadataFromContext(ctx context.Context) (*responseMetadataCollector, bool) {
56+
collector, ok := ctx.Value(responseMetadataKey{}).(*responseMetadataCollector)
57+
return collector, ok
58+
}
59+
60+
func (c *responseMetadataCollector) snapshot() metadata.MD {
61+
c.mutex.RLock()
62+
defer c.mutex.RUnlock()
63+
return c.value.Copy()
64+
}
65+
66+
func responseMetadataStreamInterceptor(ctx context.Context, desc *grpc.StreamDesc, cc *grpc.ClientConn, method string, streamer grpc.Streamer, opts ...grpc.CallOption) (grpc.ClientStream, error) {
67+
stream, err := streamer(ctx, desc, cc, method, opts...)
68+
if err != nil {
69+
return stream, err
70+
}
71+
if _, ok := responseMetadataFromContext(ctx); !ok {
72+
return stream, nil
73+
}
74+
return &responseMetadataClientStream{ClientStream: stream, ctx: ctx}, nil
75+
}
76+
77+
type responseMetadataClientStream struct {
78+
grpc.ClientStream
79+
ctx context.Context
80+
}
81+
82+
func (s *responseMetadataClientStream) RecvMsg(message interface{}) error {
83+
err := s.ClientStream.RecvMsg(message)
84+
if err != nil {
85+
header, _ := s.Header()
86+
captureResponseMetadata(s.ctx, metadata.Join(header, s.Trailer()))
87+
}
88+
return err
89+
}
90+
91+
// endpointTraceKeyValues builds OpenTelemetry attributes describing a Flight
92+
// endpoint. Ticket contents are intentionally never recorded.
93+
func endpointTraceKeyValues(endpointIndex, numEndpoints int, endpoint *flight.FlightEndpoint) []attribute.KeyValue {
94+
attrs := []attribute.KeyValue{
95+
attribute.Int("endpointIndex", endpointIndex),
96+
attribute.Int("numEndpoints", numEndpoints),
97+
}
98+
if endpoint == nil {
99+
return attrs
100+
}
101+
if endpoint.Ticket != nil {
102+
attrs = append(attrs, attribute.Int("ticketBytes", len(endpoint.Ticket.Ticket)))
103+
}
104+
if len(endpoint.Location) == 0 {
105+
attrs = append(attrs, attribute.String("locations", "<empty: using default client connection>"))
106+
} else {
107+
uris := make([]string, 0, len(endpoint.Location))
108+
for _, loc := range endpoint.Location {
109+
uris = append(uris, loc.Uri)
110+
}
111+
attrs = append(attrs, attribute.StringSlice("locations", uris))
112+
}
113+
if endpoint.ExpirationTime != nil {
114+
attrs = append(attrs, attribute.String("expirationTime", endpoint.ExpirationTime.AsTime().String()))
115+
}
116+
return attrs
117+
}
118+
119+
// logKeyValues returns OpenTelemetry attributes summarizing stream progress.
120+
func (p *streamProgress) logKeyValues() []attribute.KeyValue {
121+
attrs := []attribute.KeyValue{
122+
attribute.Int64("batchesRead", p.batchesRead),
123+
attribute.Int64("recordsRead", p.recordsRead),
124+
attribute.Int64("approxBytesRead", p.bytesEstimate),
125+
attribute.String("elapsed", time.Since(p.start).String()),
126+
}
127+
if !p.firstBatchAt.IsZero() {
128+
attrs = append(attrs, attribute.String("timeToFirstBatch", p.firstBatchAt.Sub(p.start).String()))
129+
} else {
130+
attrs = append(attrs, attribute.String("timeToFirstBatch", "never"))
131+
}
132+
if !p.lastBatchAt.IsZero() {
133+
attrs = append(attrs, attribute.String("timeSinceLastBatch", time.Since(p.lastBatchAt).String()))
134+
}
135+
return attrs
136+
}
137+
138+
// flightInfoTracingKeyValues returns OpenTelemetry attributes describing a FlightInfo:
139+
// descriptor type and command prefix, AppMetadata prefix (some backends
140+
// embed a server-side query handle there), and advisory record/byte
141+
// counts. Returns nil for a nil info.
142+
func flightInfoTracingKeyValues(info *flight.FlightInfo) []attribute.KeyValue {
143+
if info == nil {
144+
return nil
145+
}
146+
attrs := []attribute.KeyValue{
147+
attribute.Int("numEndpoints", len(info.Endpoint)),
148+
attribute.Int64("totalRecords", info.TotalRecords),
149+
attribute.Int64("totalBytes", info.TotalBytes),
150+
attribute.Bool("haveSchemaInFlightInfo", len(info.Schema) > 0),
151+
}
152+
if desc := info.FlightDescriptor; desc != nil {
153+
attrs = append(attrs, attribute.String("descriptorType", desc.Type.String()))
154+
if len(desc.Cmd) > 0 {
155+
limit := len(desc.Cmd)
156+
if limit > maxLoggedBlobBytes {
157+
limit = maxLoggedBlobBytes
158+
}
159+
attrs = append(attrs,
160+
attribute.Int("descriptorCmdBytes", len(desc.Cmd)),
161+
attribute.String("descriptorCmdPrefixHex", hex.EncodeToString(desc.Cmd[:limit])),
162+
)
163+
}
164+
if len(desc.Path) > 0 {
165+
attrs = append(attrs, attribute.String("descriptorPath", fmt.Sprint(desc.Path)))
166+
}
167+
}
168+
if len(info.AppMetadata) > 0 {
169+
limit := len(info.AppMetadata)
170+
if limit > maxLoggedBlobBytes {
171+
limit = maxLoggedBlobBytes
172+
}
173+
attrs = append(attrs,
174+
attribute.Int("appMetadataBytes", len(info.AppMetadata)),
175+
attribute.String("appMetadataPrefixHex", hex.EncodeToString(info.AppMetadata[:limit])),
176+
)
177+
}
178+
return attrs
179+
}

0 commit comments

Comments
 (0)