Skip to content
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.

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

fix(spanner): avoid desructive context augmentation that dropped all headers #11659

Conversation

odeke-em
Copy link
Contributor

This change fixes a destructive context operation that dropped all prior headers, metadata.NewOutgoingContext sadly doesn't document that it doesn't reuse the input conext negating the expectation in Go that building from a parent context brings along the prior keys and metadata. While here, this chhange adds a regression test to ensure that in the future, any dropped or lost metadata will be reported during development.

Fixes #11656

@odeke-em odeke-em requested review from a team as code owners February 25, 2025 21:27
@product-auto-label product-auto-label bot added the api: spanner Issues related to the Spanner API. label Feb 25, 2025
@odeke-em odeke-em force-pushed the spanner-request-id-avoid-destructive-context-recreation-if-request-id-is-sent branch 2 times, most recently from 86195d8 to 49061e5 Compare February 25, 2025 21:31
Copy link
Contributor

@olavloite olavloite left a comment

Choose a reason for hiding this comment

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

LGTM, with some nits on the test. We can pick that up in a follow-up PR as well if we want to ship this ASAP.

…headers

This change fixes a destructive context operation that dropped
all prior headers, metadata.NewOutgoingContext sadly doesn't
document that it doesn't reuse the input conext negating the
expectation in Go that building from a parent context brings
along the prior keys and metadata. While here, this chhange adds
a regression test to ensure that in the future, any dropped
or lost metadata will be reported during development.

Fixes googleapis#11656
@odeke-em odeke-em force-pushed the spanner-request-id-avoid-destructive-context-recreation-if-request-id-is-sent branch from 76ea4eb to d70b849 Compare February 26, 2025 09:08
@odeke-em
Copy link
Contributor Author

@olavloite kindly help me re-run the bots!

@rahul2393 rahul2393 added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Feb 26, 2025
@kokoro-team kokoro-team removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Feb 26, 2025
@olavloite olavloite merged commit 594732d into googleapis:main Feb 26, 2025
8 checks passed
@odeke-em odeke-em deleted the spanner-request-id-avoid-destructive-context-recreation-if-request-id-is-sent branch February 26, 2025 16:07
larryanz pushed a commit to larryanz/google-cloud-go that referenced this pull request Feb 27, 2025
…headers (googleapis#11659)

* fix(spanner): avoid desructive context augmentation that dropped all headers

This change fixes a destructive context operation that dropped
all prior headers, metadata.NewOutgoingContext sadly doesn't
document that it doesn't reuse the input conext negating the
expectation in Go that building from a parent context brings
along the prior keys and metadata. While here, this chhange adds
a regression test to ensure that in the future, any dropped
or lost metadata will be reported during development.

Fixes googleapis#11656

* Address review feedback+nits
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: spanner Issues related to the Spanner API.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Spanner: x-goog-spanner-request-id related changes have caused other headers not to be forwarded
4 participants