elastic / apm-agent-rum-js

https://www.elastic.co/guide/en/apm/agent/rum-js/current/index.html
MIT License
275 stars 133 forks source link

throttle() function causes Angular to not get stable #983

Open pappkamerad opened 3 years ago

pappkamerad commented 3 years ago

When using your library with Angular, I noticed that the setTimeout() in your throttle() function is causing Angular to not get 'stable' for the given interval (default is 60s).

Angular considers itself 'unstable' when ZoneJS is seeing pending micro or macro tasks in the Angular zone. ('isStable' doc: https://angular.io/api/core/ApplicationRef#is-stable-examples)

The problem, as i understand it, here is that the throttle() gets executed within the Angular zone if Angular is present.

We use this 'isStable' flag for certain features that need to know if the given route has finished loading and the page rendering/processing is done.

So, as the general solution to this is, that you run your long running tasks outside of the Angular zone, my idea was to also run your throttle setTimeout function in a separate, new zone if there is window.Zone available.

Something like:

export default function throttle(fn, onThrottle, opts) {
  var context = this;
  var limit = opts.limit;
  var interval = opts.interval;
  var counter = 0;
  var timeoutId;
  var throttleZone = typeof Zone !== 'undefined' && Zone.root.fork({name: 'apm-throttle-zone'});

  return function () {
    counter++;

    if (typeof timeoutId === 'undefined') {
        var timeoutFn = function () {
          timeoutId = setTimeout(function () {
            counter = 0;
            timeoutId = undefined;
          }, interval);
        };

        if (throttleZone) {
          throttleZone.run(timeoutFn)
        } else {
          timeoutFn();
        }
    }

    if (counter > limit && typeof onThrottle === 'function') {
      return onThrottle.apply(context, arguments);
    } else {
      return fn.apply(context, arguments);
    }
  };
}

What do you guys think?

vigneshshanmugam commented 3 years ago

Hmmm, Interesting and thanks for the detailed writeup.

We are working on updating our Angular integration and add support for the new versions >=9. We have moved the initialization logic of our code in to a separate zone https://github.com/elastic/apm-agent-rum-js/blob/859d328b51ee41c00aa9173bf44bd1161a644b4d/packages/rum-angular/src/apm.service.ts#L43-L45

Would this fix the problem that you are describing here?

pappkamerad commented 3 years ago

I tried to do that first, but unfortunately it did not work for me, no.

I think the issue is, that this throttle is called from within the patched XHR and therefore always runs in the Angular zone.