Closed Bug 967126 Opened 10 years ago Closed 10 years ago

Ensure we send unique telemetry probes from Metro vs. Desktop

Categories

(Firefox for Metro Graveyard :: General, defect, P1)

Product:
Firefox for Metro Graveyard
Component:
General
Platform:
x86_64
Windows 8
Type:
defect

Tracking

(firefox28 affected, firefox29 affected, firefox30 verified)

Status:
VERIFIED FIXED
Milestone:
Firefox 30
Tracking Flags:
Tracking Status
firefox28 --- affected
firefox29 --- affected
firefox30 --- verified

People

(Reporter: emtwo, Assigned: emtwo)

References

Details

(Whiteboard: [release28] p=2 s=it-30c-29a-28b.1 r=ff28)

Attachments

(1 file, 2 obsolete files)

v3: Update our appName to "MetroFirefox" for telemetry pings
2.63 KB, patch
emtwo
: review+
Details | Diff | Splinter Review 
If both desktop and metro send a prob with the same name, we should be able to identify which are sent from metro and which from desktop.
Blocks: metrobacklog
Whiteboard: [feature] p=0
Assignee: nobody → msamuel
Status: NEW → ASSIGNED
Priority: -- → P2
QA Contact: jbecerra
Whiteboard: [feature] p=0 → [feature] p=0 s=it-30c-29a-28b.1
Whiteboard: [feature] p=0 s=it-30c-29a-28b.1 → [feature] p=2 s=it-30c-29a-28b.1
Target Milestone: --- → Firefox 30
Per Bug 933430 Comment 11 to 14, it looks like Metro telemetry submissions are not coming through with "MetroFirefox" as the appName since build id 20131128030201.

If you set the appName to MetroFirefox before submitting a telemetry document (as well as before saving it on close), that should ensure we can differentiate measurements from desktop vs. metro.

        Attachment #8370130 -
        Flags: review?(jmathies)

    
    

    
  
Comment on attachment 8370130 [details] [diff] [review]
v1: Update our appName to "MetroFirefox" for telemetry pings

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

::: toolkit/components/telemetry/TelemetryPing.jsm
@@ +456,5 @@
>        platformBuildID: ai.platformBuildID,
>        revision: HISTOGRAMS_FILE_VERSION,
>        locale: getLocale()
>      };
> +    if(Services.sysinfo.getProperty("hasWindowsTouchInterface")) {

This doesn't look right, isn't this going to be true for desktop running on Win8 and up?

http://mxr.mozilla.org/mozilla-central/source/xpcom/base/nsSystemInfo.cpp#180

I think what you want to check for here is the existence of Services.metro.

    
    

    
  


  
Ah, oops you're right, thank you.

        Attachment #8370130 -
        Attachment is obsolete: true

        Attachment #8370130 -
        Flags: review?(jmathies)

        Attachment #8370177 -
        Flags: review?(jmathies)

    
    

    
  
Comment on attachment 8370177 [details] [diff] [review]
v2: Update our appName to "MetroFirefox" for telemetry pings

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

::: toolkit/components/telemetry/TelemetryPing.jsm
@@ +456,5 @@
>        platformBuildID: ai.platformBuildID,
>        revision: HISTOGRAMS_FILE_VERSION,
>        locale: getLocale()
>      };
> +    if(Services.metro) {

Please add a comment here explaining why we do this.

        Attachment #8370177 -
        Flags: review?(jmathies) → review+

    
    

    
  
Also, per discussion on irc, make the check (Services.metro && Services.metro.immersive).

    
    

    
  


  
With comment and more accurate check for metro. Carrying over r+

        Attachment #8370177 -
        Attachment is obsolete: true

        Attachment #8370214 -
        Flags: review+

    
    

    
  
https://hg.mozilla.org/integration/fx-team/rev/c909875fe422
Whiteboard: [feature] p=2 s=it-30c-29a-28b.1 → [fixed-in-fx-team] [feature] p=2 s=it-30c-29a-28b.1

    
    

    
  
We should consider uplifting this to Firefox 28.
Blocks: metrov1backlog
No longer blocks: metrobacklog

          status-firefox28:
          --- → affected

          status-firefox29:
          --- → affected

          status-firefox30:
          --- → affected

          tracking-firefox28:
          --- → ?
Whiteboard: [fixed-in-fx-team] [feature] p=2 s=it-30c-29a-28b.1 → [triage] [fixed-in-fx-team] [feature] p=2 s=it-30c-29a-28b.1

    
    

    
  
https://hg.mozilla.org/mozilla-central/rev/c909875fe422
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Whiteboard: [triage] [fixed-in-fx-team] [feature] p=2 s=it-30c-29a-28b.1 → [triage]  [feature] p=2 s=it-30c-29a-28b.1
Whiteboard: [triage]  [feature] p=2 s=it-30c-29a-28b.1 → [release28] p=2 s=it-30c-29a-28b.1
Target Milestone: Firefox 30 → Firefox 28

    
    

    
  
We can evaluate a low-risk uplift while 28 is on Beta but this isn't a blocker for release.

          tracking-firefox28:
          ? → ---
Whiteboard: [release28] p=2 s=it-30c-29a-28b.1 → [release28] p=2 s=it-30c-29a-28b.1 r=ff28
Priority: P2 → P1

    
  
Target Milestone: Firefox 28 → Firefox 30
Blocks: metrobacklog
No longer blocks: metrov1backlog
Whiteboard: [release28] p=2 s=it-30c-29a-28b.1 r=ff28 → p=2 s=it-30c-29a-28b.1 r=ff30
Blocks: metrov1backlog
No longer blocks: metrobacklog
Whiteboard: p=2 s=it-30c-29a-28b.1 r=ff30 → [release28] p=2 s=it-30c-29a-28b.1 r=ff28

    
    

    
  
Could anyone please give guidance in order for the QA to verify this?
Flags: needinfo?(msamuel)

    
    

    
  
Please see bug 872206 comment 25 for verifying this bug.
Flags: needinfo?(msamuel)

    
    

    
  
I have a question regarding this issue. Please see bug 872206 comment 26
Flags: needinfo?(msamuel)

    
    

    
  
Marking this verified as fixed based on bug 872206 comment 27.
Status: RESOLVED → VERIFIED
Flags: needinfo?(msamuel)

          You need to log in
          before you can comment on or make changes to this bug.
        






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: