Skip to content

fix(go/adbc/driver/internal/driverbase): Ensure to propagate the traceParent from the driver/database to connection #2951

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
zeroshade merged 5 commits into from
Jun 11, 2025
Merged

fix(go/adbc/driver/internal/driverbase): Ensure to propagate the traceParent from the driver/database to connection #2951

zeroshade merged 5 commits into from
Jun 11, 2025

Conversation

@birschick-bq
Copy link
Contributor

@birschick-bq birschick-bq commented Jun 10, 2025

This fix ensures the traceParent option is propagated from the Driver.Open to Database.Connect.

@birschick-bq
fix(go/adbc/driver/internal/driverbase): Ensure to propagate the trac…
ad97c11 
…eParent from the driver/database to connection
@birschick-bq birschick-bq requested a review from zeroshade as a code owner June 10, 2025 17:23
@github-actions github-actions bot added this to the ADBC Libraries 19 milestone Jun 10, 2025
@birschick-bq
Copy link
Contributor Author

birschick-bq commented Jun 10, 2025

@zeroshade / @lidavidm / @CurtHagenlocher
A small fix to handle the case when the traceParent option is passed in the Driver.Open call.

zeroshade
zeroshade approved these changes Jun 10, 2025
View reviewed changes
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.

Sounds good. Is there any way we can add a test for this at all?

@birschick-bq
Copy link
Contributor Author

birschick-bq commented Jun 10, 2025

Sounds good. Is there any way we can add a test for this at all?

Sure. I'll add a test.

@birschick-bq
Copy link
Contributor Author

birschick-bq commented Jun 10, 2025

Sounds good. Is there any way we can add a test for this at all?

@zeroshade - I augmented an existing test TestNewDatabaseWithOptions to check that the database trace parent is propagated to the connection. Let me know if this is sufficient.

@zeroshade
Copy link
Member

zeroshade commented Jun 10, 2025

Looks good to me! Thanks!

@birschick-bq
Copy link
Contributor Author

birschick-bq commented Jun 10, 2025

@zeroshade - Any hints on how I can get the Python tests to run without errors?
On my own repository, I'm able to re-run each step individually and they all complete successfully when run, one at a time.

I don't seem to be able to re-run an individual step in this repository.

@lidavidm
Copy link
Member

lidavidm commented Jun 10, 2025

ah. Matt this is the flake I mentioned the other day that I think is somewhere in arrow-go :)

I kicked the jobs

@zeroshade
Copy link
Member

zeroshade commented Jun 10, 2025

I'll try to look into the flake next week unless you're able to track it down lol

@birschick-bq
Copy link
Contributor Author

birschick-bq commented Jun 11, 2025

@zeroshade / @lidavidm - Would you be able to merge this change when you have a chance? Thanks.

@zeroshade zeroshade merged commit 224df19 into apache:main Jun 11, 2025
81 of 89 checks passed
@birschick-bq birschick-bq deleted the dev/birschick-bq/golang-traceparent-propagation branch June 11, 2025 17:26
@serramatutu serramatutu mentioned this pull request Aug 12, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lidavidm lidavidm lidavidm approved these changes

@zeroshade zeroshade zeroshade approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

ADBC Libraries 19

Development

Successfully merging this pull request may close these issues.

3 participants

@birschick-bq @zeroshade @lidavidm