birschick-bq commented on code in PR #2729:
URL: https://github.com/apache/arrow-adbc/pull/2729#discussion_r2080665928
##########
go/adbc/driver/internal/driverbase/connection.go:
##########
@@ -714,4 +771,50 @@ func ValueOrZero[T any](val *T) T {
return *val
}
+func maybeAddTraceParent(ctx context.Context, cnxn adbc.OTelTracing, st
adbc.OTelTracing) (context.Context, error) {
+ var traceParentStr string
+ if st != nil && st.GetTraceParent() != "" {
+ traceParentStr = st.GetTraceParent()
+ } else if cnxn != nil && cnxn.GetTraceParent() != "" {
+ traceParentStr = cnxn.GetTraceParent()
+ }
+ if traceParentStr != "" {
+ spanContext, err := propagateTraceParent(ctx, traceParentStr)
+ if err != nil {
+ return ctx, err
+ }
+ ctx = trace.ContextWithRemoteSpanContext(ctx, spanContext)
+ }
+ return ctx, nil
+}
+
+func propagateTraceParent(ctx context.Context, traceParentStr string)
(trace.SpanContext, error) {
+ if strings.TrimSpace(traceParentStr) == "" {
+ return trace.SpanContext{}, errors.New("traceparent string is
empty")
+ }
+
+ propagator := propagation.TraceContext{}
+ carrier := propagation.MapCarrier{"traceparent": traceParentStr}
+ extractedContext := propagator.Extract(ctx, carrier)
+
+ spanContext := trace.SpanContextFromContext(extractedContext)
+ if !spanContext.IsValid() {
+ return trace.SpanContext{}, errors.New("invalid traceparent
string")
+ }
+ return spanContext, nil
+}
+
+func isValidateTraceParent(traceParent string) bool {
Review Comment:
(1) As we are discarding any error when adding the trace parent, I think it
is okay to remove the validation.
##########
go/adbc/driver/internal/driverbase/rotating_file_writer.go:
##########
Review Comment:
This handles the scenario
1. when the end-user is not authorized or doesn't want to
install/deploy/configure an OTLP Collector
2. the caller cannot directly capture `stdout` - for example when a
background process is started
##########
go/adbc/driver/snowflake/statement.go:
##########
@@ -499,41 +516,58 @@ func (st *statement) ExecuteQuery(ctx context.Context)
(array.RecordReader, int6
st.streamBind = nil
rdr := concatReader{}
- err := rdr.Init(&bind)
+ err = rdr.Init(&bind)
if err != nil {
- return nil, -1, err
+ return
}
- return &rdr, -1, nil
+ reader = &rdr
+ return
}
- loader, err := st.cnxn.cn.QueryArrowStream(ctx, st.query)
+ var loader gosnowflake.ArrowStreamLoader
+ loader, err = st.cnxn.cn.QueryArrowStream(ctx, st.query)
if err != nil {
- return nil, -1, errToAdbcErr(adbc.StatusInternal, err)
+ err = errToAdbcErr(adbc.StatusInternal, err)
+ return
}
- rdr, err := newRecordReader(ctx, st.alloc, loader, st.queueSize,
st.prefetchConcurrency, st.useHighPrecision)
- nrec := loader.TotalRows()
- return rdr, nrec, err
+ reader, err = newRecordReader(ctx, st.alloc, loader, st.queueSize,
st.prefetchConcurrency, st.useHighPrecision)
+ nRows = loader.TotalRows()
+ return
}
// ExecuteUpdate executes a statement that does not generate a result
// set. It returns the number of rows affected if known, otherwise -1.
-func (st *statement) ExecuteUpdate(ctx context.Context) (int64, error) {
+func (st *statement) ExecuteUpdate(ctx context.Context) (numRows int64, err
error) {
+ numRows = -1
+
+ var span trace.Span
+ ctx, span = st.StartSpan(ctx, "ExecuteUpdate")
ctx = st.setQueryContext(ctx)
+ defer func() {
+ if !st.SetErrorOnSpan(span, err) {
+
span.SetAttributes(attribute.Int64("db.response.returned_rows", numRows))
Review Comment:
@lidavidm
To the best of my view, -1 returned in the case of all errors. It seems to
be by design.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
