microsoft / ApplicationInsights-JS

Microsoft Application Insights SDK for JavaScript
MIT License
652 stars 242 forks source link

Clarification around offline & retry support #1419

Open jpalmowski opened 4 years ago

jpalmowski commented 4 years ago

Hi I'm just seeking some clarification around the behaviour I'm seeing when the browser is offline

  1. I have a simple test app that logs an exception on load. (Angular app, not using plugin)
  2. When I'm offline AI stores the exception as expected in SessionStorage[AI_Buffer]
  3. After maxBatchInterval AI attempts to send the batch which fails
  4. The failed batch is cleared out of SessionStorage[AI_sentBuffer] but is never added back to SessionStorage[AI_Buffer] for retry

I can see code for exponential back off on retries so I'm assuming this should be a supported scenario but wanted to verify before I debugged any further.

I'm also seeking any information on if/when localStorage is going to be supported. It seems like it used to exist (https://github.com/microsoft/ApplicationInsights-JS/issues/602) and I can see some lower level util code to support it - Given your architecture is pluggable and the interfaces for session and local storage are the same I'm wondering why this hasn't been included?

My config if it helps:

config: {
        instrumentationKey: 'APP_KEY',
        enableAutoRouteTracking: false, // option to log all route changes,
        enableSessionStorageBuffer: true,
        isRetryDisabled: false,
        disableDataLossAnalysis: false
      }
MSNev commented 4 years ago

Hi @jpalmowski, sorry for the delay. As you have already seen, looking into the code the retry backoff should be kicking in to delay the retry operation (based on your isRetryDisabled: false (which is also the default)). But this check occurs after this one if ((xhr.status < 200 || xhr.status >= 300) && xhr.status !== 0) {

So if the XHR implementation is returning something that satisifes this check that it's likely why the request is getting dropped. Do (or can) you get the XHR result code returned (assuming via the F12 browser), an alternative would be to load the debug plugin (https://github.com/microsoft/ApplicationInsights-JS/tree/master/extensions/applicationinsights-debugplugin-js) which listens to the events that might provide some additional information about why the event is getting dropped.

LocalStorage: We currently don't have any scheduled work to bring LocalStorage into the App Insights codebase, so I don't have a current timeframe for when this might occur.

You are correct though in that because of the general design it's possible, and we do have an internal set of extensions that supports both LocalStorage and IndexedDb but at this point it doesn't work with the App Insights Sender. There is a general plan to move these internal plugins into the public space, but again no committed timeframe.

Bort-777 commented 2 years ago

Hi, @MSNev We are using the SDK for handling errors/events in ReactNative (mobile), but looks that it designed for online usage on browser. Supporting offline mode on mobile is actual and have to implemented in other way than cookie/localStorage management. Could you provide any plans about offline behaviour customisation or tips how to implement it on out side if possible.

MSNev commented 1 year ago

Yes, the SDK has a general assumption that it will be online all or most of the time and I don't believe that we have any general plans to assume complete offline or only online sometimes.

Currently the SDK (by default) stores events into SessionStorage (controlled by enableSessionStorageBuffer) to provide some level of resiliency between page loads (ie. the same session).

However, this generally doesn't work for Mobile / complete offline scenarios. For those it would need the LocalStorage or IndexedDb support mentioned above as well as some refactoring of the current online / offline listener (currently only used for handling retries)

All of this is current managed in the Sender.ts (channel).

As part of the SDK, it is built on a "plugin" model (every component is a plugin -- including the Sender.ts), so you "could" implement an OfflinePlugin which when offline simply stored the events into some form of persistent storage and then just not call the "nextPlugin" (thus avoiding the Sender also trying to send the events). And once it detects that it's back online it could simply pickup to persisted events and send them on down the plugin chain there are some internal helpers which you should be able to "recreate" the plugin chain between instances or simply just call track again and when your plugin gets called again you just bypass your logic. If you want to go down this path I can provide some more details.

As you mentioned ReactNative, this also seems like something that could be contributed (added) to the ReactNative extension as an optional capability.

zolakt commented 1 year ago

Can someone please help me understand this?

I have a similar scenario. I have an Ionic app that sometimes looses internet connection. While there is no connection I show an error screen with a countdown timer. After the timer expires, I reload the page with location.reload(). This happens every 30 seconds. So, eventually, the app will reload in an online state sooner or later. Sometimes the connection can be down for a while (before it is restored). Lets say it takes an hour. I'd like to collect all the errors during that time and sent them to the server once the connection is restored. So, basically I'd like to buffer around 120 error messages during that hour (one every 30 seconds).

However, on every refresh I can clearly see that the AI_buffer field in session storage is cleared. Then the page refreshes, fails again, and adds a new item to the buffer (which is empty on refresh). So effectively, I never have more than 1 item in the buffer.

Isn't the whole point of session storage that is should persist between refreshes of the same tab?

Maybe this is a Ionic/webview thing, but it doesn't seem so. e.g. I can see the session_id in the cookie changing on every refresh. It's a little difficult to test this without Ionic (or some kind of pwa). So I wanted to ask first, is this normal, that the AI-buffer field is cleared on every refresh? If so, why?

MSNev commented 1 year ago

The Send buffer should be storing the values in the Session Storage and only clear them out after they have been sent.

It also does in fact have an online / offline listener here which (should) be causing the events to be retried.

If the events are getting dropped, then your probably hitting the eventsLimitInMem (defaults to 10,000! ???) or the session based storage is not enabled of failed (because it's used too much)....

The Sender probably should be changed to not attempt to send requests when offline, and I recall that there is a retry limit (can't find the config ATM).

Another option might be to pause the sender based on your own offline listener... And as part of this maybe only reload your page if the browser reports that your online.

zolakt commented 1 year ago

I'm not really following, honestly. The problem is that the send buffer is almost always empty. Maybe I didn't explain this clearly. I've made a video, if it will helps.

For testing purposes I've set the maxBatchInterval to 10 seconds (normally I want this set to 0). The app is offline during the whole video.

  1. The page loads. It can't ping the server and displays this error screen. The item is added to AI_buffer, as expected
  2. When the countdown reaches 10, it tries to send it to app insights server and fails, as expected. Interestingly, you can see that it moves the item to AI_sentBuffer for a split second, and then immediately moves it back to AI_buffer
  3. When the countdown reaches 0, it refreshes the page with location.reload(). You can see that both buffers get completely cleared at this point. Nothing gets persisted, which is the main problem here. It even looks like something is explicitly clearing them, not the refresh itself, but I could be wrong about this
  4. The loop repeats. It again can't ping the server, and puts a new item in AI_buffer. Since nothing is persisting, the AI_buffer always has either 0 or 1 items.

So, I don't have an issue with eventsLimitInMem, since I never have more that 1 item in the buffer. And I doubt sessions storage is used too much.

https://user-images.githubusercontent.com/1048531/222115562-9e41dc76-6890-4483-b942-f7f4f31aeae1.mp4

peitschie commented 1 year ago

@zolakt a recent patch has been merged (2.8.12+ & 3+) which might allow you to address the issue you're seeing here.

By default, AppInsights uses session storage, which is cleared whenever the tab/browser is reloaded, among other conditions. Possibly, it's worth seeing if switching to local storage resolves the issue you're seeing here?

Instructions for doing this are outlined on https://github.com/microsoft/ApplicationInsights-JS/pull/2037

const appInsights = new ApplicationInsights({
  config: {
    enableSessionStorageBuffer: true,
    bufferOverride: { 
      getItem: (logger, key) => localStorage.getItem(key),
      setItem: (logger, key, value) => localStorage.setItem(key, value),
    }
  }
});
MSNev commented 1 year ago

FYI - We are now working on a new "extension" which may also help with this. The extension is being designed to allow all telemetry emitted while "offline" to be stored in either IndexedDb or LocalStorage (based on the coming new OfflineChannel configuration), we have started working on the components and plan to have it available by the end of Jan'24, portions will be available via the nightly builds as we make progress.

MSNev commented 1 year ago

If you want to see the PR's this (should) identify them

augusthjerrild commented 9 months ago

Hi @MSNev :-)

Will this latest release (3.1.0), which came out last week, make it possible to support offline situations in a react-native app? Will it work out of the box, or do I need to make some special kind of configuration before it will work offline, besides upgrading the RN Plugin :-)? Very excited about the new offline support!

MSNev commented 9 months ago

Will this latest release (3.1.0), which came out last week, make it possible to support offline situations in a react-native app?

Depending on your detection of whether your offline -- maybe (not tried this)

Will it work out of the box, or do I need to make some special kind of configuration before it will work offline

No, It's not automatic you will need to also include the new Offline Channel which is used to "store" the batched events into LocalStorage / IndexedDb.

There are a couple of additional changes which you will need as well as currently we only have a very basic IOfflineListener implementation (which is only listening to the browser events). We updated the interface to "handle" 2 levels of offline but we don't have any production implementation for this yet.

The 2 levels are

@Karlie-777 is leading the development of the Offline Channel and we still need to provide more documentation around how to use it -- And, yes we are still making improvements, so please feel free to provide feedback.

p-hlp commented 9 months ago

Great work on the 3.1 release! @MSNev @Karlie-777 I'm sure If you'd be willing to nudge us in the right direction with a minimal example how to implement theIOfflineListener and/or use the offline channel, I'd bet some people (including me) would give this a go (even if it means nightly builds for now). Happy to contribute docs / give feedback.

Karlie-777 commented 9 months ago

Thanks @p-hlp I will update the documentation soon!

raphaelcr93 commented 1 month ago

@MSNev, @Karlie-777 , could you elaborate more on the below regarding how to implement a custom network detection to override the one from the offlineListener?:

The 2 levels are

  • The runtime "event" (which really signals that the host has a network connection)
  • Manual (currently) switching to tell the SDK that while is has a network connection (because the above says it does) it should consider itself >offline because it doesn't have any internet access. This is intended to handle being on a local network which is currently disconnected.

I'm trying to use azure insights in a Mobile App and need to support offline.

Using the snippet below peitschie shared in an early comment, to override the StorageBuffer, worked very well for me.

const appInsights = new ApplicationInsights({ config: { enableSessionStorageBuffer: true, bufferOverride: { getItem: (logger, key) => localStorage.getItem(key), setItem: (logger, key, value) => localStorage.setItem(key, value), } } });

With this I can provide the functions to store the logs on my own IndexedDB to avoid losing it when the user closes the app or is offline.

However, as also pointed in the comments above, some times the logs are removed even if not synched with azure.

Most of the times it works well, but in some use cases such as when users are using VPNs, the _offlineListener.isOnline() function below still returns true, as it thinks it is connected, and the logs are wiped even if weren't properly synched with azure:

https://github.com/microsoft/ApplicationInsights-JS/blob/67f7ab1c1dd55a1486a3920f89cd1edc8e7570db/channels/applicationinsights-channel-js/src/Sender.ts#L568

If I could somehow implement an override for this function or have this second level of manual checks, I believe it would fix my problem.

Do you know if this is possible?

Appreciate the help!

Karlie-777 commented 1 month ago

@raphaelcr93 so this is the method to get offlineListener from offline plugin https://github.com/microsoft/ApplicationInsights-JS/blob/6ead58065e8dc8b5ffb7b7e5a7ac0124043f99ba/channels/offline-channel-js/src/OfflineChannel.ts#L258
and this is the code to https://github.com/microsoft/ApplicationInsights-JS/blob/6ead58065e8dc8b5ffb7b7e5a7ac0124043f99ba/shared/AppInsightsCommon/src/Offline.ts#L131 to manually reset offline status

raphaelcr93 commented 1 month ago

Hey @Karlie-777 ,

Thank you for your answer! Is it possible to get this method somehow without using the plugin?

As I'm using my own IndexedDB, I'm using the following snippet only to override the methods of the IStorageBuffer:

const appInsights = new ApplicationInsights({
  config: {
    enableSessionStorageBuffer: true,
    bufferOverride: { 
      getItem: (logger, key) => localStorage.getItem(key),
      setItem: (logger, key, value) => localStorage.setItem(key, value),
    }
  }
});

If it is possible to override or access somehow the current offlineListener without the need to user the plugin, that would be perfect.

Another question... Using the plugin would create its own IndexedDB as well? Or could I keep using mine as I'm currently doing?

Karlie-777 commented 1 month ago

@raphaelcr93 If I understand it correctly, you want to make this https://github.com/microsoft/ApplicationInsights-JS/blob/6ead58065e8dc8b5ffb7b7e5a7ac0124043f99ba/channels/applicationinsights-channel-js/src/Sender.ts#L160 to be accessible. I created an issue https://github.com/microsoft/ApplicationInsights-JS/issues/2433 here to track the feature request. And if you don't need sdk to manage storing and sending offline events automatically, then offline support plugin is not required.

raphaelcr93 commented 1 month ago

@Karlie-777 , that's correct, I would like to access the _offlineListener to be able to control the status myself the same way you do in the offline plugin!

Do you know if there is any way I could do it manually right now, with the current implementation?

Karlie-777 commented 1 month ago

@raphaelcr93 it seems that _offlineListener is not accessible from sender currently, but we will add this feature as soon as possible!

raphaelcr93 commented 1 month ago

@Karlie-777, Do you have any suggestions to apply as a workaround while we wait for the official release? On our case this is already in use and some users are being affected.

Karlie-777 commented 1 month ago

@raphaelcr93 I don't believe there is any workaround currently. let me send a quick fix by the end of this Wednesday, and then a preview version which contains the fix should be released no later than Thursday.

Karlie-777 commented 1 month ago

@raphaelcr93 preview version is published https://www.npmjs.com/package/@microsoft/applicationinsights-channel-js/v/3.3.4-nightly3.2410-03

raphaelcr93 commented 1 month ago

Thank you @Karlie-777 ,

I will test it and let you know if there I have any issues with that. Really, appreciate the help.

raphaelcr93 commented 1 month ago

Hey @Karlie-777, I'm having some issues building the module.

Any chance you have a CDN version with this fix just like the current one for 3.3.3?

https://js.monitor.azure.com/scripts/b/ai.3.gbl.min.js

Karlie-777 commented 1 month ago

@raphaelcr93 try https://js.monitor.azure.com/nightly/ai.3-nightly3.gbl.js

raphaelcr93 commented 1 month ago

@Karlie-777, can you point out how to access the offlineChannel from Javascript in this new fix? I was able to use the Javascript file above, but can't find the offlineChannel exposed, or any methods.

raphaelcr93 commented 1 month ago

@Karlie-777, actually found it! Thank you for the help, I will test it in a real case scenario.

For future reference if anyone need it, here is how I got it:

var sender = appInsights.getSender()
var offlineListener = sender.getOfflineListener()

And then you can set the state with:

offlineListener.setOnlineState

raphaelcr93 commented 1 month ago

@Karlie-777, the feature worked! Thank you!

Just a note, though! In my case, users with VPN were also returning a status equal to 0! And thus, they never went inside the If below, and the sender wiped all logs, even if they weren't send properly:

https://github.com/microsoft/ApplicationInsights-JS/blob/afb1db7043352fffeddc9e8d9061e9896b611a39/channels/applicationinsights-channel-js/src/Sender.ts#L988

I also noticed, that there were some rules around AdBlock to not check for status equal to 0: https://github.com/microsoft/ApplicationInsights-JS/blob/afb1db7043352fffeddc9e8d9061e9896b611a39/channels/applicationinsights-channel-js/src/Sender.ts#L1000

To workaround it in my use case, I had to apply a slightly change to the script in this line:

https://github.com/microsoft/ApplicationInsights-JS/blob/afb1db7043352fffeddc9e8d9061e9896b611a39/channels/applicationinsights-channel-js/src/Sender.ts#L988

What I did is to always check if the state was offline, and in this case always go inside the if to check if it needs retry:

var isOnline = _offlineListener ? _offlineListener.isOnline() : true;

if (((status < 200 || status >= 300) && status !== 0) || !isOnline) {

    // Update End Point url if permanent redirect or moved permanently
    // Updates the end point url before retry
    if(status === 301 || status === 307 || status === 308) {
        if(!_checkAndUpdateEndPointUrl(responseUrl)) {
            _self._onError(payload, errorMessage);
            return;
        }
    }

    if (!isOnline) { // offline
        // Note: Don't check for status == 0, since adblock gives this code
        if (!_isRetryDisabled) {
            const offlineBackOffMultiplier = 10; // arbritrary number
            _resendPayload(payload, offlineBackOffMultiplier);

            _throwInternal(_self.diagLog(),
                eLoggingSeverity.WARNING,
                _eInternalMessageId.TransmissionFailed, `. Offline - Response Code: ${status}. Offline status: ${!_offlineListener.isOnline()}. Will retry to send ${payload.length} items.`);
        }
        return;
    }

Not sure if it makes sense to add in the original code, but if yes, that's what I did and is working very well so far. Thank you for the help!

Karlie-777 commented 1 month ago

Use this https://github.com/microsoft/ApplicationInsights-JS/issues/2436 to Track changes

raphaelcr93 commented 1 month ago

Hey @Karlie-777,

Just found another use case that you might be able to help me with.

I noticed that when we initialize the AppInsights again the unsent data that was stored IStorageBuffer (Defined using the bufferOverride property) is completely wiped.

In this case what happens is that if the user closes the page and open it again the data is overriden by a a new set of events, even if the previous data was still stored in the indexedDB, .

I had a look at the properties 'enableSessionStorageBuffer' and 'disableDataLossAnalysis' from the main page, hoping it would help, but no luck.

Do you know about any way to avoid losing the data that was still on the buffer after initializing the appInsights library?

Karlie-777 commented 1 month ago

@raphaelcr93 when your own bufferOverride is provided, all setItem and getItem will use the overridden ones. enbableSessionStorage is mainly used when bufferoverride is not provided. once users open the page, we will try to get all previous unsent data including those data from previous sessions/keys (if there are any) and try to send them out. And when you mentioned that "data is overridden by a new set of events, event if the previous data was still stored in the indexedDB", I am not quite sure if I am following it since if events are dropped, they are either being sending out, retry too many times or the storage space limitation is reached.

raphaelcr93 commented 1 month ago

Hey @Karlie-777 , sorry for taking so long to answer! I was performing some tests and I noticed that I had some issues on my side. My implementation of the indexedDB was actually returning a promise as the getItem override action and it was causing some issues.

I fixed it by not using promises, but I still wasn't able to make it work after refreshing a page or closing the app and reopening it.

I noticed that every time we hit the refresh button, for some reason the setItem override action is called and wipe the data. I defined some logs to appear in my console, and everytime I hit refresh, this is what happens before refreshing:

setItem action action called for AI_buffer_1 Old Value: [{A SAMPLE JSON}] New Value: [] setItem action action called for AI_sentBuffer_1 Old Value: [] New Value: [{A SAMPLE JSON}] setItem action action called for AI_sentBuffer_1 Old Value: [{A SAMPLE JSON}] New Value: [] Refreshes the page and navigate to the same page

However, the data is not sent to the server.

And as expected when the page loads again all buffers are empty so no data is retried.

At this point I'm considering to try your plugin if it was tested and is working. Let me know if you have any CDN version with the plugin's code, like the one you provided to me for the nighthly build.

Thank you in advance!

Karlie-777 commented 1 month ago

@raphaelcr93 the set and get function is designed to be sync. Since indexed DB itself is async, saving/getting data from indexed DB might not be in the expected order. And If you have your own sync storage, it should be similar to session storage otherwise parsing events function might not work. And I don't believe we have offline support deployed to CDN

raphaelcr93 commented 1 month ago

Hi @Karlie-777 ,

Yes I'm using them synchronously.

However, before refreshing the screen or closing it, the override actions are still called cleaning the buffers.

Here, I made a quick sample.

Used this to initialize app Inisights:

window.CacheAppInsights = [];

function setLog (key, value) {
    console.log('Set action called for buffer: '+key);
    console.log("Old value: "+window.CacheAppInsights[key]);
    console.log("New value: "+value);
    window.CacheAppInsights[key] = value;

}

function getLog (key) {
    var result = window.CacheAppInsights[key] ? window.CacheAppInsights[key] : "[]";
    console.log('Get action called for buffer: '+key);
    console.log("Value: "+result);
    return result;
}

var configObj = {
    connectionString: $parameters.ConnectionString,
    enableAutoRouteTracking: true,
    enableCorsCorrelation: false,
    enableRequestHeaderTracking: true,
    enableResponseHeaderTracking: true,
    enableSessionStorageBuffer: true,
    disableDataLossAnalysis: false
};

configObj.bufferOverride = {};
configObj.bufferOverride.getItem = (logger, key) => getLog(key);
configObj.bufferOverride.setItem = (logger, key, value) => setLog(key,value);

window.appInsights = new Microsoft.ApplicationInsights.ApplicationInsights({
    config: configObj
    });

window.appInsights.loadAppInsights();

As a result, when I click refresh, the setItem override actions are called and the data is wiped without sending to insights:

Image

Is this how it is supposed to work? I was expecting it to be kept on memory and only sent after the screen loaded.

EDIT: Looking at the trace this behaviour is coming from the action 'performHousekeeping_1':

Image

Image

Karlie-777 commented 1 month ago

are there any page unload events triggered during the process?

raphaelcr93 commented 1 month ago

Hey @Karlie-777,

That was it! Using the property 'disableFlushOnBeforeUnload' when initializing the library, avoided the events being flushed, and worked as expected!

Thank you for pointing to the right direction.