Skip to content

Conversation

@birschick-bq
Copy link
Contributor

@birschick-bq birschick-bq commented May 14, 2025

Includes some improvements to ease instrumentation
Instrumentation for the Snowflake driver.

Example

func (base *ImplementsAdbcOTelTracing) MyFunction(ctx context.Context) (nRows int64, err error) {
    nRows = -1

    ctx, span := utils.StartSpan(ctx, "MyFunction", st)
    defer func() {
        // Optionally set attributes before exiting the function
        span.SetAttributes(semconv.DBResponseReturnedRowsKey.Int64(nRows))
        // MUST call this to ensure the span completes
        utils.EndSpan(span, err)
    }()
}

@birschick-bq
Copy link
Contributor Author

@davidhcoe
An initial template for instrumentation.

@birschick-bq birschick-bq marked this pull request as ready for review May 29, 2025 01:37
@birschick-bq birschick-bq requested a review from zeroshade as a code owner May 29, 2025 01:37
@github-actions github-actions bot added this to the ADBC Libraries 19 milestone May 29, 2025
@birschick-bq birschick-bq changed the title feat(go/adbc/driver): initial tracing instrumentation for Go driver feat(go/adbc/driver): initial tracing instrumentation for Snowflake driver May 29, 2025
@birschick-bq birschick-bq marked this pull request as draft May 29, 2025 21:16
@birschick-bq birschick-bq marked this pull request as ready for review May 30, 2025 00:34
@birschick-bq
Copy link
Contributor Author

@zeroshade - Matt

Just made a couple of small changes to ensure the error is set correctly before returning from functions, so that tracing has the correct error instance.

@lidavidm
Copy link
Member

lidavidm commented Jun 2, 2025

Note that snowflakedb/gosnowflake@d092186 (which is in 1.14.1 that I just merged) adds OTel to the Snowflake SDK, if we can confirm this works with that

@birschick-bq
Copy link
Contributor Author

@lidavidm

Note that snowflakedb/gosnowflake@d092186 (which is in 1.14.1 that I just merged) adds OTel to the Snowflake SDK, if we can confirm this works with that

Working on it. Thanks.

@birschick-bq
Copy link
Contributor Author

birschick-bq commented Jun 3, 2025

@lidavidm

Note that snowflakedb/gosnowflake@d092186 (which is in 1.14.1 that I just merged) adds OTel to the Snowflake SDK, if we can confirm this works with that

Yes. I confirmed it is working as expected

image

@birschick-bq birschick-bq marked this pull request as draft June 4, 2025 00:08
@birschick-bq birschick-bq marked this pull request as ready for review June 4, 2025 21:22
Copy link
Member

@lidavidm lidavidm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@zeroshade any comments?

Copy link
Member

@zeroshade zeroshade left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a small couple of nitpicks, but otherwise this is looking great to me!

@birschick-bq
Copy link
Contributor Author

@zeroshade Matt - addressed the two PR comments. Let me know if there is anything else to address. Thanks!

@davidhcoe
Copy link
Contributor

@birschick-bq - can you resolve the conflicts then get this merged ASAP?

@birschick-bq
Copy link
Contributor Author

@birschick-bq - can you resolve the conflicts then get this merged ASAP?

@davidhcoe Done

@birschick-bq
Copy link
Contributor Author

@zeroshade - If good to go, can we get this merged? Thanks.

Copy link
Member

@lidavidm lidavidm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@lidavidm lidavidm merged commit 01ac10b into apache:main Jun 8, 2025
41 checks passed
@birschick-bq birschick-bq deleted the dev/birschick-bq/go-otel-instrumentation branch June 11, 2025 17:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants