feat: update StreamWriterV2 to support trace id #895
Conversation
…the connection and remove stream name and schema in the following ones.
…the connection and remove stream name and schema in the following ones
…va-bigquerystorage into schema-stream-name
yayi-google
requested review from
yirutang,
bigang-li,
a team and
stephaniewang526
product-auto-label
bot
added
the
api: bigquerystorage
google-cla
bot
added
the
cla: yes
Codecov Report
@@ Coverage Diff @@
## master #895 +/- ##
============================================
+ Coverage 80.57% 80.63% +0.06%
- Complexity 1015 1016 +1
============================================
Files 76 76
Lines 5533 5542 +9
Branches 427 429 +2
============================================
+ Hits 4458 4469 +11
+ Misses 897 896 -1
+ Partials 178 177 -1
Continue to review full report at Codecov.
|
...-bigquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1beta2/StreamWriterV2.java
Show resolved
Hide resolved
| /** TraceId for debuging purpose. */ | ||
| public Builder setTraceId(String traceId) { | ||
| this.traceId = traceId; | ||
| return this; |
There was a problem hiding this comment.
Let's perform some simple sanity check here.
On the server side, this needs to be something like key value pair like http header or A:B; is okay as well (we need to document it).
A simple processing at the client side will be splitting the trace id per the standard above if we can;
other wise convert it to UserTrace:xxx;
On top of it, adding the StreamWrite:Version?
There was a problem hiding this comment.
The traceId name is strictly screened in the backend. In order for dataflow to be recognized, it has to start from Dataflow:, so apart from this function, maybe also provide a setDataflowJobId to set it directly to Dataflow:job_id?
There was a problem hiding this comment.
All these requires backend support, which is not there yet.
I would also prefer to put all the logic you mentioned into the server side, because we have strong control of the codes there.
On the other hand, it is very hard to change the client library. So a free string seems most flexible.
There was a problem hiding this comment.
Making dataflow do the right thing is also fine to me. Just that if they mess up, then we will have issues in debugging their traffic. The backend already parses these trace ids.
There was a problem hiding this comment.
Yes, we need to perform such kind of logic in the server side, but here, let's perform some sanity check and makes sure that final trace id is in a good shape as well?
There was a problem hiding this comment.
Done. Added check to make sure traceId follow the format of A:B.
|
Can you take a look at this? I have seen this happen a few times now. I worry it's a flaky test. |
Taken care in a separate PR. |
|
Looks like there are merge conflicts -- PTAL. |
Done. Fixed the conflicts. |
🤖 I have created a release \*beep\* \*boop\* --- ## [1.14.0](https://www.github.com/googleapis/java-bigquerystorage/compare/v1.13.0...v1.14.0) (2021-03-04) ### Features * update StreamWriterV2 to support trace id ([#895](https://www.github.com/googleapis/java-bigquerystorage/issues/895)) ([2e49ce8](https://www.github.com/googleapis/java-bigquerystorage/commit/2e49ce8c79cb059840c3307898ba16980f6892fa)) ### Bug Fixes * add schema update back to json writer ([#905](https://www.github.com/googleapis/java-bigquerystorage/issues/905)) ([a2adbf8](https://www.github.com/googleapis/java-bigquerystorage/commit/a2adbf80753161cbddd23d5a7db75e9250db58fa)) * Add unit test for concurrent issues we worried about, and fix some locking issues ([#854](https://www.github.com/googleapis/java-bigquerystorage/issues/854)) ([0870797](https://www.github.com/googleapis/java-bigquerystorage/commit/087079728195e20f93701e8d5e1e59ba29a7d21b)) * test failure testAppendWhileShutdownSuccess ([#904](https://www.github.com/googleapis/java-bigquerystorage/issues/904)) ([b80183e](https://www.github.com/googleapis/java-bigquerystorage/commit/b80183ea23c8b78611a42d22d8c62a4ba4904a80)) * testAppendWhileShutdownSuccess race ([#907](https://www.github.com/googleapis/java-bigquerystorage/issues/907)) ([d39443d](https://www.github.com/googleapis/java-bigquerystorage/commit/d39443d51d2625e4b3aee59d1e593229e9e449d3)) ### Dependencies * update dependency com.google.cloud:google-cloud-bigquery to v1.127.6 ([#909](https://www.github.com/googleapis/java-bigquerystorage/issues/909)) ([505938b](https://www.github.com/googleapis/java-bigquerystorage/commit/505938bcba5a4a7af9e618572bbc41f365702f47)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:
Fixes #<issue_number_goes_here> ☕️