microsoft / ApplicationInsights-JS

Microsoft Application Insights SDK for JavaScript
MIT License
650 stars 240 forks source link

[BUG] Polyfill created by fetch tracking interferes with maxmind device tracking #2422

Closed onionhammer closed 3 weeks ago

onionhammer commented 1 month ago

When app insights automatic fetch tracking is enabled, maxmind's device tracking no longer works. Is there a way to manually trace fetch dependencies back to the server? Can I manually set the 'traceparent' on the fetch request? If so where do I get the 'traceparent' from?

Steps to Reproduce

Expected behavior app insights SDK should not interfere with device fingerprinting libraries

Additional context Error produced, but only if 'disableFetchTracking' is not true: "has been blocked by CORS policy: Response to preflight request doesn't pass access control check: No 'Access-Control-Allow-Origin' header is present on the requested resource. If an opaque response serves your needs, set the request's mode to 'no-cors' to fetch the resource with CORS disabled."

MSNev commented 1 month ago

Ok, so it sounds like this maxminds (device fingerprinting) is adding some additional header to validate that the content of the full request has not changed (since sending).

Assuming this is the case the issue is (most likely) that we are adding either the traceparent and/or Request-Id header to the request in response to the distributedTracingMode, we don't have an explicit "None" option for this, however, looking at the code it is currently an "explicit" check for matching the 3 supported values (so in theory a hack to this (assuming we don't explicit reset it somewhere else), would be to set this value to something invalid (like -1 or 99, 1000, etc), and this (should) stop the header being set but still report that the "dependency" occurred.

However, to answer

Can I manually set the 'traceparent' on the fetch request

Yes, you can set the header to anything you like and we will not override it. As long as you keep the same traceid and create your own spanid for the request and set the header directly it should work.

Response to preflight request doesn't pass access control check: No 'Access-Control-Allow-Origin' header is present on the requested resource.

Are you sending the request via a proxy that is not returning the correct OPTIONS response? As the Asure Monitor (Application Insights) server should be handling (returning) the correct response based on the request sent. It may be that the request is having some additional headers added to the outbound "telemetry send" request (the sending of the events to Azure Monitor) and the fingerprinting is adding some additional headers which the Azure Monitor (Application Insights) does not accept -- you will need to configure the fingerprinting mechanism to NOT add the additional headers to these requests.

Alternatives.

onionhammer commented 3 weeks ago

@MSNev Ok, so in your 2nd paragraph, your hack is to basically disable distributed tracing by setting it to a non-"DistributedTracingModes" number?

Yes, you can set the header to anything you like and we will not override it. As long as you keep the same traceid and create your own spanid for the request and set the header directly it should work.

Are there any examples of this?

Are you sending the request via a proxy that is not returning the correct OPTIONS response? As the Asure Monitor (Application Insights) server should be handling (returning) the correct response based on the request sent

I think it's the mmapiws.com server that ultimately doesn't like the request when app insights strips / doesn't copy? these headers.. it looks like with App Inights disabled, the request to d-ipv6.mmapiws.com uses a simple POST with no preflight, whereas with app insights enabled, first a preflight is performed (to mmapiws.com) and is rejected. Somehow the app insights polyfill switches that preempts that request with a rejected OPTIONS call

onionhammer commented 3 weeks ago

Workaround for now:

Create another fetch which copies a "globalFetch" variable from window.fetch as soon as the app starts, then create a custom polyfill at the top of the stack which checks if the request is for maxmind's API, if so strip any headers that are attached by app insights.


/**
 * Polyfill the fetch in order to not execute CORS against a server which does not support it
 */
function replaceFetchOnce(globalFetch: Fetch, aiFetch: Fetch) {
  window.fetch = (args, init) => {
    if (typeof args === "string" && args.startsWith("https://d-ipv6.mmapiws.com")) {
      return globalFetch(args, {
        method: "POST",
        body: init?.body,
      });
    }

    return aiFetch(args, init);
  };
}
MSNev commented 3 weeks ago

Ok, now I'm understanding a little more there is a MUCH easier way (which needs more documentation)

We have two possible configuration items that help with this correlationHeaderExcludedDomains (string[]) and correlationHeaderExcludePatterns (RegEx[]) configs

The actual implementation that checks for these configurations (and a few others) is located here https://github.com/microsoft/ApplicationInsights-JS/blob/ed8632ae113cc7d013be825af05e1204b74a895f/shared/AppInsightsCommon/src/Util.ts#L37-L96

You can simply use this to tell us to not add the correlation headers (traceparent and / or Request-Id) to these specific domains, my previous comments where (limited (not sure why I didn't remember these at the time) to just always disabling)

@MSNev Ok, so in your 2nd paragraph, your hack is to basically disable distributed tracing by setting it to a non-"DistributedTracingModes" number?

Yes that is what I proposed, but there are no examples -- because it's not a "supported" sequence as someone (including myself) may (or may not) change that undocumented feature at some point in the future.

onionhammer commented 3 weeks ago

Ahh, this works much more simply - thanks!