[BUG] Very high cpu usage when moderate number of logs (100-200) gathered offline

[peitschie]

Description/Screenshot I'm using ApplicationInsights-JS from within a cordova-based application, which often operates offline for reasonable periods of time. After a fairly small number of offline logs have been collected (e.g., 100-200 items), the track call starts taking significant amounts of CPU, and console logs in Chrome debugger start to be spammed with warnings like:

[Violation] 'readystatechange' handler took 469ms

I think this is because the current mechanism deserialises and then re-serialises all the telemetry items on each send attempt. A quick profile shows that the there's a lot of time being lost to the _setBuffer call in SessionStorage:

https://github.com/microsoft/ApplicationInsights-JS/blob/master/channels/applicationinsights-channel-js/src/SendBuffer.ts#L312

This issue is made worse due to #2034, which causes a send attempt after each new track call once the send buffer size is exceeded.

Steps to Reproduce

• OS/Browser: Any. These profiles were captured with Android 12's WebView class
• SDK Version [e.g. 22]: 2.8.3
• How you initialized the SDK: All default settings
• Track ~200 items while being offline

Expected behavior CPU usage remains nominal, despite being offline.

Additional context Issue doesn't happen while online, as the stored session storage never gets that large.

[MSNev]

Yeh, this is because of the way the data is stored into SessionStorage as a single JSON Blob... So adding / removing events as they are sent / removed requires this approach.

This will require changing or introducing a new Session Storage mechanism

[peitschie]

Is there a preference to the approach here?

I'd happy to have a crack at improving the storage mechanisms to better suite this offline use-case if you can give me a preferred direction 🙂

Seems the alternatives are:

1. Make the storage interface more pluggable (and probably async?) so a better offline solution can be plugged in (I'd be fine with adding an IndexedDb based storage mechanism if you like)
2. I saw that #1419 had a suggestion about adding an intercepting plugin before the Sender to basically catch events and store them when offline (e.g., in IndexedDb or something). Is this a good approach instead?
3. Otherwise, I can work to come up with an improvement to the session-based storage that reduces the reserialisations to avoid the high CPU load here, as this is a pretty specific use case where we are only ever adding new items, to an ever growing set

Any ideas or suggestions?

[MSNev]

Being able to provide your own storage instead of the Session or Array based ones is probably the ideal option so that you can provide whatever off-line option required.

In terms of async... Thats a bigger issue as v2.x targets es3 so you won't be able to use async / await or Promise (directly) and even in v3.x we have raised to es5 -- but we still need to support IE (which doesn't have any native Promise implementation)... If the browser API returns a "thenable" (ie. Promise) then this can be used (like we do for fetch() ) but things are a little messy.

Using @nevware21/ts-async, it's interfaces and implementations might be an option to expose an asynchronous response while still supporting IE... We are using this as part of our testing framework in v3.x.

[peitschie]

With https://github.com/microsoft/ApplicationInsights-JS/issues/2034 now being fixed, this is now roughly 50ms per log in the worst case scenario at the default event limits, and logs are now quickly discarded without incurring the serialisation costs once the limit is exceeded.

Though the serialisation approach could be optimized, it's no longer a major pain point for me, so happy for this to be closed if it's not really a priority for anyone else.

[MSNev]

Thanks for the additional context, I think keeping this open as an enhancement would be a good thing as for some users 50ms is still way too excessive, so we (should) really rework this.

[rhutchison]

This is also an issue, when someone is using an extension like ublock. If you load app insights through npm, for example and initialize/track, the events are failing and stack up/retry, causing significant performance issues in the application. There are no hooks in to error events, to be able to unload/disable appinsights.

[MSNev]

@rhutchison

There are no hooks in to error events,

I think this is an excellent observation, as AI already has unload capability if we expose this it will provide the option to stop the entire issue.

[MSNev]

#289