Skip to content

Conversation

@olavloite
Copy link
Collaborator

Set the user-agent header to go-gorm-spanner if it is Spanner gorm that is creating the connection.

Set the user-agent header to go-gorm-spanner if it is Spanner gorm that is
creating the connection.
@olavloite olavloite requested a review from a team as a code owner March 13, 2025 18:27
driver.go Outdated
// Check if it is Spanner gorm that is creating the connection.
// If so, we should set a different user-agent header than the
// default go-sql-spanner header.
callers := make([]uintptr, 20)

Choose a reason for hiding this comment

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

nit: what's the idea behind choosing 20 frames in the stack?

Choose a reason for hiding this comment

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

nit: can we separate the logic identifying if the request is coming from gorm or not into a new function?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

  1. Go requires us to set a (max) number of stack items. 20 is relatively randomly chosen, but should be large enough to account for the internal functions that are called in the Go sql package between the time that the call 'leaves' gorm and enters the Spanner driver. Setting it at the exact level that would be needed today could be dangerous, as both the gorm and sql packages could add more internal function calls that become part of the stack that we see here in the future.
  2. I've moved the logic for checking whether the connection comes from gorm into a separate method.

@olavloite olavloite merged commit 1c6381a into main Mar 18, 2025
18 checks passed
@olavloite olavloite deleted the set-gorm-header branch March 18, 2025 15:23
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.

3 participants