Allow OT toolkit to set GUID for analytics

RESOLVED FIXED in Firefox 34

Status

RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: abr, Assigned: abr)

Tracking

unspecified
mozilla35
Points:
---
Bug Flags:
qe-verify -

Firefox Tracking Flags

(firefox34 fixed, firefox35 fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

4 years ago
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.
(Assignee)

Comment 1

4 years ago
Created attachment 8488821 [details] [diff] [review]
Allow OT toolkit to set GUID for analytics

Patch to store OpenTok client GUIDs in a "loop.ot.guid" string pref
Attachment #8488821 - Flags: review?(standard8)

Comment 2

4 years ago
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 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"
Attachment #8488821 - Flags: review?(standard8) → review+
(Assignee)

Updated

4 years ago
Depends on: 1066219
(Assignee)

Comment 6

4 years ago
Created attachment 8493373 [details] [diff] [review]
Allow OT toolkit to set GUID for analytics
(Assignee)

Updated

4 years ago
Attachment #8488821 - Attachment is obsolete: true
(Assignee)

Comment 7

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

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

Comment 8

4 years ago
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.
Attachment #8493373 - Flags: review?(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.
Attachment #8493373 - Flags: review?(standard8) → review+
https://hg.mozilla.org/mozilla-central/rev/f0c025c53090
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla35
Deprioritizing this for QE verification. Please needinfo me to request testing.
Flags: qe-verify-
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
Attachment #8493373 - Flags: approval-mozilla-aurora?

Updated

4 years ago
status-firefox34: --- → fixed
status-firefox35: --- → fixed
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.
Attachment #8493373 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.