IN Queries Failing w/ OTEL Configured

[mnotti]

Not sure if this is a bug or I'm doing something wrong here...

When I add tracing like so:

driver, err := otelsql.Register("postgres", "postgres")
if err != nil {
   return nil, fmt.Errorf("registering Postgres driver for instrumentation: %v", err)
}
pgConnection, err := sqlx.Connect(driver, pgOpts)

I'm getting errors only with IN queries that I construct like so:

query, args, err := sqlx.In(getTeamsForEnvsQuery, envIds)
if err != nil {
    return nil, errors.Wrap(err, "error creating 'IN' query")
}
var results []TeamEnvResult
if err = t.SelectContext(ctx, &results, t.Rebind(query), args...); err != nil {
    return nil, errors.Wrap(err, "error querying team table for teamIds")
}

where getTeamsForEnvsQuery is:

SELECT e.id as "environmentId", p."teamId"
FROM product p
JOIN environment e ON e."productId" = p.id
WHERE e.id IN (?)

(using github.com/jmoiron/sqlx as my postgres client)

When I forget about otelsql and just register the driver like so i get no errors and everything runs smoothly:

pgConnection, err := sqlx.Connect("postgres" /*driver name*/, pgOpts)

Am I doing something wrong here? Or is this an issue?

[XSAM]

Thanks for filing this issue. @mnotti

Would you like to paste errors so I will have more context about this issue? Also, do you have a small project that reproduces the errors? It would help me a lot if you could put up a ready reproducer.

[mnotti]

Hi @XSAM sorry for the long delay, I ended up tabling the issue and forgetting about it until it bit me again (which is now) ...

Slightly different situation now, but I have an example of similar behavior w/ clickhouse instead of postgres. (when I drop the otel stuff it works)

Pushed an example here: https://github.com/mnotti/otel-failure-example which when executed w/ go test ./... will fail with the following error:

client_test.go:43:
        	Error Trace:	client_test.go:43
        	Error:      	Received unexpected error:
        	            	driver: skip fast-path; continue as if unimplemented
        	Test:       	TestTableCreateInsertSelect

However, if you comment out https://github.com/mnotti/otel-failure-example/blob/8e3d929c026d45ce66e609572d0e508d1ea96e3d/client.go#L33-L37 and uncomment L38 to remove the OTEL driver logic, everything passes just fine.

What's strange is it hiccups on

tx, err := ch.BeginTx(ctx, nil)
assert.NoError(t, err)

[XSAM]

Hi, @mnotti thanks for providing the reproducer.

otelsql assumes every driver.Conn implements these driver.XXXContext/driver.XXXTx interfaces that use context.Context for span collection.

https://github.com/XSAM/otelsql/blob/b8a2dd4f7f2455ddf98eb8f08e5f2f87be809e08/conn.go#L24-L35

But if a driver.Conn did not implement these interfaces, like clickhouse.stdDriver did not implement driver.ConnBeginTx, it will throw driver.ErrSkip error.

https://github.com/ClickHouse/clickhouse-go/blob/ee79713daf7a10ba3b363797f74b0909be463b49/clickhouse_std.go#L92-L95

https://github.com/XSAM/otelsql/blob/b8a2dd4f7f2455ddf98eb8f08e5f2f87be809e08/conn.go#L184-L187

---

There are two solutions in my mind:

1. otelsql can return a certain struct that does not implement certain interfaces depending on the underlying implementation.

   c := &otConn{
   	Conn: conn,
   	cfg:  cfg,
   }
   
   if _, ok := conn.(driver.ConnBeginTx); ok {
   	return c
   }
   
   // Only implements driver.Driver
   return struct {
   	driver.Conn
   	driver.Pinger
   	driver.ExecerContext
   	driver.QueryerContext
   	...
   }{c, c, c, c}
   

   But there are too many combinations to fully support these exceptions. I would not recommend this.

2. Let clickhouse.stdDriver support driver.ConnBeginTx.

[jhchabran]

@mnotti about

Let clickhouse.stdDriver support driver.ConnBeginTx.

I went around a similar problem by wrapping the original driver, you can check it here: https://github.com/sourcegraph/sourcegraph/pull/39300/files#diff-bc34aa163694ecf12848f338a0eccb4a222a5d61dc30061d597fc78f8238c906R86

[XSAM]

I think this issue should be fixed, as the clickhouse recently implemented the driver.ConnBeginTx.

https://github.com/ClickHouse/clickhouse-go/blob/8b2d0a80754beb334f1ffc9e8b50df4d433cdd40/clickhouse_std.go#L202

---

Feel free to re-open this issue if you have any questions.