Event queue

40 views
Skip to first unread message

andris...@gmail.com

unread,
Apr 10, 2018, 3:44:49 AM4/10/18
to divolte-collector
Hello,

First of all, thank you for creating Divolte. In just a couple of days I was able to create a simple proof of concept, which is just amazing.

Now, I have a question from the perspective of a new user:
How can I know when it is safe to use divolte.signal? My use case is very simple - I would like to send a custom event as soon as possible after page has loaded. I'm planning to use Google Tag Manager to load divolte asynchronously. I don't know the exact moment when divolte.js will have finished loading, thus I can't know the exact moment after I can start using "divolte.signal" without fear of Javascript errors and/or missing events. 

One possible solution that other tagging solutions are using is to define queue as a global variable (e.g. var _divolteQueue = _divolteQueue || [];) even before the actual divolte.js gets loaded and executed. In this solution, events can be fired at any time. It would seem that this would also be a very simple change to the Divolte's source code: instead of initializing SignalQueue to an empty variable, one could initialize it from a global Javascript variable. 

What do you guys think about this?

Best regards,
Andris Spruds

P.S.
I do have a couple of unrelated comments/questions related to Divolte's documentation, so let me know if you are also interested to hear about those.

Friso van Vollenhoven

unread,
Apr 10, 2018, 4:19:20 AM4/10/18
to divolte-...@googlegroups.com
Hi Andris,

Unlike other tag solutions, Divolte's JavaScript currently provides no means of queueing events before the script has loaded. There's some room for improvement there. While I understand the simplicity of just declaring an array before script load, which later gets turned into a queue object, I would actually prefer a solution that initially defines a divolte module and signal function with the same signature as the one that gets introduced after script load, but I'm not sure how practical that is (in terms of keeping the tag code concise.

What we currently do with GTM, is tie Divolte signals to tags that we are near certain will only fire after the script load and trigger the script load quite early on (actually hard coded into the page, not using GTM). This makes the script load potentially compete with page resources, but we've decided to allow that (as opposed to deferred loading). Then for additional safety, we log events like this: window.divolte && divolte.signal("event_type", { ... });. This way, there won't be JS errors. We won't know about missed events, but ongoing analysis of the data shows that it's unlikely or negligible. In general, it always makes sense to perform sanity checks on your collected data to confirm that there's nothing spurious happening.


Cheers,
Friso


--
You received this message because you are subscribed to the Google Groups "divolte-collector" group.
To unsubscribe from this group and stop receiving emails from it, send an email to divolte-collector+unsubscribe@googlegroups.com.
To post to this group, send email to divolte-collector@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/divolte-collector/d1724234-3377-4dae-b1ac-9f309a1b63d8%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

andris...@gmail.com

unread,
Apr 10, 2018, 10:41:40 AM4/10/18
to divolte-collector
Hi Friso,

You are right - after thinking about it, I agree that the interface for the divolte.signal() should not change. I therefore propose the following implementation:

window._divolteQueue=window._divolteQueue || [];
window.divolte = window.divolte||{'signal':function(){window._divolteQueue.push(arguments)}};

The rest of the implementation is here: https://github.com/aspruds/divolte-collector/commit/e5894a1ce4242388420b073b4a7d9f3fca1f880a#diff-e043ca5e1414af504eff74d9033619caL1342

Would this be something you would consider accepting as a merge request?

Best regards,
Andris Spruds
To unsubscribe from this group and stop receiving emails from it, send an email to divolte-collec...@googlegroups.com.
To post to this group, send email to divolte-...@googlegroups.com.

Friso van Vollenhoven

unread,
Apr 10, 2018, 3:55:22 PM4/10/18
to divolte-...@googlegroups.com
It makes sense to add some logging when the pre-load signal queue is being used, for testing and debugging purposes.

For a useful PR, there need to be a number of test cases that evaluate the different scenarios using html pages that stage a number of different orders of events (e.g. tag code runs before script loads, tag code runs after script has already loaded, signal calls before and after the script load, etc.). See here from some inspiration.

To consider any change to the JS code, the full test suite needs to run without regressions on a large number of different browser / OS combinations. To prepare for this, it helps to at least try it out on a handful of different browsers and OS that you have access to.

Then still, changes like these are tricky because they might lead to regressions that can go unnoticed. You'll probably want a reasonable volume user to test it in the wild for a while to be certain of not losing events. You never know how the Ask.com toolbar on IE9 interferes...

If the tag updates, all docs need to be updated, including the public website (the latter is outside of your control, so that's not part of a PR). The working of the tag prior to the script loading should also be documented, as some users might prefer to load the script blocking instead and opt out of using this queue (this is common on company internal web apps or B2B type of portals where 100% of traffic are logged in users anyway and tracking is a given).

The old (existing) way of including the tag needs to keep working. Although breaking changes are allowed in minor versions, I wouldn't consider it sympathetic to impose the burden of changing your frontend code in a coordinated deployment together with the Divolte on users. To ensure this, it makes sense to run all existing tests with the old and new tag notation.

Finally, in order to contribute to Divolte, there needs to be a contributor license agreement in place. You can find the CLA for the project here: https://github.com/divolte/divolte-collector/tree/master/cla.

All of that said, I think it's an important feature. Especially for sites where page load speed is critical.


Cheers,
Friso

andris...@gmail.com

unread,
Apr 17, 2018, 9:59:16 AM4/17/18
to divolte-collector
Hello Friso,

Thank you for the detailed response. I have made some progress with the queue implementation, however, it seems that I have run into some issues with "full test suite needs to run without regressions". In particular, it seems that SeleniumJavaScriptTest.shouldGenerateIDsOnComplexSeriesOfEvents is failing randomly (every 3rd execution or so). This particular test is executing "driver.findElement(By.id("custom")).click()" and expecting an event on the server side. The divolte script itself is included with "defer async", so for me it seems that the test itself is non-deterministic. In other words, the test does not wait for divolte.js to load before executing "click". Is it possible that this is a bug in the test suite?

Best regards,
Andris Spruds

Andris Spruds

unread,
Apr 18, 2018, 12:14:43 AM4/18/18
to divolte-...@googlegroups.com
I just wanted to provide some context to my previous question. What I'm seeing is Selenium test failures on stock (master) Divolte:
2) cd divolte-collector
3) export SELENIUM_DRIVER=phantomjs
4) gradle test --tests "io.divolte.server.SeleniumJavaScriptTest"

The test fails with 
> Task :test 
io.divolte.server.SeleniumJavaScriptTest > shouldGenerateIDsOnComplexSeriesOfEvents[Selenium JS test: Local JS test run (quirks-mode=false)] FAILED
    java.lang.RuntimeException at SeleniumJavaScriptTest.java:183
io.divolte.server.SeleniumJavaScriptTest > shouldGenerateIDsOnComplexSeriesOfEvents[Selenium JS test: Local JS test run (quirks-mode=true)] FAILED
    java.lang.RuntimeException at SeleniumJavaScriptTest.java:183

Best regards,
Andris Spruds 

Friso van Vollenhoven

unread,
Apr 18, 2018, 7:17:02 AM4/18/18
to divolte-...@googlegroups.com
I am not sure if all those tests work on Phantom. Have you tried with a driver for a actual browser as well?
Friso


--
You received this message because you are subscribed to the Google Groups "divolte-collector" group.
To unsubscribe from this group and stop receiving emails from it, send an email to divolte-collector+unsubscribe@googlegroups.com.
To post to this group, send email to divolte-collector@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/divolte-collector/CAN6ShBtwPizLVeTxd6KO8VTQ%2BqCQFd86vz42XZs%2B%3Dew9XwujxA%40mail.gmail.com.

Friso van Vollenhoven

unread,
Apr 18, 2018, 7:23:43 AM4/18/18
to divolte-...@googlegroups.com
That said, it is totally plausible that there is some level of flakiness in these. The point is to get it to a state where we're confident that it works and then as part of the release process someone runs the whole suite against BrowserStack or similar with a lot of different browser / OS combinations and checks for regressions.

Friso

Andris Spruds

unread,
Apr 18, 2018, 7:27:25 AM4/18/18
to divolte-...@googlegroups.com
OK, I will try to test it with Chrome driver. 

Thank you,

Andris Spruds

andre...@godatadriven.com

unread,
Apr 18, 2018, 3:38:08 PM4/18/18
to divolte-collector
Hi Andris,

I think you're right about that test. The gotoPage() actions wait for the script to load before proceeding. However indeed on that action (after back navigation) there's no such waiting prior to firing off the custom event, which means there's probably a race condition like you suggest. I've added issue 208 to document this.

Nice catch.

Cheers,

 - Andrew
Reply all
Reply to author
Forward
0 new messages