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

maxAjaxCallsPerView doesn't account for filtering by TelemetryInitializer #1887

Closed
JasonTheProgrammer opened this issue Aug 17, 2022 · 8 comments

Comments

@JasonTheProgrammer
Copy link

Is your feature request related to a problem? Please describe.
I'm using a telemetry initializer to filter out a large volume of AJAX remote dependency calls that fit a particular criteria. Depending on user activity, there may be thousands of dependency calls that do not get logged.

The problem though, is that these filtered / excluded events still count towards the count of tracked AJAX attempts, which is limited per maxAjaxCallsPerView configuration parameter.

So, say there are 1,000 AJAX requests on the page. 980 of them get filtered by my telemetry initializer. The remaining 20 may not all get logged, because the 980 that were filtered out, all counted towards the default limit of 500, then I get error:
"Maximum ajax per page view limit reached, ajax monitoring is paused until the next trackPageView(). In order to increase the limit set the maxAjaxCallsPerView configuration parameter."

Describe the solution you'd like
AJAX remote dependancy calls that are filtered by a telemetry initializer should not count towards the maxAjaxCallsPerView.

Describe alternatives you've considered
I can set maxAjaxCallsPerView to -1, so I do get all AJAX dependencies logged. I can then use the telemetry initializer to implement my own "max ajax calls per view" logic, so there's no loss of functionality. It's just:
a) A bit of extra work, and;
b) An unexpected outcome - it's strange to have, say, only 20 AJAX dependencies logged, while simultaneously getting the error "Maximum ajax per page view limit reached".

Additional context
Refer: extensions\applicationinsights-dependencies-js\src\ajax.ts

_trackAjaxAttempts variable is checked, and incremented in this function.

@MSNev
Copy link
Collaborator

MSNev commented Aug 18, 2022

That is not going to work as the TelemetryInitializer is further down the pipeline from the code that produces (or not produces) the dependency request.

What I think you are trying to do is just stop the request from being tracked, which there is a config for that is evaluated here the config is called excludeRequestFromAutoTrackingPatterns if can be an array of RegEx's or strings.

@MSNev
Copy link
Collaborator

MSNev commented Aug 18, 2022

And if you are controlling the requests, there is another more "hacky" way, but id prefer you use the above if that works for you.

@JasonTheProgrammer
Copy link
Author

TelemetryInitializer is further down the pipeline

That was my strong suspicion, but I'm not familiar enough with the code base to be able to verify that without a learning curve.

excludeRequestFromAutoTrackingPatterns

Good to know. Looks useful, but doesn't quite fit my use case. The requests I'm wanting to exclude are being made by a JS component that's very chatty (an interactive map). I don't want to log its successful requests, but I do want to log failed requests, so I need a more nuanced filter than excludeRequestFromAutoTrackingPatterns allows.

if you are controlling the requests

As above, nope. And it's hard to imagine any other man-in-the-middle or monkey-patching options would be superior to the alternative I first posted: set maxAjaxCallsPerView to -1 and use an initializer to limit the logged number of calls if desired.

As a consolation, maybe this should just be a request for documentation update on https://github.com/MicrosoftDocs/azure-docs/blob/main/articles/azure-monitor/app/javascript.md, to point out that maxAjaxCallsPerView does not account for initializer's filtering requests?

@MSNev
Copy link
Collaborator

MSNev commented Aug 18, 2022

Ok, understand.
So for your use case it seems like something like the telemetry initializer but that is hooked into only the ajax pipeline...
That sounds like a viable option, I'll tag this as an enhancement (so it's not auto closed). But it may be a while before we can schedule this for design / review.

@JasonTheProgrammer
Copy link
Author

something like the telemetry initializer but that is hooked into only the ajax pipeline

Yes, that would work.

At first glance, it looks like extending excludeRequestFromAutoTrackingPatterns from type string[] | RegExp[] to string[] | RegExp[] | ((xhr?: XMLHttpRequestInstrumented, request?: Request | string, init?: RequestInit) => boolean) would help. But, it looks like the current sequence of events means it would get called before the request response is received, meaning the response couldn't be inspected to determine whether the telemetry should be excluded.

may be a while before we can schedule this for design / review

No sweat.

@MSNev MSNev added this to the 2.8.7 milestone Aug 27, 2022
@MSNev
Copy link
Collaborator

MSNev commented Aug 27, 2022

Ok, I had to add some code around the same area so I went ahead and added support for this at the same time.

MSNev added a commit that referenced this issue Aug 29, 2022
MSNev added a commit that referenced this issue Aug 30, 2022
@MSNev MSNev added the fixed - waiting release PR Committed and waiting deployment label Aug 30, 2022
@MSNev
Copy link
Collaborator

MSNev commented Aug 30, 2022

Ok, I managed to sneak this into the next release (should start later this week) as it was around the same location as some other required changes.

I've added a addDependencyInitializer and included some documentation here.

All going well there should be an automated nightly deployment tonight with this change included that you should be able to test before the final release.

MSNev added a commit that referenced this issue Aug 30, 2022
@MSNev MSNev closed this as completed Sep 14, 2022
@github-actions
Copy link

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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 15, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants