1221867 - [Metrics] Custom HIstograms with multiple underscores in name does not work.

Status: VERIFIED FIXED

(Firefox OS Graveyard :: Metrics, defect)

Description

[Tamara Hills [:thills]]

I noticed that a histogram with multiple underscores in the name does not work.  Small patch to fix this up and make the replace function replace globally in the string and not just the first instance.

Comment 1

[GitHub Autolander Bot]

Attachment: [gaia] tamarahills:bugfix/1221867-fix-custom-hist-with-multiple-underscores > mozilla-b2g:master

Comment 2

[Tamara Hills [:thills]]

Comment on attachment 8683480 [details] [review]
[gaia] tamarahills:bugfix/1221867-fix-custom-hist-with-multiple-underscores > mozilla-b2g:master

Hi Marshall,

This is a small patch just to fix a problem where the histogram names can't have underscores in them as it causes a problem for the parsing in gecko.  This patch just replaces all of the underscores with dashes. Before it was replacing just the first underscore with a dash.

Thanks,

-tamara

Comment 3

[Marshall Culpepper [:marshall_law]]

Comment on attachment 8683480 [details] [review]
[gaia] tamarahills:bugfix/1221867-fix-custom-hist-with-multiple-underscores > mozilla-b2g:master

Looks straight forward

Comment 4

[Tamara Hills [:thills]]

https://treeherder.mozilla.org/#/jobs?repo=gaia&revision=4fdb6cc74951373ae284ef7d550d2afe35d1e99a

Comment 5

[Tamara Hills [:thills]]

https://github.com/mozilla-b2g/gaia/commit/65973b65c648691ce4da0e5fe913235260fe3c5e

Comment 6

[Tamara Hills [:thills]]

Comment on attachment 8683480 [details] [review]
[gaia] tamarahills:bugfix/1221867-fix-custom-hist-with-multiple-underscores > mozilla-b2g:master

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): This is a new feature in 2.5
[User impact] if declined: The contacts app currently uses a histogram name with multiple underscores.  If this is not allowed then contacts app would not be collecting custom metrics
[Testing completed]: Local testing completed 
[Risk to taking this patch] (and alternatives if risky):  Very small.  Two line change that uses a regexp to look for multiple underscores.  Alternative is to have contacts team remove multiple underscores from their name, but this would not address the root cause.
[String changes made]:No.

Comment 7

[Mahendra Potharaju [:mahe]]

Comment on attachment 8683480 [details] [review]
[gaia] tamarahills:bugfix/1221867-fix-custom-hist-with-multiple-underscores > mozilla-b2g:master

Approved for 2.5 uplift. 

Thanks

Comment 8

[Iris Hsiao [:ihsiao]]

Verified@
Build ID               20151107001102
Gaia Revision          23cab7ea0fcecab7689d340baf604e024e88f9a3
Gaia Date              2015-11-09 06:13:17
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/e2a910c048dc82fc3be53475f18e7f81f03e377b
Gecko Version          45.0a1
Device Name            aries
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.worker.20151106.232924
Firmware Date          Fri Nov  6 23:29:32 UTC 2015
Bootloader             s1

### Verify steps###
1. git clone https://github.com/IrisHsiao/Telemetry-test-app.git
2. Modify histogram's names with multiple underscores, go to the folder of your test app, and then edit [test app folder]/js/app.js
-var count  = new AdvancedTelemetryHelper(ath_ins.HISTOGRAM_COUNT, 'mycount');
-var linear = new AdvancedTelemetryHelper(ath_ins.HISTOGRAM_LINEAR, 'mylinear', 1, 1000, 12);
-var exp = new AdvancedTelemetryHelper(ath_ins.HISTOGRAM_EXP, 'myexp', 1, 500, 5);
+var count  = new AdvancedTelemetryHelper(ath_ins.HISTOGRAM_COUNT, 'my_test_count');
+var linear = new AdvancedTelemetryHelper(ath_ins.HISTOGRAM_LINEAR, 'my_test_2_linear', 1, 1000, 12);
+var exp    = new AdvancedTelemetryHelper(ath_ins.HISTOGRAM_EXP, 'my_test_2-exp', 1, 500, 5);
3. Install test app
>> 11-09 17:32:47.098  5092  5092 I Australia: Content JS INFO: telemetry|my-test-count|4|1|0|0 
>> 11-09 17:32:47.108  5092  5092 I Australia: Content JS INFO: telemetry|my-test-1-linear|1|1|1000|12 
>> 11-09 17:32:47.108  5092  5092 I Australia: Content JS INFO: telemetry|my-test-2-exp|0|1|500|5 
4. Enter Simple:"1", Linear: "50", Exp: "100" in order in the text field on test app, and check logs
>> 11-09 17:33:05.678  5092  5092 I Australia: Content JS INFO: telemetry|my-test-count|1 
>> 11-09 17:33:11.468  5092  5092 I Australia: Content JS INFO: telemetry|my-test-1-linear|50 
>> 11-09 17:33:16.438  5092  5092 I Australia: Content JS INFO: telemetry|my-test-2-exp|100 
5. Go to test server and verify the data correction in addonHistograms section of the payload

### Verify result###
Custom HIstograms with multiple underscores in name works

Comment 9

[Carsten Book [:Tomcat]]

Hi, could you provide a 2.5 PR request ? Thanks!

Comment 10

[Carsten Book [:Tomcat]]

landed on 2.5 https://github.com/mozilla-b2g/gaia/commit/e71f10c3bf0d8fcbd34bbb7f10acc7589d7d5647