analytics-next icon indicating copy to clipboard operation
analytics-next copied to clipboard

Non-blocking lazy destinations

Open zikaari opened this issue 2 years ago • 2 comments

This patch changes the architecture around destinations work. Previously AJS would use the waterfall like method when loading destinations (and other plugins) where it'd wait for their loading sequence to finish before it'd start processing events. With this patch the destinations load in "background" and AJS will start processing events the moment critical plugins are ready.

The events will get forwarded to each destination and it can send the event whenever it becomes ready (assuming it does). This way if 4 out of 6 destinations are ready in 0.2s after page load, and the other 2 take 0.8s, then those 4 won't be bottlenecked by the slower ones.

Learn more: Lazy destinations SDD

zikaari avatar Sep 01 '23 19:09 zikaari

🦋 Changeset detected

Latest commit: 9f8d9ebf9d36d8f36820335d318b887d8f4a60b5

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 4 packages
Name Type
@segment/analytics-next Minor
@segment/analytics-core Minor
@segment/analytics-generic-utils Minor
@segment/analytics-node Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

changeset-bot[bot] avatar Sep 01 '23 19:09 changeset-bot[bot]

I think this is a great start. I have a few high-level comments/questions:

  1. Do we need to make any changes to how classic integrations load? It looks like the existing load() method for classic integrations doesn't wait for anything except our own 1st party code to load, and has it's own event buffering in place. Assuming I'm not misreading the existing code, I worry this might be too aggressive for classic integrations since I think they aren't giving us any problems with never-ending loads. (Edit: I'm only really asking if the timeout is necessary for classic integrations, I think the change you made to the core event queue to make them load without having to await them is still an improvement)

  2. It looks like we add the same timeout to all action destination actions, regardless of what lifecycle hook they are configured for. While classic integrations are always destinations, action destination actions can be any plugin type. This has me a little worried that we might end up being too aggressive with our values here. Let's take the google-analytics-4 destination as an example - it has a before action (setConfigurationFields). We treat any non-destination load failure as fatal, so our timeout could cause analytics.js to completely fail to initialize if say it took longer than 3 seconds for the 3rd party library to load.

I think my initial inclination is to only apply the timeout for destination plugins for now. I think further enhancements could be to potentially remove enrichment plugins that fail due to the timeout error rather than let the register function re-throw, and then figure out what to do about before plugins since (spit-balling ideas - maybe set the integration associated with that plugin to false but still load analytics.js, not sure...)

chrisradek avatar Oct 24 '23 22:10 chrisradek

@silesky @chrisradek Please have another look, the last 2 commits are the actual new changes I made

zikaari avatar Feb 27 '24 18:02 zikaari