1066816 - Allow OT toolkit to set GUID for analytics

Status: RESOLVED FIXED

(Hello (Loop) :: Client, defect)

Description

[Adam Roach [:abr]]

As discussed in Bug 1033587 comment 3, an ongoing source of pain for TokBox is
the inability to get clean analytics for the desktop client due to the absence
of origins in the context that Loop runs. The latest OpenTok library (2.2.9;
see Bug 1066219) incorporates a new API explicitly designed to address this
situation. We need a small bit of glue on our side to attach this new API to a
persistent data store.

Comment 1

[Adam Roach [:abr]]

Attachment: Allow OT toolkit to set GUID for analytics

Patch to store OpenTok client GUIDs in a "loop.ot.guid" string pref

Comment 2

[song]

OT2.2.9 should resolve this issue. Here's some information about modifying GUID:

OT.overrideGuidStorage(storageInterface)

Assigns the object storageInterface as the storage interface for the entire framework.

storageInterface must implement get and set methods to be valid. If storageInterface is invalid then it will be rejected and an exception will be raised.

Storage Interface

A valid Storage Interface implements set and get methods.

set(guid, completionHandler) set takes the guid to set and a function (completionHandler) to call when the set succeeds or fails. The signature of completionHandler is function(error), where erroris Null if no error occurred or a String.

get(completionHandler) get takes a function (completionHandler) to call when the guid is either retrieved or an error occurs. The signature of completionHandler is function(error, guid). error is Null if no error occurred or a String. guid is a string representing the retrieved Guid.

Usage

// Implement our default storage mechanism, which sets/gets a cookie
// called 'opentok_client_id'
OT.overrideGuidStorage({
  get: function(completion) {
    completion(null, OT.$.getCookie('opentok_client_id'));
  },

  set: function(guid, completion) {
    OT.$.setCookie('opentok_client_id', guid);
    completion(null);
  }
});
// Implement a more complicated storage mechanism
var ComplexStorage = function() {
  // Do a bunch of complex stuff, track internal state, etc

  // expose get and set APIs
  this.set = function(guid, completion) {
    AwesomeBackendService.set(guid, function(response) {
      completion(response.error || null);
    });
  };

  this.get = function(completion) {
    AwesomeBackendService.get(function(response, guid) {
      completion(response.error || null, guid);
    });
  };
};

OT.overrideGuidStorage(new ComplexStorage());

Comment 3

[Mark Banner (:standard8)]

Comment on attachment 8488821 [details] [diff] [review]
Allow OT toolkit to set GUID for analytics

Review of attachment 8488821 [details] [diff] [review]:
-----------------------------------------------------------------

r=Standard8 with the comment extended.

::: browser/components/loop/content/js/conversation.jsx
@@ +409,5 @@
>      // Do the initial L10n setup, we do this before anything
>      // else to ensure the L10n environment is setup correctly.
>      mozL10n.initialize(navigator.mozLoop);
>  
> +    // Plug in an alternate client ID mechanism

Lets just extend the comment slightly:

"... as localStorage and cookies don't work in the conversation window"

Comment 4

[Adam Roach [:abr]]

https://hg.mozilla.org/integration/fx-team/rev/ecc5cb4dcdfe

Comment 5

[Ed Morley [:emorley]]

Backed out for:
https://tbpl.mozilla.org/php/getParsedLog.php?id=48198564&tree=Fx-Team
https://tbpl.mozilla.org/php/getParsedLog.php?id=48200322&tree=Fx-Team

remote:   https://hg.mozilla.org/integration/fx-team/rev/6f609219f77a

Comment 6

[Adam Roach [:abr]]

Attachment: Allow OT toolkit to set GUID for analytics

Comment 7

[Adam Roach [:abr]]

This should fix the tree burn issue -- checking to make double-sure:

https://tbpl.mozilla.org/?tree=Try&rev=ca80038a0035

Comment 8

[Adam Roach [:abr]]

Comment on attachment 8493373 [details] [diff] [review]
Allow OT toolkit to set GUID for analytics

Now checks for OT object (and appropriate method) so we don't hose the unit tests.

Comment 9

[Mark Banner (:standard8)]

Comment on attachment 8493373 [details] [diff] [review]
Allow OT toolkit to set GUID for analytics

So normally we stub in the tests rather than the code, but I looked at this earlier and it is indeed difficult to stub with the way it is.

I therefore think we should leave it as you've got it, and we can improve it later.

Comment 10

[Adam Roach [:abr]]

https://hg.mozilla.org/integration/fx-team/rev/f0c025c53090

Comment 11

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/f0c025c53090

Comment 12

[u279076]

Deprioritizing this for QE verification. Please needinfo me to request testing.

Comment 13

[Randell Jesup [:jesup] (needinfo me)]

Comment on attachment 8493373 [details] [diff] [review]
Allow OT toolkit to set GUID for analytics

Approval Request Comment
Uplift request for patches staged and tested on Fig

Comment 14

[Randell Jesup [:jesup] (needinfo me)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/623abe4840e5
https://hg.mozilla.org/releases/mozilla-aurora/rev/1d2300d17d3b

Comment 15

[Lawrence Mandel [:lmandel] (use needinfo)]

Comment on attachment 8493373 [details] [diff] [review]
Allow OT toolkit to set GUID for analytics

I worked with Randell and Maire on uplifting a large number of Loop bugs at once. All of the bugs have been staged on Fig and tested by QE before uplift to Aurora. As well, all of the bugs are isolated to the Loop client. Randell handled the uplift with my approval. I am adding approval to the bug after the fact for bookkeeping.