feat(go/adbc): prototype OpenTelemetry trace file exporter in go driver #2729

birschick-bq · 2025-04-22T04:38:00Z

Initial implementation of integrating OpenTelemetry tracing into the go drivers.

Instantiates an OTel exporter in the driverbase.database based on the value of the environment variable OTEL_TRACES_EXPORTER. Supports the values none, otlp, console and adbcfile

none (default) indicates no tracing should be exported.
otlp indicates tracing should be exported to an OTLP receiver. By default, the endpoint is http://localhost:4318 using HTTP/protobufs protocol. The OTLP exporter can be configured using the environment variables described here.
console indicates tracing should be exported to stdout.
adbcfile indicates tracing should be exported to a tracing file folder using a rotating file mechanism. The default folder is ~/.adbc/traces/ and the files are named according the driver used and the (UTC) time they were first created. Files are generated such that no file is greater than ~ 1 MB and there are never more than 100 archived trace files.

If the OTLP exporter option is chosen, a OTLP receiver is required. One option is the OpenTelemetry Collector.

This PR includes some instrumentation in the Snowflake driver to indicate how to add trace information.

Resolves: #2210

…lass

go/adbc/driver/snowflake/connection.go

birschick-bq · 2025-04-30T22:20:36Z

@davidhcoe / @CurtHagenlocher
Prototype/POC for OTel tracing in the go drivers.

zeroshade

Overall looks good, but there's lots of areas that can be simplified or otherwise more concise. Plus a few spots where you lose the span context

go/adbc/driver/internal/driverbase/connection.go

go/adbc/driver/snowflake/statement.go

go/adbc/driver/internal/driverbase/database.go

go/adbc/driver/internal/driverbase/rotating_file_writer.go

go/adbc/driver/snowflake/driver.go

zeroshade

Just a few more comments, but this is looking good to me!

@lidavidm any comments?

go/adbc/driver/internal/driverbase/connection.go

go/adbc/driver/internal/driverbase/rotating_file_writer.go

lidavidm · 2025-05-06T23:03:58Z

go/adbc/ext.go

Missing docstrings?

lidavidm · 2025-05-06T23:09:16Z

go/adbc/driver/internal/driverbase/rotating_file_writer.go

Why is ADBC handling this? If it's for testing, isn't the standard to dump to OTLP and let the OTLP collector dump to stdout/file?

This handles the scenario

when the end-user is not authorized or doesn't want to install/deploy/configure an OTLP Collector

the caller cannot directly capture stdout - for example when a background process is started

@lidavidm - Think about the scenario like what exists for ODBC drivers. If a user has an issue with a product that consumes the driver, they will need to capture logs to send to the support team of said product. The file writer enables the capturing of the text-based files so the user can send those logs, in addition to whatever product logs may exist.

If we call this through the C# interop layer (like we do with Snowflake and Flight SQL today), then the only way to get the captured output is via the log files because there is no stdout capability.

This doesn't integrate with the existing logging mechanism, though...

In any case, I'd ask

(1) let's not dump stuff directly into $HOME; can we use something more proper/follow desktop standards?
(2) adbcfile is a bit of an odd name to me, but I guess we're stuck trying not to accidentally alias some (future) standard OTel sink somehow
(3) Ideally we could use the existing log config as the sink? Or unify them? We currently have an API to set an slog sink; it would be best if there was at least some plan to migrate/settle on a single log provider, since OTel has its own logging API. (Thankfully, the current logging API is also in ext.go so I'm more comfortable updating it.)

Also, is there any upstream/community decision or discussion on file exporters? It seems odd to me that this isn't provided at all.

I see this: https://opentelemetry.io/docs/specs/otel/protocol/file-exporter/

This doesn't integrate with the existing logging mechanism, though...

Previously, I was not able to successfully get log outputs when I call through the C# interop layer. I only tried with Snowflake. I set the ADBC_DRIVER_SNOWFLAKE_LOG_LEVEL environment variable but couldn't find if/where the log output was written.

@lidavidm / @davidhcoe

1. Migration from SLOG

Logging levels don't really apply to traces. Traces have attributes, statuses or other properties which could be filtered before export. But that would be complicated to configure. That is a good candidate for using OTel Collector.

Currently, the slog output (in go) is set to output to stderr. Which, as @davidhcoe notes, is difficult to redirect to a file in his scenario where new processes are being spawned in the background.

Maybe we could transition such that if ADBC_DRIVER_{driver-name}_LOG_LEVEL is set to one of [debug, info, warn, warning, error], that would enable OTel traces to be exported to stdout (or stderr). That is, be the equivalent of OTEL_TRACES_EXPORTER=console

2. Location of trace files in file system

I would just suggest that the location be portably available to the executing user.

It ensures the user has the permission to write the files.

It reduces a possible need for configuration option if the default is reasonable.

It is more likely that it doesn't inadvertently expose the files to other users in the system.

Your suggestion of os.UserConfigDir() is a good choice.

3. Choice of adbcfile for OTEL_TRACES_EXPORTER

This seems to be the the most obvious choice given the existing values for this configuration. It avoids the possible name clash.

From my exposure to the OTel SDK console exporters in different languages (go and C#) - they have different schema format. That is the main dilemma of why an existing file exporter has not be defined and published, yet. From my opinion, I don't think the schema matters for this use case - debug. If it was going to be consumed by some automation (i.e., OTel Collector), then schema would be important.

They (OTel) seem to be stuck on the schema format. But I think also they have the requirement that the contents could be used for an automated pipeline. So schema is important in this scenario.

OTel conceives of logs and traces as part of the same thing, which is why I ask (and OTel has its own logging standard) - so ideally we would have some plan to unify logging, if not right now

go/adbc/driver/internal/driverbase/driver.go

go/adbc/driver/internal/driverbase/connection.go

go/adbc/adbc.go

go/adbc/driver/internal/driverbase/statement.go

lidavidm · 2025-05-06T23:47:31Z

go/adbc/driver/snowflake/statement.go


+	defer func() {
+		if !st.SetErrorOnSpan(span, err) {
+			span.SetAttributes(attribute.Int64("db.response.returned_rows", numRows))


Is -1 ending up here potentially problematic for downstream consumers?

To be clear: does OTel accept -1 for this attribute? Since ADBC specifies -1 as the return value for when the number of rows is unknown.

Also do we need a utility for this given this pattern seems to recur?

@lidavidm

Negative numbers are not a problem.

lidavidm · 2025-05-13T00:39:10Z

go/adbc/driver/snowflake/statement.go


+	defer func() {
+		if !st.SetErrorOnSpan(span, err) {
+			span.SetAttributes(attribute.Int64("db.response.returned_rows", numRows))


To be clear: does OTel accept -1 for this attribute? Since ADBC specifies -1 as the return value for when the number of rows is unknown.

lidavidm · 2025-05-13T00:43:42Z

go/adbc/driver/snowflake/statement.go


+	defer func() {
+		if !st.SetErrorOnSpan(span, err) {
+			span.SetAttributes(attribute.Int64("db.response.returned_rows", numRows))


Also do we need a utility for this given this pattern seems to recur?

birschick-bq · 2025-05-13T02:35:09Z

@lidavidm

Also do we need a utility for this given this pattern seems to recur?

Yes. I agree.

In general, I'd say that I'm not really satisfied with the transparency of this instrumentation example. I was going to look at some other possible solutions like evaluating the original source as a lambda expressions.

This was only intended to give an example of instrumentation. We're going to need to provide a more complete instrumentation for each of the drivers. I'd suggest we look at improving the ease and reusability when we tackle instrumentation.

lidavidm · 2025-05-13T03:35:23Z

@zeroshade any more comments?

I still think we need to unify log/trace config at some point, but we don't have to do that here

zeroshade

LGTM now. I agree that we can address unifying logs and otel as a follow up

go/adbc/adbc.go

lidavidm

Thanks!

lidavidm · 2025-05-14T01:39:27Z

I filed #2823 and #2824.

birschick-bq added 7 commits April 21, 2025 21:26

feat(go/adbc): prototype OpenTelemetry rotating trace file exporter

16044cc

Merge branch 'main' into dev/birschick-bq/go-otel-tracing

c585e89

Merge branch 'main' into dev/birschick-bq/go-otel-tracing

f16349a

corrected lint issue

9e552d0

corrected lint issues

9bc88dd

improved instantiation

27a6f12

Handle exporter environment variable. Implement in base classes.

972f7a6

birschick-bq changed the title ~~feat(go/adbc): prototype OpenTelemetry rotating trace file exporter~~ feat(go/adbc): prototype OpenTelemetry trace file exporter in go driver Apr 28, 2025

birschick-bq added 14 commits April 28, 2025 13:47

Correct lint issues.

b282bb0

More lint issues

ec140ee

Remove rotating file writer

766b191

Small formatting improvements

dfa67f2

Refactor InitTracer to database, instead of connection

706924c

Created driverbase.statement for implementing OTelTracing as a base c…

fe248aa

…lass

Simplify connection

b9c9300

Increased instrumentation example

317d056

Corrected lint issue

a274a21

Merge branch 'main' into dev/birschick-bq/go-otel-tracing

2539a33

Attempt to correct 'bad file descriptor' test error

ef3e25f

Correctly implemented file re-use

f2ba0a1

Fixed file size test

7c6fb51

Revert inadvertant change on c#

1d79b0c

birschick-bq commented Apr 30, 2025

View reviewed changes

go/adbc/driver/snowflake/connection.go Show resolved Hide resolved

birschick-bq marked this pull request as ready for review April 30, 2025 22:19

birschick-bq requested a review from zeroshade as a code owner April 30, 2025 22:19

github-actions bot added this to the ADBC Libraries 18 milestone Apr 30, 2025

zeroshade requested changes May 1, 2025

View reviewed changes

birschick-bq added 2 commits May 2, 2025 20:48

Updates for code review comments

8ac79df

Fix lint issues

06c9884

zeroshade reviewed May 5, 2025

View reviewed changes

go/adbc/driver/internal/driverbase/database.go Outdated Show resolved Hide resolved

zeroshade reviewed May 5, 2025

View reviewed changes

go/adbc/driver/internal/driverbase/rotating_file_writer.go Outdated Show resolved Hide resolved

refactor methods with context to revert breaking change

0da4874

zeroshade reviewed May 6, 2025

View reviewed changes

go/adbc/driver/snowflake/driver.go Outdated Show resolved Hide resolved

birschick-bq added 2 commits May 6, 2025 10:23

code review updates

038bd9b

code review updates

5b52eb3

birschick-bq requested a review from zeroshade May 6, 2025 17:34

zeroshade reviewed May 6, 2025

View reviewed changes

some more code review updates

4bdc7c8

lidavidm requested changes May 6, 2025

View reviewed changes

birschick-bq added 3 commits May 7, 2025 17:21

unit test for GetSetTraceParent

762fb21

code review changes

07ae106

remove unit test for validating trace parent

6a11233

lidavidm approved these changes May 13, 2025

View reviewed changes

birschick-bq added 2 commits May 12, 2025 19:47

Merge branch 'main' into dev/birschick-bq/go-otel-tracing

d6745ae

Merge branch 'main' into dev/birschick-bq/go-otel-tracing

b4cf69a

zeroshade self-requested a review May 13, 2025 16:05

zeroshade approved these changes May 13, 2025

View reviewed changes

Merge branch 'main' into dev/birschick-bq/go-otel-tracing

5a72e2d

lidavidm reviewed May 13, 2025

View reviewed changes

go/adbc/adbc.go Show resolved Hide resolved

Mark telemetry options with EXPERIMENTAL in go docs comments

705a3bf

lidavidm approved these changes May 14, 2025

View reviewed changes

lidavidm merged commit d61c1b4 into apache:main May 14, 2025
41 checks passed

This was referenced May 14, 2025

format: standardize option values for OpenTelemetry integration #2823

Open

go/adbc: unify logging config with OTel config #2824

Open

birschick-bq deleted the dev/birschick-bq/go-otel-tracing branch June 11, 2025 17:30

serramatutu mentioned this pull request Aug 12, 2025

Sync upstream dbt-labs/arrow-adbc#52

Closed

feat(go/adbc): prototype OpenTelemetry trace file exporter in go driver #2729

feat(go/adbc): prototype OpenTelemetry trace file exporter in go driver #2729

Uh oh!

Conversation

birschick-bq commented Apr 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

birschick-bq commented Apr 30, 2025

Uh oh!

zeroshade left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

zeroshade left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

birschick-bq May 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

1. Migration from SLOG

2. Location of trace files in file system

3. Choice of adbcfile for OTEL_TRACES_EXPORTER

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

birschick-bq May 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

birschick-bq commented Apr 22, 2025 •

edited

Loading

birschick-bq May 9, 2025 •

edited

Loading

3. Choice of `adbcfile` for OTEL_TRACES_EXPORTER

birschick-bq May 13, 2025 •

edited

Loading