feat(observability): trace Database methods #2119
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
product-auto-label
bot
added
size: xl
odeke-em
force-pushed
the
trace-Database
branch
2 times, most recently
from
87ebbf7 to
c6e430f
Compare
product-auto-label
bot
added
size: l
odeke-em
force-pushed
the
trace-Database
branch
3 times, most recently
from
65ca9f3 to
548d980
Compare
product-auto-label
bot
added
size: xl
odeke-em
force-pushed
the
trace-Database
branch
3 times, most recently
from
204b67a to
186ed39
Compare
|
@surbhigarg92 kindly please re-review! |
odeke-em
force-pushed
the
trace-Database
branch
from
1388dcc to
aa58945
Compare
|
Hello @surbhigarg92 kindly please take a look. Thank you! |
odeke-em
force-pushed
the
trace-Database
branch
4 times, most recently
from
4c1aa52 to
f4f4565
Compare
|
@surbhigarg92 @harshachinta kindly please help trigger the bot runs. Thank you. |
surbhigarg92
approved these changes
Sep 30, 2024
test/database.ts
Outdated
| // depending on the version of Node.js, the error might be either of: | ||
| // * Cannot read properties of null (reading 'proto') | ||
| // * Cannot read property 'proto' of null | ||
| (err as grpc.ServiceError).message.includes('Cannot read propert'); |
There was a problem hiding this comment.
instead of this do something like
const errorMessage = (err as grpc.ServiceError).message;
errorMessage.includes("Cannot read properties of null (reading 'proto')") ||
errorMessage.includes("Cannot read property 'proto' of null")
There was a problem hiding this comment.
Awesome and thank you @surbhigarg92!
| assert.strictEqual(runFn, fakeRunFn); | ||
| // Given that we've wrapped the transaction runner with observability | ||
| // tracing, directly comparing values runFn and fakeRunFn. | ||
| // assert.strictEqual(runFn, fakeRunFn); |
There was a problem hiding this comment.
is it possible to add a check on some properties of these functions, instead of removing this check ?
There was a problem hiding this comment.
Sadly not, unless we are perform a traversal by utility examination but I found that to be overkill and not worth it TBH
Adds trace spans for Database methods, as well as tests for methods: * getSession * getSnapshot * run * runStream * runTransaction tracing of other methods shall come in follow-up PRs. Updates googleapis#2079 Built from PR googleapis#2087 Updates googleapis#2114
odeke-em
force-pushed
the
trace-Database
branch
from
82bc857 to
eeb628b
Compare
|
@surbhigarg92 thank you very much for the reviews! I've addressed all feedback and gotten the PR ready for merge by squashing commits into one. Kindly help me re-run the bots. |
surbhigarg92
added
automerge
gcf-owl-bot
bot
removed
the
owlbot:run
yoshi-kokoro
removed
the
kokoro:run
gcf-merge-on-green
bot
removed
the
automerge
gcf-merge-on-green bot
pushed a commit
that referenced
this pull request
Oct 30, 2024
🤖 I have created a release *beep* *boop* --- ## [7.15.0](https://togithub.com/googleapis/nodejs-spanner/compare/v7.14.0...v7.15.0) (2024-10-30) ### Features * (observability, samples): add tracing end-to-end sample ([#2130](https://togithub.com/googleapis/nodejs-spanner/issues/2130)) ([66d99e8](https://togithub.com/googleapis/nodejs-spanner/commit/66d99e836cd2bfbb3b0f78980ec2b499f9e5e563)) * (observability) add spans for BatchTransaction and Table ([#2115](https://togithub.com/googleapis/nodejs-spanner/issues/2115)) ([d51aae9](https://togithub.com/googleapis/nodejs-spanner/commit/d51aae9c9c3c0e6319d81c2809573ae54675acf3)), closes [#2114](https://togithub.com/googleapis/nodejs-spanner/issues/2114) * (observability) Add support for OpenTelemetry traces and allow observability options to be passed. ([#2131](https://togithub.com/googleapis/nodejs-spanner/issues/2131)) ([5237e11](https://togithub.com/googleapis/nodejs-spanner/commit/5237e118befb4b7fe4aea76a80a91e822d7a22e4)), closes [#2079](https://togithub.com/googleapis/nodejs-spanner/issues/2079) * (observability) propagate database name for every span generated to aid in quick debugging ([#2155](https://togithub.com/googleapis/nodejs-spanner/issues/2155)) ([0342e74](https://togithub.com/googleapis/nodejs-spanner/commit/0342e74721a0684d8195a6299c3a634eefc2b522)) * (observability) trace Database.batchCreateSessions + SessionPool.createSessions ([#2145](https://togithub.com/googleapis/nodejs-spanner/issues/2145)) ([f489c94](https://togithub.com/googleapis/nodejs-spanner/commit/f489c9479fa5402f0c960cf896fd3be0e946f182)) * (observability): trace Database.runPartitionedUpdate ([#2176](https://togithub.com/googleapis/nodejs-spanner/issues/2176)) ([701e226](https://togithub.com/googleapis/nodejs-spanner/commit/701e22660d5ac9f0b3e940ad656b9ca6c479251d)), closes [#2079](https://togithub.com/googleapis/nodejs-spanner/issues/2079) * (observability): trace Database.runTransactionAsync ([#2167](https://togithub.com/googleapis/nodejs-spanner/issues/2167)) ([d0fe178](https://togithub.com/googleapis/nodejs-spanner/commit/d0fe178623c1c48245d11bcea97fcd340b6615af)), closes [#207](https://togithub.com/googleapis/nodejs-spanner/issues/207) * Allow multiple KMS keys to create CMEK database/backup ([#2099](https://togithub.com/googleapis/nodejs-spanner/issues/2099)) ([51bc8a7](https://togithub.com/googleapis/nodejs-spanner/commit/51bc8a7445ab8b3d2239493b69d9c271c1086dde)) * **observability:** Fix bugs found from product review + negative cases ([#2158](https://togithub.com/googleapis/nodejs-spanner/issues/2158)) ([cbc86fa](https://togithub.com/googleapis/nodejs-spanner/commit/cbc86fa80498af6bd745eebb9443612936e26d4e)) * **observability:** Trace Database methods ([#2119](https://togithub.com/googleapis/nodejs-spanner/issues/2119)) ([1f06871](https://togithub.com/googleapis/nodejs-spanner/commit/1f06871f7aca386756e8691013602b069697bb87)), closes [#2114](https://togithub.com/googleapis/nodejs-spanner/issues/2114) * **observability:** Trace Database.batchWriteAtLeastOnce ([#2157](https://togithub.com/googleapis/nodejs-spanner/issues/2157)) ([2a19ef1](https://togithub.com/googleapis/nodejs-spanner/commit/2a19ef1af4f6fd1b81d08afc15db76007859a0b9)), closes [#2079](https://togithub.com/googleapis/nodejs-spanner/issues/2079) * **observability:** Trace Transaction ([#2122](https://togithub.com/googleapis/nodejs-spanner/issues/2122)) ([a464bdb](https://togithub.com/googleapis/nodejs-spanner/commit/a464bdb5cbb7856b7a08dac3ff48132948b65792)), closes [#2114](https://togithub.com/googleapis/nodejs-spanner/issues/2114) ### Bug Fixes * Exact staleness timebound ([#2143](https://togithub.com/googleapis/nodejs-spanner/issues/2143)) ([f01516e](https://togithub.com/googleapis/nodejs-spanner/commit/f01516ec6ba44730622cfb050c52cd93f30bba7a)), closes [#2129](https://togithub.com/googleapis/nodejs-spanner/issues/2129) * GetMetadata for Session ([#2124](https://togithub.com/googleapis/nodejs-spanner/issues/2124)) ([2fd63ac](https://togithub.com/googleapis/nodejs-spanner/commit/2fd63acb87ce06a02d7fdfa78d836dbd7ad59a26)), closes [#2123](https://togithub.com/googleapis/nodejs-spanner/issues/2123) --- This PR was generated with [Release Please](https://togithub.com/googleapis/release-please). See [documentation](https://togithub.com/googleapis/release-please#release-please).
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Adds trace spans for Database methods, as well as tests
for methods:
tracing of other methods shall come in follow-up PRs.
Updates #2079
Built from PR #2087
Updates #2114