851653 - Implement FHR data provider for social providers

Status: RESOLVED FIXED

(Firefox Graveyard :: SocialAPI, defect)

Description

[Shane Caraveo (:mixedpuppy)]

We will be implementing FHR and Telemetry hooks for social.  They will fall into three pieces with an intention to land some or all for fx22.  Some background conversation in bug 840828.  The outline below is a result of meetings, further conversation welcome, but patches will start to flow here.


AddonProvider
- minor changes to include service addons in the FHR collection

SocialMetricsProvider
- note: this is probably the hardest part
- Memory metrics if possible
- current provider
- socialapi enabled/disabled state
- which providers are actually used (vs enabled as an addon)
  - does not include interactions with provider, or duration of use

Telemetry (does the user:)
- note: some of these may not be trivial to implement and will fall out of the list for fx22
- number of enable/disable toggles
- open panels from the toolbar
- open flyouts from the sidebar
- open browser tabs from any social content
- have the sidebar open or closed
- use the marks button (e.g. Like)
- chat
- use share (Fx23/24)

Comment 1

[Shane Caraveo (:mixedpuppy)]

Attachment: update fhr addonprovider with service type

I think this patch is ready for review (could be wrong)

Comment 2

[Shane Caraveo (:mixedpuppy)]

Attachment: WIP SocialMetricsProvider

I doubt this one works yet unless I'm particularly lucky, just asking for early feedback

Comment 3

[Shane Caraveo (:mixedpuppy)]

Attachment: WIP telemetry hooks

Adding a series of telemetry hooks.  These are what I feel are valuable for understanding feature use, but are subject to change based on feedback.  As well, not entirely understanding telemetry buckets/etc, but stuff is showing up in about:telemetry.

John, you can see the list of data points in the patch, just wanting you to look those over, each has a description.

Comment 4

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 725626 [details] [diff] [review]
WIP telemetry hooks

># HG changeset patch
># User Shane Caraveo <[e-mail]>
># Date 1363385574 21600
>
>diff --git a/browser/base/content/browser-social.js b/browser/base/content/browser-social.js

>+          Services.telemetry.getHistogramById("SOCIAL_PROVIDER_CHANGED").add(1);

social:provider-set is called in cases that aren't what I would consider "provider changes". Seems like this should live in the provider dropdown command event handler (and the probe description should be clearer about what exactly a "change" means).

>     this.chatbar.openChat(aProvider, aURL, aCallback, aMode);
>     // We only want to focus the chat if it is as a result of user input.
>     let dwu = window.QueryInterface(Ci.nsIInterfaceRequestor)
>                     .getInterface(Ci.nsIDOMWindowUtils);
>     if (dwu.isHandlingUserInput)
>       this.chatbar.focus();
>+    Services.telemetry.getHistogramById("SOCIAL_CHAT_OPENED").add(1);

Shouldn't this go in openChat() itself, to also catch the mozSocialAPI caller?

>+      Services.telemetry.getHistogramById("SOCIAL_FLYOUT_OPENED").add(1);

It's not clear to me that this is a useful thing to track, since it depends on a lot of factors (whether the provider supports flyouts, whether the sidebar is open, whether the user uses them, etc.), and we won't be able to distinguish those.

>+    Services.telemetry.getHistogramById("SOCIAL_STATUS_PANEL_OPENED").add(1);

Similar concerns about this one.

>diff --git a/browser/modules/Social.jsm b/browser/modules/Social.jsm

>+      Services.telemetry.getHistogramById("SOCIAL_LOAD_ERROR").add(1);

>+        Services.telemetry.getHistogramById("SOCIAL_LOAD_ERROR").add(1);

I wonder whether it would be useful to track these cases separately by UI pieces - i.e. track panel load errors separately from sidebar, separately from chat window, etc.

>diff --git a/toolkit/components/telemetry/Histograms.json b/toolkit/components/telemetry/Histograms.json

General feedback, it helps to be very precise about what exactly is being measured and how in the histogram descriptions, to reduce the likelihood of misinterpretation. And we should have a very solid understanding of what these probes are going to help us understand (i.e. why they're useful) - doesn't seem like we have that yet.

Comment 5

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 725589 [details] [diff] [review]
WIP SocialMetricsProvider

gps can probably best comment on code structure and how-we're-measuring. As to what-we're-measuring, "number of errors", "enabled transition" seem like a good start, though as I mention earlier we may want finer-grain data for errors.

Comment 6

[Gregory Szorc [:gps]]

Comment on attachment 725586 [details] [diff] [review]
update fhr addonprovider with service type

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

We should probably bump the version of the measurement (we bump the version when we add new fields or change meaning of existing fields). However, you'll need to copy the measurement so the previously-recorded fields are still reported in the payload. However, we currently have fun bugs with this because the payload format doesn't facilitate multiple versions of the same measurement. See discussions in bug 837292.

::: services/healthreport/tests/xpcshell/test_provider_addons.js
@@ +123,5 @@
>  
>    let serializer = active.serializer(active.SERIALIZE_JSON);
>    let serialized = serializer.singular(data.singular);
>    do_check_eq(typeof(serialized), "object");
> +  do_check_eq(Object.keys(serialized).length, 4);   // Our two keys, plus _v.

Need to update comment.

Comment 7

[Gregory Szorc [:gps]]

Comment on attachment 725589 [details] [diff] [review]
WIP SocialMetricsProvider

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

::: services/healthreport/providers.jsm
@@ +48,5 @@
>                                    "resource://gre/modules/UpdateChannel.jsm");
>  XPCOMUtils.defineLazyModuleGetter(this, "PlacesDBUtils",
>                                    "resource://gre/modules/PlacesDBUtils.jsm");
> +XPCOMUtils.defineLazyModuleGetter(this, "SocialService",
> +                                  "resource://gre/modules/SocialService.jsm");

I don't believe you actually use this symbol.

@@ +1074,5 @@
> +  fields: {
> +    providers: FIELD_DAILY_LAST_TEXT,
> +  },
> +
> +  _serializeJSONSingular: function (data) {

Why do you override this function? This function is only called for "last" fields, of which there are none in this measurement. This function will never be called.

@@ +1099,5 @@
> +  name: "counts",
> +  version: 1,
> +
> +  fields: {
> +    loadErrors: DAILY_COUNTER_FIELD,

What do we hope to do by knowing the global count of all load errors across all providers? This doesn't seem very actionable to me.

@@ +1100,5 @@
> +  version: 1,
> +
> +  fields: {
> +    loadErrors: DAILY_COUNTER_FIELD,
> +    featureToggled: DAILY_COUNTER_FIELD,

featureToggled is rather generic. I imagine you'd regret this naming in the future. Exactly which feature is being toggled?

@@ +1109,5 @@
> +
> +this.SocialMetricsProvider = function () {
> +  Metrics.Provider.call(this);
> +
> +  this._prefs = new Preferences({defaultBranch: null});

I think you cargo culted this. It isn't used. Remove it.

@@ +1123,5 @@
> +    SocialCountsMeasurement,
> +  ],
> +
> +  onInit: function () {
> +    // setup our observers

Nit: Capitalize and add period to all comments. (Increases code readability for non-English speakers.)

@@ +1147,5 @@
> +    switch (topic) {
> +      case "social:load-error":
> +        // data is origin, store counter for origin
> +        return this.enqueueStorageOperation(function errorCounts() {
> +          counts.incrementDailyCounter("loadErrors");

Return the result of this function. Otherwise, we don't know when this function has completed and enqueueStorageOperation gets all upset.

@@ +1155,5 @@
> +        // data is 'enabled' or 'disabled'
> +        let self = this;
> +        return this.enqueueStorageOperation(function toggleCounts() {
> +          counts.incrementDailyCounter("featureToggled");
> +          self.setState("enabled", data == "enabled");

Always return a promise from a enqueueStorageOperation function.

setState expects a string. I'd prefer the argument be a string instead of relying on coercion.

Why do you call setState? I see no call to getState. Your are incurring work for nothing.

@@ +1156,5 @@
> +        let self = this;
> +        return this.enqueueStorageOperation(function toggleCounts() {
> +          counts.incrementDailyCounter("featureToggled");
> +          self.setState("enabled", data == "enabled");
> +        }

Just bind the function instead of using self. If this takes a week or two to land, you may even be able to use fat arrows!

Comment 8

[Gregory Szorc [:gps]]

Comment on attachment 725626 [details] [diff] [review]
WIP telemetry hooks

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

You should ask someone on the perf team for Telemetry feedback.

Comment 9

[:Irving Reid (No longer working on Firefox)]

Most of the telemetry values in this would be much better implemented as simple measurements (either booleans or counts) rather than as histograms. Bug 852974 will provide a straightforward hook to register simple measurements, but if you're in a hurry you can follow the implementation pattern in bug 810146 and file a follow-up bug to convert to the not-yet-existing API.

Comment 10

[Gregory Szorc [:gps]]

Is there a web UI for the simple measurements? If not, switching away from histograms (which have a UI - albeit a crude one) would be a difficult pill to swallow.

Comment 11

[:Irving Reid (No longer working on Firefox)]

Simple measurements automatically show up in the Telemetry Histogram and Telemetry Evolution charts, under the name "SIMPLE_MEASURES_<whatever>" (for example SIMPLE_MEASURES_ADDONMANAGER_INSTALLEDUNPACKED)

Comment 12

[Gregory Szorc [:gps]]

(In reply to Irving Reid (:irving) from comment #11)
> Simple measurements automatically show up in the Telemetry Histogram and
> Telemetry Evolution charts, under the name "SIMPLE_MEASURES_<whatever>" (for
> example SIMPLE_MEASURES_ADDONMANAGER_INSTALLEDUNPACKED)

Oh, I never noticed that before - awesome!

Comment 14

[Shane Caraveo (:mixedpuppy)]

Attachment: WIP telemetry hooks

uses simple measures rather than histograms.  The resulting json stored looks like this:

{"http://localhost:8888":{"providerSet":1,"flyoutOpened":2,"notificationPanelOpened":1,"chatOpened":2,"alert":2,"alertClick":1},"https://webrtc-demo.vcap.mozillalabs.com":{},"https://www.facebook.com":{"providerSet":1}}

This is much more useful for us to evaluate feature use in social than the histograms were, but I'm not sure about backend processing of that data and how we'll handle that.

Comment 15

[John Jensen]

(In reply to Shane Caraveo (:mixedpuppy) from comment #3)

> John, you can see the list of data points in the patch, just wanting you to
> look those over, each has a description.

Shane -- I looked these over; they seem reasonable to me. They all fall into the twin buckets of performance/stability and user utility -- i.e. some indication as to whether or not the features, as designed are being used.

> And we should have a very solid understanding of what these probes are 
> going to help us understand

These seem fairly clear to me

Comment 16

[John Jensen]

(In reply to Shane Caraveo (:mixedpuppy) from comment #3)

> John, you can see the list of data points in the patch, just wanting you to
> look those over, each has a description.

Shane -- I looked these over; they seem reasonable to me. They all fall into the twin buckets of performance/stability and user utility -- i.e. some indication as to whether or not the features, as designed are being used.

Comment 17

[:Irving Reid (No longer working on Firefox)]

Comment on attachment 728476 [details] [diff] [review]
WIP telemetry hooks

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

The TelemetryPing.jsm hook looks fine.

::: toolkit/components/social/SocialService.jsm
@@ +436,5 @@
> +
> +  getSimpleMeasures: function() {
> +    let m = {};
> +    for (let provider of SocialServiceInternal.providerArray) {
> +      m[provider.origin] = provider.getSimpleMeasures();

Are these "provider.origin" strings (URLs, if I read your example correctly) from a restricted enough set that they're not considered privacy sensitive? I'm pretty sure they are, but just want to check.

Also, I'm not sure how well the Telemetry dashboard will handle non-alphanumeric strings in measure names; is there a simpler identifier you could use for the provider, if it turns out the dashboard chokes on URLs?

@@ +477,5 @@
>  SocialProvider.prototype = {
> +  recordSimpleMeasure: function(name, value) {
> +    this._simpleMeasures[name] = value;
> +  },
> +  

Trailing white space

@@ +487,5 @@
> +
> +  getSimpleMeasures: function() {
> +    return this._simpleMeasures;
> +  },
> +  

More trailing white space

Comment 18

[:Gavin Sharp [email: [e-mail]]]

(In reply to :Irving Reid from comment #17)
> Are these "provider.origin" strings (URLs, if I read your example correctly)
> from a restricted enough set that they're not considered privacy sensitive?
> I'm pretty sure they are, but just want to check.

That's an interesting point - for the moment they generally are, but we also allow installing arbitrary providers. This doesn't seem much different than how we include extension IDs in FHR, but provider origins are potentially more "identifying" than extension IDs.

Comment 19

[Mark Hammond [:markh] [:mhammond]]

Comment on attachment 728476 [details] [diff] [review]
WIP telemetry hooks

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

Drive-by with a few comments.  Shane, let me know if you want me to take over this.

::: browser/base/content/browser-social.js
@@ +79,5 @@
>            SocialSidebar.update();
>            SocialShareButton.update();
>            SocialToolbar.update();
>            SocialMenu.populate();
> +          if (data)

I think this is what Gavin previously said - that this will be called on each startup while social is enabled and should probably move to the command handler where it reflects a user action.

@@ +565,5 @@
>        // Force a layout flush by calling .clientTop so
>        // that the docShell of this frame is created
>        panel.firstChild.clientTop;
> +      Social.setErrorListener(iframe, this.setFlyoutErrorMessage.bind(this));
> +      Social.provider.incrementSimpleCounter("flyoutOpened");

it would be ideal if this could differentiate between the flyout opening due to a mouse move/hover, versus a user click.  Maybe one way to mitigate this would be to also collect counts for user clicks in each of our windows? (ie, the sidebar, chats and panels?)

@@ +1122,5 @@
>      let evt = sbrowser.contentDocument.createEvent("CustomEvent");
>      evt.initCustomEvent(aEnabled ? "socialFrameShow" : "socialFrameHide", true, true, {});
>      sbrowser.contentDocument.documentElement.dispatchEvent(evt);
> +    if (Social.provider)
> +      Social.provider.incrementSimpleCounter("sidebarToggled");

Similar to the above - I believe this will fire each time the browser starts and opens the sidebar.  We probably want it in Social.toggleSidebar()?

::: browser/base/content/socialchat.xml
@@ +472,5 @@
>            this.insertBefore(cb, this.firstChild);
>            this.selectedChat = cb;
>            this.chatboxForURL.set(aURL, Cu.getWeakReference(cb));
>            this.resize();
> +          aProvider.incrementSimpleCounter("chatOpened");

I believe this is only called by browser-social, so maybe having it there makes more sense?

::: toolkit/components/social/SocialService.jsm
@@ +437,5 @@
> +  getSimpleMeasures: function() {
> +    let m = {};
> +    for (let provider of SocialServiceInternal.providerArray) {
> +      m[provider.origin] = provider.getSimpleMeasures();
> +    }

We could just use the hostname portion of the origin and if a period (.) is an issue just replace it with '-'.

Re the origin's being potentially identifying - the best I can come up with is a "whitelist" of providers we care about and group the rest into an "others" bucket - but that means ongoing tweaking of this whitelist as more providers jump on board.  Who is in a position to make a decision on this (even just a "short term" position, such as "ok for now, but let's review this again in a year")?

::: toolkit/components/social/WorkerAPI.jsm
@@ +129,5 @@
>                                            !!action, // text clickable if an
>                                                      // action was provided.
>                                            null,
>                                            listener,
>                                            type); 

existing trailing whitespace - but it's so close to this patch we might as well remove it :)

Comment 20

[Shane Caraveo (:mixedpuppy)]

(In reply to Mark Hammond (:markh) from comment #19)
> Comment on attachment 728476 [details] [diff] [review]
> WIP telemetry hooks
> 
> Review of attachment 728476 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Drive-by with a few comments.  Shane, let me know if you want me to take
> over this.

It would be good to get feedback from Irving and/or gps to see if this direction is better than the previous patch that used histograms.  I'm happy with you taking this, but I think we decided to focus on the AddonProvider patch first.  There are three parts here, we could also divide and conquer.

> ::: browser/base/content/browser-social.js
> @@ +79,5 @@
> >            SocialSidebar.update();
> >            SocialShareButton.update();
> >            SocialToolbar.update();
> >            SocialMenu.populate();
> > +          if (data)
> 
> I think this is what Gavin previously said - that this will be called on
> each startup while social is enabled and should probably move to the command
> handler where it reflects a user action.

The command handler wont catch changes due to other factors (e.g. disabling the current provider, activating a new provider), we need to take those into account.

> @@ +565,5 @@
> >        // Force a layout flush by calling .clientTop so
> >        // that the docShell of this frame is created
> >        panel.firstChild.clientTop;
> > +      Social.setErrorListener(iframe, this.setFlyoutErrorMessage.bind(this));
> > +      Social.provider.incrementSimpleCounter("flyoutOpened");
> 
> it would be ideal if this could differentiate between the flyout opening due
> to a mouse move/hover, versus a user click.  Maybe one way to mitigate this
> would be to also collect counts for user clicks in each of our windows? (ie,
> the sidebar, chats and panels?)

I've been avoiding the idea of capturing clicks.  The primary data point here, and with several of the telemetry data points, is how much this feature is being used, not so much how it is initiated.

> @@ +1122,5 @@
> >      let evt = sbrowser.contentDocument.createEvent("CustomEvent");
> >      evt.initCustomEvent(aEnabled ? "socialFrameShow" : "socialFrameHide", true, true, {});
> >      sbrowser.contentDocument.documentElement.dispatchEvent(evt);
> > +    if (Social.provider)
> > +      Social.provider.incrementSimpleCounter("sidebarToggled");
> 
> Similar to the above - I believe this will fire each time the browser starts
> and opens the sidebar.  We probably want it in Social.toggleSidebar()?

That's a good move, I cant think of any other actions that cause this.

> ::: browser/base/content/socialchat.xml
> @@ +472,5 @@
> >            this.insertBefore(cb, this.firstChild);
> >            this.selectedChat = cb;
> >            this.chatboxForURL.set(aURL, Cu.getWeakReference(cb));
> >            this.resize();
> > +          aProvider.incrementSimpleCounter("chatOpened");
> 
> I believe this is only called by browser-social, so maybe having it there
> makes more sense?

makes sense.

> ::: toolkit/components/social/SocialService.jsm
> @@ +437,5 @@
> > +  getSimpleMeasures: function() {
> > +    let m = {};
> > +    for (let provider of SocialServiceInternal.providerArray) {
> > +      m[provider.origin] = provider.getSimpleMeasures();
> > +    }
> 
> We could just use the hostname portion of the origin and if a period (.) is
> an issue just replace it with '-'.
> 
> Re the origin's being potentially identifying - the best I can come up with
> is a "whitelist" of providers we care about and group the rest into an
> "others" bucket - but that means ongoing tweaking of this whitelist as more
> providers jump on board.  Who is in a position to make a decision on this
> (even just a "short term" position, such as "ok for now, but let's review
> this again in a year")?

I don't think this one is an issue, but I'll bring it up with the privacy group.

Comment 21

[Richard Newman [:rnewman]]

(In reply to Mark Hammond (:markh) from comment #19)

> Re the origin's being potentially identifying - the best I can come up with
> is a "whitelist" of providers we care about and group the rest into an
> "others" bucket - but that means ongoing tweaking of this whitelist as more
> providers jump on board.  Who is in a position to make a decision on this
> (even just a "short term" position, such as "ok for now, but let's review
> this again in a year")?

This is exactly what we do for search providers in FHR.

https://hg.mozilla.org/services/services-central/file/825de60e341b/services/healthreport/providers.jsm#l1215

and anything that's not on it goes in "other".

The list of partners came from bizdev. We have no formal plan in place to keep it up to date, but I trust that informal will work well enough!

Comment 22

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 725589 [details] [diff] [review]
WIP SocialMetricsProvider

To reduce scope, I think we should focus on reporting the list of all providers, "activated" providers, and "selected" provider, and leave it at that for revision 1. That seems consistent with what we report for add-ons.

Comment 23

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 728476 [details] [diff] [review]
WIP telemetry hooks

I think it's probably a good idea to split telemetry into a separate bug.

Comment 24

[Shane Caraveo (:mixedpuppy)]

(In reply to Mark Hammond (:markh) from comment #19)

> Re the origin's being potentially identifying - the best I can come up with
> is a "whitelist" of providers we care about and group the rest into an
> "others" bucket - but that means ongoing tweaking of this whitelist as more
> providers jump on board.  Who is in a position to make a decision on this
> (even just a "short term" position, such as "ok for now, but let's review
> this again in a year")?


After thinking about this more, I agree this is a fine direction.  We can use the existing whitelist and builtin manifests as the whitelist for fhr and telemetry and put any 3rd party install into "other".

Shane

Comment 25

[Mike Connor [:mconnor]]

A couple of thoughts:

* We're effectively treating social providers in the same way as add-ons, so I'm not sure that the first/third party makes sense in that light.  We could use eTLD+1 if we want to prevent PPI disclosure.
* Add-ons have an option to not be reported to Mozilla services (AMO, etc) which we respect.  I would suggest that we could add something like that to a provider manifest to allow providers to opt out of this reporting.

Comment 26

[Shane Caraveo (:mixedpuppy)]

(In reply to Mike Connor [:mconnor] from comment #25)
> A couple of thoughts:
> 
> * We're effectively treating social providers in the same way as add-ons, so
> I'm not sure that the first/third party makes sense in that light.  We could
> use eTLD+1 if we want to prevent PPI disclosure.

We could do that.  We're not too far off that now, we're using origin, not a full url.

> * Add-ons have an option to not be reported to Mozilla services (AMO, etc)
> which we respect.  I would suggest that we could add something like that to
> a provider manifest to allow providers to opt out of this reporting.

iiuc, we then record for all addons unless they opt out?

Comment 27

[Mike Connor [:mconnor]]

yes,  all add-ons are reported except for those who opt out.

Comment 28

[:Gavin Sharp [email: [e-mail]]]

For versions of Firefox prior to 23, we don't support activation/installation of arbitrary providers, only those "whitelisted" in the product. And so there's no need to worry about PPI from the provider origin or implementing an opt-out system for the near term. Let's focus on getting something minimal we can uplift to beta.

Comment 29

[:Gavin Sharp [email: [e-mail]]]

(For landing on trunk, we can just not report data for non-built-in providers.)

Comment 30

[Shane Caraveo (:mixedpuppy)]

I've moved the telemetry patch to bug 860549 and requested review from gavin, given the feedback+ already received.  The latest patch has minor updates to take in other feedback in various comments.

Comment 31

[Shane Caraveo (:mixedpuppy)]

Attachment: update fhr addonprovider with service type

versioned addon counts.

obsoleting other patches, telemetry patch moved to new bug.

Comment 32

[Shane Caraveo (:mixedpuppy)]

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

Comment 33

[Shane Caraveo (:mixedpuppy)]

Attachment: update fhr addonprovider with service type

forgot to include test fix with version bump.  carrying forward r+

Comment 34

[Shane Caraveo (:mixedpuppy)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/9ad895247869

Comment 35

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/9ad895247869

Comment 36

[Shane Caraveo (:mixedpuppy)]

Comment on attachment 738684 [details] [diff] [review]
update fhr addonprovider with service type

[Approval Request Comment]
Bug caused by (feature/regressing bug #): FHR
User impact if declined: we need metrics around socialapi sooner than later to help monitor the health of firefox with social enabled, and to guide improvements for v2
Testing completed (on m-c, etc.): on m-c
Risk to taking this patch (and alternatives if risky): low
String or IDL/UUID changes made by this patch: none

Comment 37

[Mike Connor [:mconnor]]

We should probably wait until we've received data and verified it's what we're expecting before uplift happens.  Maybe aim for Monday?

Comment 38

[Shane Caraveo (:mixedpuppy)]

(In reply to Mike Connor [:mconnor] from comment #37)
> We should probably wait until we've received data and verified it's what
> we're expecting before uplift happens.  Maybe aim for Monday?

sounds good.

Comment 39

[bhavana bajaj [:bajaj]]

[Relman comment] Check with Shane regarding comment# 37 before approving on aurora.

Shane, please report in the bug if we have the supporting information which confirms the patch works fine.

Comment 40

[Alex Keybl [:akeybl]]

Any news Shane?

Comment 41

[Shane Caraveo (:mixedpuppy)]

(In reply to Alex Keybl [:akeybl] from comment #40)
> Any news Shane?

mconnor put together a query to test out the data samples we have so far, I think it looks good to go.

Comment 42

[bhavana bajaj [:bajaj]]

(In reply to Shane Caraveo (:mixedpuppy) from comment #41)
> (In reply to Alex Keybl [:akeybl] from comment #40)
> > Any news Shane?
> 
> mconnor put together a query to test out the data samples we have so far, I
> think it looks good to go.

Thanks Shane ! Please add qawanted here if you think we need some testing/verification in this bug.

Comment 43

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/32fbb9114d17

Comment 44

[u279076]

Considering this has in-testsuite coverage and no requests for QA verification have been raised I'm marking this bug as [qa-]. Please add the qawanted keyword and remove the [qa-] whiteboard tag if there's something QA needs to do here.