Allow tracking of https requests to storage account as ApplicationInsights dependencies #13481
Closed
2 tasks done
Assignees
Labels
bug
This issue requires a change to an existing behavior in the product in order to be resolved.
Client
This issue points to a problem in the data-plane of the library.
customer-reported
Issues that are reported by GitHub users external to the Azure organization.
Monitor
Monitor, Monitor Ingestion, Monitor Query
Milestone
Comments
ghost
added
needs-triage
deyaaeldeen
added
Storage
ghost
added
the
needs-team-attention
|
@deyaaeldeen We need to first investigate this from a app insights perspectives and less from storage because whatever the underlying cause is would apply to other packages we support @richardpark-msft Can you take a crack at this first? |
ramya-rao-a
added
Monitor
|
I'm confused about the request here. I thought AppInsights was doing work to intercept requests made by any module, rather than every npm module that makes a request doing something special to allow AppInsights to work. |
|
/cc @hectorhdzg Hey Hector, do you have an idea of what could be going on here? |
|
@nulltoken Application Insights node.js SDK supports automatic tracking of Azure SDKs but you need to install @opentelemetry/tracing package to make it work, I will make sure to update documentation to reflect this. |
|
@nulltoken actually on second thought I realized that approach is when using internal logging for Azure SDKs, we may have an issue in the node.js SDK that is preventing this to work I'm currently investigating and will let you know about my findings |
@hectorhdzg Thanks a lot for this. |
ramya-rao-a
added
bug
|
@hectorhdzg - looks like that PR merged. When do the appinsights packages get published? |
|
applicationisnsights latest version must have a fix for this |
|
@hectorhdzg Thanks for that. However, my repro case seems to still not work even with upgraded dependencies. Help? Output: dependencies: yarn.lock: Click to unfold |
|
@nulltoken I'm using same dependencies versions and I'm getting several dependency calls with names like these |
|
@hectorhdzg Thanks for getting back to me.
Hmmm. The repro code doesn't filter much... I'm going to create a dedicated repository to hold the repro case in order to make it easier to troubleshoot. |
|
@hectorhdzg Here it is: https://github.com/nulltoken/azure-storage-blob-repro-case Any chance you could fetch it, fill it with a connection string, a container name, a blob path, run |
|
@nulltoken thanks for the repro case, I can see the issue is regarding OpenTelemetry API mismatch, @azure/storage-blob is referencing an older version of @opentelemetry/api causing the issue, please install api manually to workaround this issue for now. @richardpark-msft is currently working on updating OpenTelemetry dependencies across Azure SDKs so this issue will be automatically fixed when SDKs are updated |
|
@hectorhdzg Thanks a lot for the quick feedback. ❤️
I'll test that out tomorrow and will report back here with my results. |
diff --git a/package.json b/package.json
index 5011fd4..1e9c369 100644
--- a/package.json
+++ b/package.json
@@ -20,6 +20,7 @@
},
"dependencies": {
"@azure/storage-blob": "^12.4.1",
+ "@opentelemetry/api": "^0.16.0",
"@opentelemetry/tracing": "^0.16.0",
"applicationinsights": "^1.8.10"
}did the trick! |
6 tasks
|
@hectorhdzg I think this issue may now be closed. However, digging deeper, I've hit #13798 that might be related to tracing. |
|
Thanks @nulltoken @richardpark-msft, @hectorhdzg, lets follow up in #13798 |
|
I ran npm install @opentelemetry/api@latest but the tracing still does not work. Still disconnected components in application map and transaction details. Is there anything else I have to do or do I simply have to wait for a fix? |
nulltoken
added a commit
to nulltoken/azure-storage-blob-repro-case
that referenced
this issue
Apr 7, 2021
@hectorhdzg Hrmpf. It looks like upgrading dependencies to latest versions makes the "trick" regress. Applying this patch brings us back to the initial position where storage accounts related dependencies were not logged diff --git a/package.json b/package.json
index 1e9c369..ebd81fb 100644
--- a/package.json
+++ b/package.json
@@ -19,9 +19,9 @@
"repro": "yarn node -r ts-node/register --unhandled-rejections=strict ./src/repro.ts"
},
"dependencies": {
- "@azure/storage-blob": "^12.4.1",
- "@opentelemetry/api": "^0.16.0",
- "@opentelemetry/tracing": "^0.16.0",
+ "@azure/storage-blob": "^12.5.0",
+ "@opentelemetry/api": "^1.0.0-rc.0",
+ "@opentelemetry/tracing": "^0.18.2",
"applicationinsights": "^1.8.10"
}
}Did I mess up the upgrade? FWIW, I've upgraded the repro repository would you need it for troubleshooting at https://github.com/nulltoken/azure-storage-blob-repro-case
|
|
@nulltoken give a try using OpenTelemetry API latest before release candidate version (0.18.1), you can see here that API RC version is still not compatible with other packages, we are waiting for version 1.0.0 to be available in OpenTelemetry to update all packages we own and hopefully this issue will not happen anymore. |
|
@hectorhdzg 👍 Switching back @opentelemetry/api to 0.18.1 worked. Huge thanks! |
|
@nulltoken And you don't get the #13798 problem? What dependency versions are you using? |
@macbeth I haven't tried it yet since switching to 0.18.1. Will test and keep you updated. |
openapi-sdkautomation bot
pushed a commit
to AzureSDKAutomation/azure-sdk-for-js
that referenced
this issue
May 11, 2021
Network february release (Azure#14333) * Adds base for updating Microsoft.Network from version stable/2020-11-01 to version 2021-02-01 * Updates readme * Updates API version in new specs and examples * init (Azure#13496) Co-authored-by: matyang222 <[email protected]> * Swagger change for CustomIpPrefix. Adding four new attributes. (Azure#13456) * update swagger * fix apiversion * fix * add Co-authored-by: Weiheng Li <[email protected]> * typo: paramter in applicationGateway.json (Azure#13538) * VPN NAT for Virtual Network Gateway feature changes(networkFeb) (Azure#13481) * commit1 * commit2 * resolving comments * pythonMd Co-authored-by: Khushboo Baheti <[email protected]> * fix virtual network resource (Azure#13570) * Added a new feature FlowTimeoutInMinutes under Virtual Network Proper… (Azure#13519) * Added a new feature FlowTimeoutInMinutes under Virtual Network Properties * Updated the type from string to integer, added a non-null example * Added missing format for 'integer' type * Add new failedMessage property for CustomIpPrefix (Azure#13607) * update swagger * fix apiversion * fix * add * add failedreason property * update swagger * fix apiversion * fix * add failedreason property * update Co-authored-by: Weiheng Li <[email protected]> * Added Preferred Routing Gateway Support (Azure#13611) * Feature: Address space update in peered vNets (Azure#13521) * Adding new fields and operation to support the address space update in peered vNets Adding new fields and operation to support the address space update in peered vNets * Adding the new query param in the example As per the review comment, adding the new query param in the example request response of swagger. * Adding the new query param in examples Adding the new query param in examples * Restricting the sync param Restricting the sync param to hold only true as value. We never need to send false. Co-authored-by: Hari Prasad Perabattula <[email protected]> * Remove max file size limit enforcement as it is done in NRP (Azure#13679) * Tesha/fix waf policy examples crs version (Azure#13697) * Remove max file size limit enforcement as it is done in NRP * Update the CRS version in the examples to reflect latest * Fix (Azure#13734) Co-authored-by: Khushboo Baheti <[email protected]> * Swagger for NRP's VipSwap operation (Azure#13639) * Swagger for NRP's VipSwap operation * Fixing validation errors * minor fix * Adding api version * Remove required (Azure#13969) Co-authored-by: Will Ehrich <[email protected]> * Hotfix extended location parameter hierarchy (Azure#13864) * add to feb branch * delete project name reference * expose two new client cert properties: validatedCertData, clientCertIssuerDN (Azure#13989) * adding workloadType property for Baremetal scenarios (Azure#14101) * Added bastion sku (Azure#14248) * fix nrp resources based on s360 checks (Azure#14219) * Adding Azure Network Manager association to the EffectiveNetworkSecurityGroups API (Azure#14265) * Added Azure Network Manager association to the EffectiveNetworkSecurityGroupAssociation * Adding example for networkManager response in the EffectiveNSG call * Add deleteOption to PublicIPAddress (Azure#14343) * Add deleteOption to PublicIPAddress * run validators Co-authored-by: Bashar Gharaibeh <[email protected]> Co-authored-by: Matthew Yang <[email protected]> Co-authored-by: matyang222 <[email protected]> Co-authored-by: Tom Li <[email protected]> Co-authored-by: Weiheng Li <[email protected]> Co-authored-by: Nick Schonning <[email protected]> Co-authored-by: Khushboo Baheti <[email protected]> Co-authored-by: Khushboo Baheti <[email protected]> Co-authored-by: guptas14 <[email protected]> Co-authored-by: Satya-anshu <[email protected]> Co-authored-by: arvenka <[email protected]> Co-authored-by: Hari Prasad Perabattula <[email protected]> Co-authored-by: Hari Prasad Perabattula <[email protected]> Co-authored-by: tejasshah7 <[email protected]> Co-authored-by: shnaya434 <[email protected]> Co-authored-by: William Ehrich <[email protected]> Co-authored-by: Will Ehrich <[email protected]> Co-authored-by: litchiyangMSFT <[email protected]> Co-authored-by: biaogao <[email protected]> Co-authored-by: bhbhise <[email protected]> Co-authored-by: mscorp-buchen <[email protected]> Co-authored-by: Arpit Agarwal <[email protected]> Co-authored-by: basharg <[email protected]> Co-authored-by: Bashar Gharaibeh <[email protected]>
6 tasks
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Describe the bug
It seems that leveraging
BlobClientdoesn't (by default) allow interception of underlying https calls through the https://github.com/microsoft/applicationInsights-node.js library.To Reproduce
Steps to reproduce the behavior:
Following piece of code demonstrates the issue. It makes 3 external https calls and logs to the console the intercepted dependencies
When run, this code generates the following output in the console.
As one can see, direct calls through
node-fetchare intercepted, and although the blob is properly downloaded (and its bytes length displayed in the console), it's not intercepted by ApplicationInsights library.Expected behavior
Monitoring of dependencies is of great importance in production, in order to ensure the overall ecosystem is working as expected. We are currently relying on AWS S3 for storage and are transitioning to Azure storage accounts. Interception of calls made with the aws sdk work perfectly.
Not being to leverage Microsoft ApplicationInsights node library when working with Microsoft Azure sdk library is a bit of a showstopper from an ops standpoint for us.
It's possible that customizing the instantiation of the
BlobClientmay help, however going through the options, I've been unable to see one that would help me.Any help helping us see those dependencies being tracked would be greatly appreciated.
The text was updated successfully, but these errors were encountered: