microsoft / ApplicationInsights-node.js

Microsoft Application Insights SDK for Node.js
MIT License
324 stars 141 forks source link

2.5.1+ doesn't initialize when used with azure-functions-core-tools #1144

Closed CreativeTechGuy closed 1 year ago

CreativeTechGuy commented 1 year ago

TLDR: Two different libraries, applicationinsights and azure-functions-core-tools both overwrite Node.js' require method, thus breaking the magic that azure-functions-core-tools is doing and preventing applicationinsights to hook into the local Azure Function server. This causes automatic logging when running Azure Functions locally to not work and getCorrelationContext() to return null.

This is an incredibly complicated issue. I feel it's best explained by tracing the code since there's a lot going on.

Now, the problem is that there's two packages which are loaded at runtime, both overwriting a global with a custom implementation. azure-functions-core-tools is loaded first so it overwrites first, then require-in-the-middle loads second and overwrites the overwrite. Thus when applicationinsights tries to check for the existence of @azure/functions-core at runtime, it cannot find it. As a result of all of this. our calls to appinsights.getCorrelationContext() return null as it was never initialized as the Azure Function hooks were never setup.

This issue was introduced in 2.5.1 and was not an issue in 2.5.0 of applicationinsights. It's unclear which package is the root cause, it seems like both are equally to blame as they aren't playing nice with each other. Both azure-functions-core-tools and require-in-the-middle "try" to overwrite in a way to be compatible, but due to the use of the Proxy in azure-functions-core-tools it is still incompatible if loaded first.

JacksonWeber commented 1 year ago

Hi @CreativeTechGuy, if you want to eliminate the conflict here you could disable Azure SDK auto-instrumentation. This won't fix the core issue that the two dependencies both monkey-patch the require method but might be able to resolve your issue for now. Let me know that workaround helps, thanks!

CreativeTechGuy commented 1 year ago

@JacksonWeber This doesn't change anything unfortunately. The issue is that the auto-instrumentation doesn't work. So manually disabling the thing that doesn't work doesn't really do anything.

ejizba commented 1 year ago

I took a look and it seems like we're just waiting for the fix to propogate through various dependencies.

  1. require-in-the-middle fixed this in v7.0.0
  2. @opentelemetry/instrumentation updated to require-in-the-middle v7.0.0 in their v0.39.0 release
  3. @azure/opentelemetry-instrumentation-azure-sdk is currently on @opentelemetry/instrumentation v0.38.0 and needs to update to v0.39.0
  4. applicationinsights will need to update to the latest @azure/opentelemetry-instrumentation-azure-sdk

@JacksonWeber @hectorhdzg if you could help speed along those last two updates of packages that would be much appreciated :)

CreativeTechGuy commented 1 year ago

Thank you @ejizba!

JacksonWeber commented 1 year ago

@ejizba Absolutely. Thank you for noticing the updates in the dependency web there!

JacksonWeber commented 1 year ago

@CreativeTechGuy @ejizba The above issue should be resolved as our dependency on azure/opentelemetry-instrumentation has been updated and that package's dependence on require-in-the-middle and all opentelemetry packages has been updated.

CreativeTechGuy commented 1 year ago

Is there a plan to release that fix? It looks like it's been committed but the package hasn't been released in a few weeks.

hectorhdzg commented 1 year ago

@CreativeTechGuy we have a monthly release cadence in our side, we will be publishing a new version of Application Insights Node.js SDK end of next week.