[BUG] Unable to include telemetry correlation headers

[tcihak-fqa]

Description
We using this SDK in an SPA using fetch.
The requests to the my .NET Web APIs do not include the telemetry correlation headers ('Request-Id' and 'Request-Context').
Dependency telemetry is being collected just fine.

Request Headers
  Request URL: https://api.domain.com:8082/resource
  Request Method: GET
  Status Code: 200 
  Referrer Policy: no-referrer-when-downgrade
  :authority: api.domain.com:8082
  :method: GET
  :path: /resource
  :scheme: https
  accept: application/json
  accept-encoding: gzip, deflate, br
  accept-language: en-US,en;q=0.9,es;q=0.8,uk;q=0.7
  content-type: application/json
  origin: https://client.domain.com
  referer: https://client.domain.com
  sec-fetch-dest: empty
  sec-fetch-mode: cors
  sec-fetch-site: cross-site
  user-agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) 
  Chrome/80.0.3987.149 Safari/537.36

Response headers
  access-control-allow-credentials: true
  access-control-allow-origin: https://client.domain.com
  access-control-expose-headers: Request-Context
  content-encoding: gzip
  content-length: 361
  content-type: application/json; charset=utf-8
  date: Sat, 28 Mar 2020 17:18:46 GMT
  request-context: appId=cid-v1:<some id>
  server: Microsoft-IIS/10.0
  status: 200
  x-powered-by: ASP.NET

• OS/Browser:
  Windows / Chrome (Version 80.0.3987.149)

• SDK Version [e.g. 22]:
  Version 2.5.2

• How you initialized the SDK:
  NPM setup / webpack

import { ApplicationInsights } from '@microsoft/applicationinsights-web';

export class Logger {
  _ai;

  constructor() {
    this._init();
  }

  _init() {
    var options = {
     config: {
       instrumentationKey: 'my key',
       appId: 'my-app',
       autoTrackPageVisitTime: true,
       disableAjaxTracking: true,
       disableFetchTracking: false,
       enableCorsCorrelation: true,
       enableRequestHeaderTracking: true,
       enableDebug: true,
       loggingLevelConsole: 2,
    }
    this._ai = new ApplicationInsights(options);
    this._ai.loadAppInsights();
  }
  // Logging methods excluded
}

[MSNev]

By Default these headers will only be include if the current window hostname is the same as the API endpoint.

You can include an additional config entry 'correlationHeaderDomains' which lists the domains that will have these headers added.

e.g.
const config = { enableCorsCorrelation: true, correlationHeaderDomains: ["azure.com", "prefix.bing.com"] } as ICorrelationConfig

[tcihak-fqa]

I added following to my existing config and the headers were still not added.

correlationHeaderDomains: ['api.domain.com', 'client.domain.com', 'domain.com']

[josundt]

I am experiencing the same as @tcihak-fqa, and I have a similar setup.

After upgrading @microsoft/applicationinsights-web to v2.5.3 (from v2.4.4), correlation headers are missing in all fetch requests.

I tested to enable the new W3C distributed tracing mode

{
    // ...
    distributedTracingMode: DistributedTracingModes.W3C
    // ...
}

...still no correlation headers of any sort.
No other distributedTracingMode options seem to work either.

This must be a bug, it worked before upgrading.

[MSNev]

Looking at the logic in canIncludeCorrelationHeader() the host matching includes the port number, so you'll need to include the port number if the correlationHeaderDomains if you are using one.

UrlHelper.parseUrl("https://api.domain.com:8082/resource").host
"api.domain.com:8082"

[MSNev]

@josundt Let me trace into this and see if I broke something that the tests didn't cover.

[MSNev]

@josundt What is your environment? And are you using a polyfill for fetch?

There was a bug that I fixed which was causing duplicated requests when using a polyfill (default for react-native) fetch and you enabled metrics for both fetch and xhr requests. To fix this case if it detects that fetch is a polyfill it no longer reports fetch requests and only reports the XHR (previously the XHR was causing an internal exception which dropped the event)

[josundt]

@MSNev :
I'm running an ASP.NET core app with kestrel on https://localhost:5001.

No fetch polyfill is used (polyfill for fetch is dynamically loaded only if the browser does not have it natively - with the browser I use for testing the native fetch API is supported).

I use JS modules (traspiled from TypeScript) and import @microsoft/applicationinsights-web - it is included in webpack bundling from node_modules.

The HTML/CSS/JS code is hosted in the same web application; on the same domain/port as the API I'm calling with fetch (where the correlation headers are missing). No CORS should be required.

FYI: It made no difference to add "localhost:5001" to correlationHeaderDomains,

I had a few more configuration settings than in my code below, but I have tested stripping the configuration down to a minimum as follows and I still have the problem:

import { ApplicationInsights, DistributedTracingModes } from "@microsoft/applicationinsights-web";

// ...
            const ai = new ApplicationInsights({
                config: {
                    //correlationHeaderDomains: ["localhost:5001"], // Made no difference
                    instrumentationKey: "the-instrumentation-key",
                    disableAjaxTracking: false,
                    disableCorrelationHeaders: false,
                    disableFetchTracking: false,
                    distributedTracingMode: DistributedTracingModes.AI, // .AI_AND_W3C or .W3C also does not add any correlation headers
                }
            });

            ai.loadAppInsights();

[MSNev]

@josundt I think I found the root cause for your issue, basically the code is adding the headers to the intercepted fetch request however if you don't pass the optional 2nd parameter (init), it creates one internally and populated the headers on this object... BUT, it doesn't pass this onto the real fetch!!!

As a workaround, if you pass a value as part of this 2nd parameter the headers should be sent correctly.

I'm working on a fix for this now.

[MSNev]

@josundt I've created a new issue for this as I don't think your issue is related to the original one mentioned here as XHR requests don't have the same issue
[BUG] Telemetry correlation headers are not included for all fetch requests #1240

[MSNev]

Actually this is probably the same issue as I missed the "with fetch" at the top of this issue.
So this should also be fixed with PR #1241

[josundt]

@MSNev
I have tested now and found that you were right about your findings.

But I also found that the fetch "interception" provided by applicationinsights-js (where you append the correlation headers) has flaws, since it only happens when calling fetch with a string (the URL) as first argument. It does not happen when using a Request object.

The fetch function in the Fetch API has different oveloads:

// The actual signature:
fetch(info: string | Request, init?: RequestInit): Promise<Response>;

// The same, written as overloads:
fetch(url: string): Promise<Response>;
fetch(url: string, init: RequestInit): Promise<Response>;
fetch(request: Request): Promise<Response>;
fetch(request: Request, init: RequestInit): Promise<Response>;

The Request constructor has this signature:

new (url: string, init?: RequestInit);

And the headers property in RequestInit can be undefined, a <string, string> object literal or a Headers object.

interface RequestInit {
    //...
    headers: undefined | Record<string, string> | Headers;
    //...
}

All these variations need to be covered by your fetch "interception".

You seem to intercept the fetch method correctly to append correlation header only when fetch is called with the URL (string) as the first argument, but it does not handle when sending a Request instance.

Please review my findings (see tests below).
All these fetch calls should end up with correlation headers, but it seems only some of them work:

        const url = "/api/values";

        // 1. Not working:
        await fetch(url);

        // 2. Works:
        await fetch(url, {});

        // 3. Works:
        await fetch(url, {
            headers: {}
        });

        // 4. Works:
        await fetch(url, {
            headers: new Headers()
        });

        // 5. Does not work:
        await fetch(new Request(url));

        // 6. Does not work:
        await fetch(new Request(url, {}));

        // 7. Does not work:
        await fetch(new Request(url, {
            headers: {}
        }));

        // 8. Does not work:
        await fetch(new Request(url, {
            headers: new Headers()
        }));

Please update the code and test suite to cover all these scenarios.

[MSNev]

REALLY nice followup and yet more tests that we don't have -- But thats mostly because until I refactored the interception our Phantom (build) test infrastructure didn't have any test coverage for fetch as it's not supported by Phantom.

I'll upgrade my PR #1240 to include additional tests and make sure the Request() argument works as it looks like there is a few other cases (tracking request headers) may not work as well for some of these

[josundt]

Great. Looking forward to see telemetry with request browser/server correlation in our application suite after the next release!

[tcihak-fqa]

Thanks @josundt for all your troubleshooting on this issue!
It will be nice to have the test coverage so it doesn't regress in the future.

[MSNev]

Fixed deployed as NPM and next CDN channels

[MSNev]

Now also fully deployed to primary CDN

[github-actions]

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.