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] Incompatible with Angular Hydration #117

Open
Newah1 opened this issue Jul 28, 2023 · 10 comments
Open

[BUG] Incompatible with Angular Hydration #117

Newah1 opened this issue Jul 28, 2023 · 10 comments
Labels
enhancement New feature or request zone support

Comments

@Newah1
Copy link

Newah1 commented Jul 28, 2023

Description/Screenshot

https://angular.io/guide/hydration

Angular offers a new feature for non-destructive client side rehydration. This allows the SSR to serve the page to the client, and the client to take the DOM and only partially recreate the DOM and listeners based on what Angular automatically to have been changed.

What I found was that ApplicationInsights-JS schedules a timeout that negatively impact Zone.js ability to put the page in a "stable" condition.

With ApplicationInsights and hydration enabled, page-load times averaged > 10 seconds. With it disabled, page loads were much more responsive, averaging less than 5 seconds..

Steps to Reproduce

Create an Angular application. Enable SSR and hydration using the following link as a guide: https://angular.io/guide/hydration

Add ApplicationInsights-JS using this guide : https://learn.microsoft.com/en-us/azure/azure-monitor/app/javascript-framework-extensions?tabs=react

Observe load time difference with ApplicationInsights turned on, vs turned off. There will be, at minimum, a ten second difference in total load times before you get the console message confirming hydration.

  • OS/Browser: Chrome Version 114.0.5735.199
  • SDK Version: ^3.0.0
  • How you initialized the SDK: Through the recommended steps in the guide above

Expected behavior

ApplicationInsights does not create timeouts, or offers the ability to avoid them via config, or uses another method to avoid interrupting Angular's Zone.js.

Additional context
Other steps taken:

  • Attempted reducing "maxBatchInterval" to 10 milliseconds to avoid the wait - this reduced wait times from 15 seconds down to 10.
    • What I found is that in ApplicationInsights-JS/channels/applicationinsights-channel-js/src /Sender.ts on line 1013 a default timeout is created that uses the max value of your batch interval or the retry interval. This will default to the retry interval on init, which is default 10 seconds.
image
  • I attempted cancelling the timeout using flush() and pause() on the sender object, but this did not cancel the init, which zone.js tracks and causes the delay for hydration.
  • Also noticed flush does not actually execute the passed in callback:
image
@Newah1 Newah1 changed the title [BUG] Incompatible with Angular Client Side Rehydration [BUG] Incompatible with Angular Hydration Jul 28, 2023
@MSNev
Copy link
Contributor

MSNev commented Jul 28, 2023

This sounds like an issue with Zone.js and not Application Insights...
It sounds like Zone.js is performing some level of monitoring around "active" timers, other frameworks (and I presume this one as well) have the ability to mark a section of the code to be "ignored" in relation to any created background timers.

ApplicationInsights does not create timeouts, or offers the ability to avoid them via config, or uses another method to avoid interrupting Angular's Zone.js.

We only create timers once we have received and batched events (ie. they are waiting to be sent), we can't ApplicationInsights does not create timeouts, as this would stop any events from being sent.

Attempted reducing "maxBatchInterval" to 10 milliseconds to avoid the wait - this reduced wait times from 15 seconds down to 10.

Changing to 10ms is not recommended as this will cause excessive network connections.

On the flushComplete, yes this is a known issue and we are in the process of putting the foundations in place to be able to correctly call the callback. The signature exists for the extension associated with the internal SDK, portions of which are now on GitHub. So for now there is no (easy) solution for that option.

@MSNev
Copy link
Contributor

MSNev commented Jul 28, 2023

Doing a quick search on Zone.Js it appears that you need to be using NgZone's to identify when you want the timer operations to be monitored and when you don't

https://indepth.dev/posts/1513/from-zone-js-to-zoneless-angular-and-back-how-it-all-works

And this summary from another site sums up my original thoughts, so your application should manage these transitions between your re-hydration and any AI usage.

In a Nutshell: How Does Zone.js Work?

Zone.js patches all common async APIs like setTimeout, setInterval, the promise API, etc. to keep track of all async operations.

Here are the basic concepts you should understand:

Zone is a mechanism for intercepting and keeping track of asynchronous work.

@Newah1
Copy link
Author

Newah1 commented Jul 28, 2023

@MSNev appreciate you stepping in and providing your insight

My concern is that this is documented and advertised as functional with Angular and documentation is provided by Microsoft that details how this integration can be performed:

https://learn.microsoft.com/en-us/azure/azure-monitor/app/javascript-framework-extensions?tabs=angular

This is out-of-the-box ApplicationInsights and out-of-the-box Angular (with hydration enabled), and if there's an issue with this configuration it seems like something ought to be fixed.

I think part of the struggle here is that I've attempted to put ApplicationInsight's timeouts into another zone, however, because of the way the application is architected this has proven to be difficult, and that's why I'm posting here to see if this falls under ApplicationInsight's purview.

At the very least, the thing that worries me is the inability for me to configure there to be no timeout - that Insights creates a non-negotiable ten second timeout on startup is something that config allows no work-around for.

Changing to 10ms is not recommended as this will cause excessive network connections.

I know this is not recommended, but was part of my effort to prevent it from creating timeouts - at least the one that is created on startup.

We only create timers once we have received and batched events (ie. they are waiting to be sent), we can't ApplicationInsights does not create timeouts, as this would stop any events from being sent.

Understood - poor wording on my part, but what I'm trying to get at is I'm looking for some level of control over these timeouts, especially the one created on startup.

@Newah1
Copy link
Author

Newah1 commented Jul 28, 2023

To clarify, and if it helps, I'm only looking for a way to get ApplicationInsights to play nice with hydration so that it can get to a stable state - in other words, I only need the page to initialize successfully, and past that it's ok if it keeps behaving as usual.

Is that a concern that would be handled here, or do you still think that may be an issue with zonejs?

@MSNev
Copy link
Contributor

MSNev commented Jul 28, 2023

This is out-of-the-box ApplicationInsights and out-of-the-box Angular (with hydration enabled), and if there's an issue with this configuration it seems like something ought to be fixed.

As not everyone enables hydration, how do you suggest that we cause a dependency on zones (and different versions of zones) when not everyone uses them?

Moving this issue to the angular plugin repo to see if anyone from the community has any ideas (ie. as we are not the angular experts)

To clarify, and if it helps, I'm only looking for a way to get ApplicationInsights to play nice with hydration so that it can get to a stable state

👍 This would be ideal, however, there is no-one currently on the ApplicationInsights teams who currently have this level knowledge about Angular zones.

especially the one created on startup.

While we currently don't have any "override" for the timeouts in ApplicationInsights, v3.x is now using a library which would allow us to introduce the option.

But in the meantime, Simplistically, the "first" timer will be getting created as part of the first trackPageView call causing an event to be recorded... So you could delay that until your page has finished rendering (and assuming nothing else "records" an event), but with the current releases this would also cause the "pageview" time to be wrong (ish). with the next release we are resetting the "time" to that from the browser reported "navigation" event (which would stop that issue).

The only other option (which would still have the same time issue) would be to add a telemetry initializer to block (return false from the callback) the event (page view and any XHR / fetch requests) and then "replay" (calling track) when your ready...

The only other possibility might be to create a new plugin which in it's processTelemetry implementation would cause / use a zone to tell the zone stull to not track any created timers (as the timer will be created as part of that initial track call) -- A telemetry initializer callback (I don't think) would be enough for this scenario.

@MSNev MSNev transferred this issue from microsoft/ApplicationInsights-JS Jul 28, 2023
@MSNev
Copy link
Contributor

MSNev commented Jul 28, 2023

However, the "above" plugin would affect "ALL" events getting reported -- I'm not sure if every application / usage for every event would want that -- maybe it would.

@Newah1
Copy link
Author

Newah1 commented Jul 31, 2023

To follow up, I was able to get the load to a stable state by injecting zone into my service that initializes ApplicationInsights. I then pass the init through

this.zone.runOutsideAngular(()=>{})

This alone wasn't enough - I'm using the Angular plugin for ApplicationInsights. I had to write a new class that extends AngularPlugin and override two methods:

trackPageView and processTelemetry

This looks like this:

image

I then pass the new class to ApplicationInsights initialization and this allows Angular to load with hydration

@MSNev
Copy link
Contributor

MSNev commented Aug 1, 2023

Not nice :-(

Technically, you should only need to one in the processTelemetry, if that works then we could probably provide an ability in the AngularPlugin to provide an optional wrapping callback or an optional "zone" (or an object that implements an interface that looks a lot like a zone..) and have it do this in it's processTelemetry

Why (should) this work?
As mentioned above every processTlemetry of every installed plugin is called and event the trackPageView => track (AppInsightsCore) => processTelmetry (for every plugin) synchronously, so by the time the trackPageView returns every processTelemetry has been called....

Assuming that no plugin "caches" the event being processed and continues "processing" later (normally via a setTimeout (but could also be some other asynchronous task). By default we don't (currently) have any of these, but there are some optional (internal) plugins which can (and do at times) "delay" the complete processing of events.

@Newah1
Copy link
Author

Newah1 commented Aug 2, 2023

Thanks @MSNev any of those solutions sound good to me. I will say, oddly, I didn't get this working by only wrapping process telemetry. In my tests, new ApplicationInsights had to be wrapped in the zone, as well.

@MSNev MSNev added enhancement New feature or request zone support labels Aug 2, 2023
@MSNev
Copy link
Contributor

MSNev commented Aug 2, 2023

That is odd.... I've tagged as an enhancement (so it won't go stale and get auto closed) and I've also created a new zone support tag as it sounds like this is going to become an increasing important change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request zone support
Projects
None yet
Development

No branches or pull requests

2 participants