Azure / azure-sdk-for-js

This repository is for active development of the Azure SDK for JavaScript (NodeJS & Browser). For consumers of the SDK we recommend visiting our public developer docs at https://docs.microsoft.com/javascript/azure/ or our versioned developer docs at https://azure.github.io/azure-sdk-for-js.
MIT License
1.98k stars 1.15k forks source link

`@azure/monitor-opentelemetry` NodeJS Library Only Supports Node 18+; Docs Suggest Minimum Node Version is 14 #30217

Open epicstar opened 1 week ago

epicstar commented 1 week ago

Describe the bug

@azure/monitor-opentelemetry v1.6.0 seems to only run on NodeJS 18+, when the opentelemetry-js supports up to 14+.

The Get Started implies you support Node 14.

This is the error I get when running my project running on Node 14.7:

[ 33.125] [ERROR] [source internal/modules/cjs/loader.js:899]: Uncaught Error: Cannot find module 'node:os'

The readme in @azure/monitor-opentelemetry suggests you guys support the latest list of LTS versions, which the minimum is Node 16 (where the node:os module started). However, opentelemetry-js supports 14+ which you call out in your Get Started guide. So I was assuming that your lib supported Node 14+ also.

So it's either that you support the NodeJS version that OpenTelemetry minimally supports supports, or you openly state you support up to Node LTS versions (18 atm) in the Get Started guide.

To Reproduce Steps to reproduce the behavior:

  1. install node 14
  2. Read the prerequisites in the [Get Started Guide](Get Started
  3. create stub project following the steps in the Get Started guide, including starting the Azure monitor code.

Expected behavior Node App runs.

Screenshots image

Prerequisites from the page: opentelemetry-js

Additional context I wish I could update to the latest LTS version of NodeJS, but we're bound by what BrightSign installs on their players :(

jeremymeng commented 1 week ago

@epicstar thank you for reporting this issue! Unfortunately the latest versions of our Azure SDK packages only support NodeJS v18 and later. We encourage customers to upgrade to at least v18 as there are vulnerabitlity in NodeJS v16 that won't be fixed.

epicstar commented 1 week ago

Thank you. Here's another instance of confusing NodeJS version support in the documentation: https://learn.microsoft.com/en-us/azure/azure-monitor/app/opentelemetry-nodejs-migrate?tabs=cleaninstall#node--14-support

Node < 14 support

OpenTelemetry JavaScript's monitoring solutions officially support only Node version 14+. Check the OpenTelemetry supported runtimes for the latest updates. Users on older versions like Node 8, previously supported by the ApplicationInsights SDK, can still use OpenTelemetry solutions but can experience unexpected or breaking behavior.

FefoAljedani commented 1 week ago

``

hectorhdzg commented 1 week ago

@epicstar Azure Monitor OpenTelemetry package rely on both Azure SDK and OpenTelemetry dependencies, that is why we mention both in our official docs, right now OpenTelemetry supports older versions of the runtime comparing to Azure SDK, but that could change, maybe we can add more explicit message saying that the supported runtime is where both dependencies are satisfied. Migration docs you shared are outdated and I will make sure these are up to date, I would recommend you take a look at Application Insights SDK v2 for older Node.js runtime support.

epicstar commented 1 week ago

I would recommend you take a look at Application Insights SDK v2 for older Node.js runtime support.

I've tried 2.9.5 and got the same node:os import error. I'll go down the list of minor versions and will let you know which one works on Node 14. Thank you!

epicstar commented 3 days ago

I couldn't find a version of application insights v2 that supported Node 14. I went down to 2.4.2 with the same node:os import error. I think what's happening is that yarn is updating a transitive dependency of applicationinsights v2 to something that doesn't support Node 14. I even tried deleting the yarn lock. I'm not exactly sure which transitive dependency that is causing the issue. Regardless, because it's beyond the scope of @azure/monitor-opentelemetry, I don't expect a solution on this other than doc changes and will just create an issue in that repo.

jeremymeng commented 3 days ago

would pinning transitive dependencies helps? Not that we want customers to continute working on unsecured version of Nodejs, but something like

  "resolutions": {
    "@azure/core-rest-pipeline": "1.14.0",
    "@azure/core-util": "1.7.0",
    "@azure/logger": "1.0.4"
  }
hectorhdzg commented 3 days ago

@epicstar like @jeremymeng mentioned pinning the dependencies may help, @azure/core-auth is also a dependency in Application Insights v2 that is causing the issue, I will update Application Insights to support older versions of Node.js as you noticed is a bug in our side, at least in Application insights v2 side of things, automatic dependency updates broke this most likely, but you will not be able to use AAD feature in the SDK, if you need the feature you will need to update to newer runtimes, I will make sure this is reflected in docs as well.