nodejs
nodejs copied to clipboard
feat: added azure blob instrumentation
This feature focuses on instrumenting the classes blobClient and blockBlobClient in azure blob storage: https://instana.kanbanize.com/ctrl_board/56/cards/146704/details/
Methods instrumented for blob:
- Upload
- Download
- Delete
NOTE: Instrumenting BlockBlobClient and BlobClient give easy parsing for proper data required for span creation such as blobname, container name and operation. If using sendOperation, we need to parse it from the url and it doesn't seem neat. Also sendOperation is from @azure/core-http , and even though it is used in almost every operations in storage package, it doesn't act as common point of instrumentation when it comes to a major package like cosmosDB
Things to cover
- Page Blob class
- Container Class for instrumenting container functionalities
TODO: Test azure tracing SDK with our otel integration
Good Job 🍾
Please rebase against main and replace the usage of ProcessControls
. The syntax has changed.
See example: https://github.com/instana/nodejs/commit/d1b38951f8d8111bdc2cce81aa1a361df4d1fc77#diff-92983afeee93cf42a78a3dd57e5a6e3e96662d66c7f7c6954548e0723322c601L34
Hey!
I need to do another review iteration with fresh eyes. Please take a look at my first comments 👍
Here are some additional findings:
- ESM app is missing. Please add an ESM app.mjs and check if azure blob storage works with ESM - see.
- I could not find the ticket reference in the PR nor in the commits.
- All of your files did not run through code formatting - please do.
Reply to this: Addressed the Issues
Thank you @aryamohanan and me will do another review 👍
The plan is to review and merge this on Monday or better said: after the next release.
Pushed a commit to run all tests using [ci all-tests]
. Waiting for success result.
Passed 👍