Request and dependencies reporting enchancements

[lmolkova]

[EDITED]

I have a simple service that calls google

'use strict';

const appInsights = require("applicationinsights"); //0.22.0
appInsights.setup("removed");
appInsights.start();

const express = require('express'); //4.15.4
const request = require('request'); //2.81.0

// Constants
const PORT = 8089;
const HOST = '0.0.0.0';

// App
const app = express();
app.get('/', (req, res) => {
    
  console.log('Starting request: GET google'); // Print the response status code   
  request("http://google.com", function (error, response, body) {
      console.log('Response from google statusCode: %s', response && response.statusCode);
    });

  res.send('Hello world\n');
});


app.listen(PORT, HOST);
console.log(`Running on http://${HOST}:${PORT}`);

Here is a request telemetry that is reported when I make a request

Here is dependencies that are tracked for this request:

1. [SOLVED] Dependencies are not correlated with request
2. (minor) Two dependencies are tracked (because of redirected request). Is it expected? Is it avoidable? It should be consistent between SDKs (.NET reports just one)
3. (minor) Request parent Id should be null - new id has just been generated for this operation. It should be consistent with .NET SDK.
4. (minor) what is the point of saving "undefined | roleName:undefined" in the 'source'?

[lmolkova]

@OsvaldoRosado @MSLaguana please have a look

[OsvaldoRosado]

Thanks for reporting this!

Are you still missing correlation if you use appInsights.wrapWithCorrelationContext around the call to the request library?

[OsvaldoRosado]

I suspect the double tracking has to do with how the request library internally works. We might be able to have a special patch in the SDK to handle that (perhaps with diagnostic-channel ?)

[lmolkova]

Are you still missing correlation if you use appInsights.wrapWithCorrelationContext around the call to the request library?

Perhaps I'm doing something wrong, but appInsights.getCorrelationContext() is null AND if I call wrapWithCorrelationContext, still dependencies are not correlated to request.
Please check.

app.get('/', (req, res) => {
    
  console.log('operation ', appInsights.getCorrelationContext()); // operation is null!
  console.log('Starting request: GET google'); // Print the response status code   
  var call = function () {request("http://google.com", function (error, response, body) {
      console.log('Response from google statusCode: %s', response && response.statusCode);
  })};
  var wrap = appInsights.wrapWithCorrelationContext(call);
  wrap();
   
  res.send('Hello world\n');
});

Also, does it mean that correlation is not automagical in some cases? What does it depend on?

[OsvaldoRosado]

Also, does it mean that correlation is not automagical in some cases? What does it depend on?

We can't always guarantee correlation in Node as its not supported at a language level. The constraints given by its single-threaded continuation-passing approach mean we need to "patch the world" and catch any methods that do go async, and replace them with functions that will preserve our context.

It is very strange that you get null from getCorrelationContext. I do not see this in my own express apps. That sounds like the reason why correlation is not working.

[OsvaldoRosado]

Just realized you do not import applicationinsights before other libraries. Can you please try changing the order of your imports?

[lmolkova]

Just realized you do not import applicationinsights before other libraries. Can you please try changing the order of your imports?

Indeed, that was the issue, everything works even without wrapWithCorrelationContext.
Let me change issue title and description a bit, so we don't forget about other minor things.
But I think those are 'enhancements' rather than 'bugs'

[lmolkova]

@OsvaldoRosado Thanks a lot!

[prannayb]

There is a bug in getCorrelationContext()
https://github.com/Microsoft/ApplicationInsights-node.js/blob/develop/applicationinsights.ts#L106

if (this._isCorrelating) { return CorrelationContextManager.CorrelationContextManager.getCurrentContext(); }
this._isCorrelating is always undefined since in 1.0.0 you've moved to private scope variables rather than properties on the instance (this). The code should be if(_isCorrelating) { ...

[OsvaldoRosado]

Great catch! Filed #348