Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] ignoreHeaders as part of config does not build with typescript #2336

Open
aviita opened this issue Apr 26, 2024 · 5 comments
Open

[BUG] ignoreHeaders as part of config does not build with typescript #2336

aviita opened this issue Apr 26, 2024 · 5 comments
Assignees

Comments

@aviita
Copy link

aviita commented Apr 26, 2024

Description/Screenshot
Reading Application Insights Javascript SDK configuration documentation I understood that ignoreHeaders can be given as part of config to limit headers being included to dependency telemetry in case of enableResponseHeaderTracking being enabled.

The documentation explicitly state:

These configuration fields are optional and default to false unless otherwise stated.

For instructions on how to add SDK configuration, see Add SDK configuration.

And in the add sdk configuration documentation:

The optional SDK configuration is passed to the Application Insights JavaScript SDK during initialization.

To add SDK configuration, add each configuration option directly under connectionString.

Steps to Reproduce
With node v18.18.1 and npm 9.8.1

  1. Checkout repo: https://github.com/aviita/react-typescript-app-application-insights-ignore-headers-bug.git
  2. Run npm run build or see eslint output for telemetry.ts.

So the repro is in the app in github. Using

Firstly I also tried updating my app insights web and react js packages with:

npm i @microsoft/applicationinsights-react-js@latest @microsoft/applicationinsights-web@latest

How I try to init:

import {
  ApplicationInsights,
  ITelemetryItem,
} from "@microsoft/applicationinsights-web";
import { ReactPlugin } from "@microsoft/applicationinsights-react-js";
import { createBrowserHistory } from "history";

const history = createBrowserHistory();

const reactPlugin = new ReactPlugin();

const appInsights = new ApplicationInsights({
  config: {
    connectionString: process.env.VITE_APP_AI_INSTRUMENTATION_CONNECTION_STRING,
    disableFetchTracking: false,
    //enableRequestHeaderTracking: true, // Enable to get request headers in dependency logs
    enableResponseHeaderTracking: true, // Enable to get response headers in dependency logs
    ignoreHeaders: [
      "Authorization",
      "X-API-Key",
      "WWW-Authenticate",
      "date",
      "x-ms-blob-type",
      "x-ms-lease-status",
      "x-ms-request-id",
      "x-ms-version",
      "content-type",
      "X-Powered-By",
      "X-Firefox-Spdy",
      "Set-Cookie",
      "Request-Context",
      "Content-Encoding",
      "Access-Control-Allow-Origin",
      "Content-Encoding",
      "Vary",
    ], // Ignore these headers in dependency logs
    disableAjaxTracking: false,
    enableCorsCorrelation: true,
    extensions: [reactPlugin /* debugPluginInstance */],
    extensionConfig: {
      [reactPlugin.identifier]: {
        history,
      },
      /*
        [DebugPlugin.identifier]: {
          trackers: toTrack
        }
        */
    },
  },
});

appInsights.loadAppInsights();
export { reactPlugin, appInsights };

Expected behavior

  • ignoreHeaders can be passed as a parameter to config
  • ignoreHeaders is documented clearly.

Actual behvior:
Currently I get build error on npm run build:

Object literal may only specify known properties, and 'ignoreHeaders' does not exist in type 'IConfiguration & IConfig'.ts(2353)

Additional context
I would like to be able to include some specific headers to my dependency telemetry from our react app. I am specifically interested in X-Cache, cache-control and Content-Length headers to se relevant data to determine if our caching configuration needs improvement or if we should do improvements to our backend performance (Azure Storage Account tier selection in this case).

It would also be nice to have rather an include headers clause than exclude headers in our case, but anyways I can work with this if I figure out how to get it working.

aviita pushed a commit to aviita/react-typescript-app-application-insights-ignore-headers-bug that referenced this issue Apr 26, 2024
* Added link to the [bug][1] into Readme
* Added more complete instrumentation, by instrumenting App with an event
  - Would have been unnecessary though I think as default instrumentation seems to already send data
* Commented telemetry.ts on what is the bug (error description)

[1]: microsoft/ApplicationInsights-JS#2336
@aviita
Copy link
Author

aviita commented Apr 26, 2024

Also reproduces with @microsoft/applicationinsights-web@2.8.16.

@MSNev MSNev assigned MSNev and siyuniu-ms and unassigned MSNev Apr 26, 2024
@MSNev
Copy link
Collaborator

MSNev commented Apr 26, 2024

The "correct" location of the configuration is actually part of the extensionConfig so it should be something like

extensionConfig["AjaxDependencyPlugin"]: { ignoreHeaders: [] }

@MSNev
Copy link
Collaborator

MSNev commented Apr 26, 2024

@siyuniu-ms we don't have any documentation in the dependency extension about how to configure these, can you please

  • Add to the readme
  • Update the typedoc (comments in the code with examples <- part of the longer term documentation "fix")
  • Sync with PM's to figure out who should update the azure monitor docs

@aviita
Copy link
Author

aviita commented Apr 29, 2024

The "correct" location of the configuration is actually part of the extensionConfig so it should be something like

extensionConfig["AjaxDependencyPlugin"]: { ignoreHeaders: [] }

Thanks, it did look like an extension when I searched the repo code, but the way it was documented seemed like it was supposed to be usable from the main config. From code I could not figure out thought what was the extension name and it seemed to be also part of the core package, so that confused me further.

Another confusion is that lots of the other properties in ICorrelationConfig are accepted by the main configuration.

Can I still configure the enableResponseHeaderTracking through main config while just adding this single ignore?

Will anyways now configure it with these extension configs.

@aviita
Copy link
Author

aviita commented Apr 29, 2024

And I think I got it working otherwise, but I likely still need to ensure the X-Cache header is also exposed to the frontend (with Access-Control-Expose-Headers in my response headers) to allow SDK to get hold of it and log it. This is because we are currently doing cross origin requests for the requests we are interested in to check the headers for.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants