-
Notifications
You must be signed in to change notification settings - Fork 131
chore(x-goog-spanner-request-id): commit foundation #3643
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
Conversation
google-cloud-spanner/src/main/java/com/google/cloud/spanner/XGoogSpannerRequestId.java
Outdated
Show resolved
Hide resolved
google-cloud-spanner/src/main/java/com/google/cloud/spanner/XGoogSpannerRequestId.java
Show resolved
Hide resolved
google-cloud-spanner/src/main/java/com/google/cloud/spanner/XGoogSpannerRequestId.java
Outdated
Show resolved
Hide resolved
5183b3c
to
5f27364
Compare
@sakthivelmanii thanks for the first round of comments! I've addressed them, please take a look and also kindly help me run the bots. |
google-cloud-spanner/src/main/java/com/google/cloud/spanner/XGoogSpannerRequestId.java
Outdated
Show resolved
Hide resolved
google-cloud-spanner/src/main/java/com/google/cloud/spanner/XGoogSpannerRequestId.java
Outdated
Show resolved
Hide resolved
643f345
to
bb9fdcf
Compare
@odeke-em mark this PR as chore. feat creates a new minor version. This change really does nothing. In upcoming PRs where you are using this class, you can define it as feat. |
thank you @sakthivelmanii I didn't know that about automatically creating releases: today I learnt! |
google-cloud-spanner/src/main/java/com/google/cloud/spanner/XGoogSpannerRequestId.java
Show resolved
Hide resolved
google-cloud-spanner/src/main/java/com/google/cloud/spanner/XGoogSpannerRequestId.java
Outdated
Show resolved
Hide resolved
google-cloud-spanner/src/main/java/com/google/cloud/spanner/XGoogSpannerRequestId.java
Outdated
Show resolved
Hide resolved
google-cloud-spanner/src/main/java/com/google/cloud/spanner/XGoogSpannerRequestId.java
Outdated
Show resolved
Hide resolved
google-cloud-spanner/src/main/java/com/google/cloud/spanner/XGoogSpannerRequestId.java
Outdated
Show resolved
Hide resolved
google-cloud-spanner/src/main/java/com/google/cloud/spanner/XGoogSpannerRequestId.java
Outdated
Show resolved
Hide resolved
google-cloud-spanner/src/test/java/com/google/cloud/spanner/XGoogSpannerRequestIdTest.java
Outdated
Show resolved
Hide resolved
google-cloud-spanner/src/test/java/com/google/cloud/spanner/XGoogSpannerRequestIdTest.java
Outdated
Show resolved
Hide resolved
google-cloud-spanner/src/test/java/com/google/cloud/spanner/XGoogSpannerRequestIdTest.java
Outdated
Show resolved
Hide resolved
google-cloud-spanner/src/test/java/com/google/cloud/spanner/XGoogSpannerRequestIdTest.java
Outdated
Show resolved
Hide resolved
85ab665
to
8bd366d
Compare
@sakthivelmanii @olavloite thanks for the reviews, please take another round. |
ccda785
to
c084579
Compare
google-cloud-spanner/src/main/java/com/google/cloud/spanner/XGoogSpannerRequestId.java
Outdated
Show resolved
Hide resolved
google-cloud-spanner/src/main/java/com/google/cloud/spanner/XGoogSpannerRequestId.java
Outdated
Show resolved
Hide resolved
@RunWith(JUnit4.class) | ||
public class XGoogSpannerRequestIdTest { | ||
private static final Pattern REGEX_RAND_PROCESS_ID = | ||
Pattern.compile("1.([0-9a-z]{16})(\\.\\d+){3}\\.(\\d+)$"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The process ID should be a 32-bit unsigned number, so length 8 in hex format:
Pattern.compile("1.([0-9a-z]{16})(\\.\\d+){3}\\.(\\d+)$"); | |
Pattern.compile("1.([0-9a-z]{8})(\\.\\d+){3}\\.(\\d+)$"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@olavloite we had a long discussion with Simil and team in the same group that you are, about why it should be 64-bits due to higher collisions if it were 32-bits per https://orijtech.notion.site/x-goog-spanner-request-id-always-on-gRPC-header-to-aid-in-quick-debugging-of-errors-14aba6bc91348091a58fca7a505c9827#14aba6bc9134807f9f17d3ef58e26128
This change adds in the bases to bring in the functionality for the "x-goog-spanner-request-id" that'll be used for debugging without the limits of trace continuity and sampling. Updates googleapis#3537 Address code review comments
0a14334
to
331e06e
Compare
This change adds in the bases to bring in the functionality for the "x-goog-spanner-request-id" that'll be used for debugging without the limits of trace continuity and sampling.
Updates #3537