include revision url to help match Histograms.json with packet

RESOLVED FIXED in mozilla22

Status

()

Toolkit
Telemetry
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: harsha, Assigned: vladan)

Tracking

unspecified
mozilla22
x86
Mac OS X
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

6 years ago
Hi,
  we are doing telemetry validation for metrics. We use Histograms.json as a spec to validate telemetry submissions. It would be ideal if we can get a mapping file that can show buildId => Histogram.json(hg revision). 
Thanks,
Harsha
Vlad, Nathan - I'm pretty confident that this information exists. From where can this information be pulled?
Assignee: lmandel → nobody
(Assignee)

Comment 2

6 years ago
This mapping can be generated by comparing revisions of Histograms.json with the revision used to build Nightlies. The revision used in a Nightly is published on our FTP server, e.g. http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2013-01-17-03-09-33-mozilla-central/firefox-21.0a1.en-US.win32.txt
(Assignee)

Updated

6 years ago
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → WORKSFORME
(Assignee)

Updated

6 years ago
Assignee: nobody → vdjeric
Status: RESOLVED → REOPENED
Resolution: WORKSFORME → ---
(Reporter)

Comment 3

6 years ago
Hi Vladan,
    I am looking into all appChannels not just nightly. Is there anyway I can use mercury to see which version of Histograms.json went into a particular channel build.
Thanks,
Harsha

Comment 4

6 years ago
Discussed this with Vladan. This info is already in about:buildconfig, Vladan will expose it in the ping.
Summary: requesting for mapping between buildID and corresponding Histograms.json revision → include revision url to help match Histograms.json with packet
(Assignee)

Comment 5

6 years ago
Created attachment 709913 [details] [diff] [review]
Report HG revision of Histograms.json file in Telemetry ping
Attachment #709913 - Flags: review?(nfroyd)
(Assignee)

Comment 6

6 years ago
(In reply to Harsha [:harsha] from comment #3)
> Hi Vladan,
>     I am looking into all appChannels not just nightly. Is there anyway I
> can use mercury to see which version of Histograms.json went into a
> particular channel build.
> Thanks,
> Harsha

Hello Harsha, it looks like there might be not be an easy way to get the historical mapping for the non-Nightly channels, but you will want to check with the releng folk.

I think you can still compare the release dates of individual aurora/beta/release builds with Histograms.json revisions on those branches. There are also hg tags for *some* of these builds but it doesn't look like there are tags for each release :( And finally, this info is in the about:buildconfig page of these builds, but that's likely to be a very painful way to get it. 

I would still talk to the releng people, they might have the list of revisions used in release builds somewhere.

Comment 7

6 years ago
Comment on attachment 709913 [details] [diff] [review]
Report HG revision of Histograms.json file in Telemetry ping

this needs a test. Just checking for existence of a non-empty histogramsFileVersion is ok.

I'd also call it .revision

This isn't obvious for branches since it's not a full url like http://hg.mozilla.org/mozilla-central/rev/0c45e6378f1f
Attachment #709913 - Flags: feedback-
Comment on attachment 709913 [details] [diff] [review]
Report HG revision of Histograms.json file in Telemetry ping

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

Yeah, what Taras said.  Tests at a minimum, knowing what channel we're on would help a ton here too.

::: toolkit/components/telemetry/Makefile.in
@@ +62,5 @@
>  ifdef MOZILLA_OFFICIAL
>  DEFINES += -DMOZILLA_OFFICIAL
>  endif
>  
> +MOZ_HISTOGRAMS_VERSION ?= $(shell hg parent $(srcdir)/Histograms.json --template="{node|short}\n" 2>/dev/null)

Why the version of the file, rather than the version of the tree?  Seems like it would be easier to fetch from hg.mozilla.org with the tree version.

There ought to be some other way of getting this information (defined in rules.mk or config.mk somewhere, maybe?).  It would be nice to not add another shell invocation to get this.
Attachment #709913 - Flags: review?(nfroyd)
(Assignee)

Comment 9

6 years ago
Created attachment 715727 [details] [diff] [review]
Report tree revision in Telemetry ping, v2

Renamed field to "info.revision", added a check to the telemetry xpcshell test, field is now in the form [repo path]/rev/[tree revision].

There is no existing makefile variable with the tree revision. For example, about:buildconfig relies on a MOZ_SOURCE_STAMP variable defined (with shell commands) in toolkit/content/Makefile.in. I guess I could move that call one level higher into toolkit/Makefile.in but it doesn't look like the right place. confvars.sh might be another option. I'm open to suggestions
Attachment #709913 - Attachment is obsolete: true
Attachment #715727 - Flags: review?(nfroyd)
Comment on attachment 715727 [details] [diff] [review]
Report tree revision in Telemetry ping, v2

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

This looks good, just minor nits on the test.

::: toolkit/components/telemetry/tests/unit/test_TelemetryPing.js
@@ +155,5 @@
>  
>    do_check_eq(payload.info.reason, reason);
>    do_check_true("appUpdateChannel" in payload.info);
>    do_check_true("locale" in payload.info);
> +  do_check_true("revision" in payload.info && payload.info.revision.length > 0);

Can you please split this into 2 do_check_trues so that if one of them fails, it's obvious which one?

The paranoid side of me would like to check:

do_check_true(payload.info.revision.startsWith("http"))

as well.  Your call as to whether that's being too paranoid.
Attachment #715727 - Flags: review?(nfroyd) → review+
https://hg.mozilla.org/mozilla-central/rev/2c9557cd695f
Status: REOPENED → RESOLVED
Last Resolved: 6 years ago6 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla22
You really should have asked for build peer sign-off on the Makefile patch. The hardcoding of Mercurial likely doesn't work too well for people using Git.

Fortunately, IIRC Telemetry isn't enabled by default in local builds, so most developers shouldn't run into this.
And we have make functions to deal with these since bug 746277.
My local Windows build is failing with the following error, possibly because I used a weird URI format in my hgrc:

cl : Command line error D8038 : invalid argument 'HISTOGRAMS_FILE_VERSION=ssh://mbrubeck%40mozilla.com@hg.mozilla.org/mozilla-centralrev/9a0c5e4b1d0f'

c:\Users\mbrub_000\src\elm\config\rules.mk:1010:0: command 'c:/Users/mbrub_000/src/elm/obj-metro/_virtualenv/Scripts/python.exe -O c:/Users/mbrub_000/src/elm/build/cl.py cl -FoTelemetry.obj -c -D_HAS_EXCEPTIONS=0 -I../../../dist/stl_wrappers  -DHISTOGRAMS_FILE_VERSION="ssh://mbrubeck%40mozilla.com@hg.mozilla.org/mozilla-centralrev/9a0c5e4b1d0f" -DXPCOM_TRANSLATE_NSGM_ENTRY_POINT=1 -DMOZILLA_INTERNAL_API -D_IMPL_NS_COM -DEXPORT_XPT_API -DEXPORT_XPTC_API -D_IMPL_NS_GFX -D_IMPL_NS_WIDGET -DIMPL_XREAPI -DIMPL_NS_NET -DIMPL_THEBES  -DNO_NSPR_10_SUPPORT -DEXCLUDE_SKIA_DEPENDENCIES  -DUNICODE -D_UNICODE -DNOMINMAX -D_CRT_RAND_S -DCERT_CHAIN_PARA_HAS_EXTRA_FIELDS -D_SECURE_ATL -DCHROMIUM_BUILD -DU_STATIC_IMPLEMENTATION -DOS_WIN=1 -DWIN32 -D_WIN32 -D_WINDOWS -DWIN32_LEAN_AND_MEAN  -DCOMPILER_MSVC -Ic:/Users/mbrub_000/src/elm/xpcom/build -Ic:/Users/mbrub_000/src/elm/ipc/chromium/src -Ic:/Users/mbrub_000/src/elm/ipc/glue -I../../../ipc/ipdl/_ipdlheaders  -Ic:/Users/mbrub_000/src/elm/toolkit/components/telemetry -I. -I../../../dist/include  -Ic:/Users/mbrub_000/src/elm/obj-metro/dist/include/nspr -Ic:/Users/mbrub_000/src/elm/obj-metro/dist/include/nss        -TP -nologo -W3 -Gy -Fdgenerated.pdb -wd4345 -wd4351 -wd4482 -wd4800 -wd4819 -we4553 -GR-  -DNDEBUG -DTRIMMED -Zi -UDEBUG -DNDEBUG -O1 -Oy -MD            -FI ../../../dist/include/mozilla-config.h -DMOZILLA_CLIENT  c:/Users/mbrub_000/src/elm/toolkit/components/telemetry/Telemetry.cpp' failed, return code 2
Blocks: 844188
(Assignee)

Comment 16

6 years ago
(In reply to Gregory Szorc [:gps] from comment #13)
> You really should have asked for build peer sign-off on the Makefile patch.
> The hardcoding of Mercurial likely doesn't work too well for people using
> Git.
> 
> Fortunately, IIRC Telemetry isn't enabled by default in local builds, so
> most developers shouldn't run into this.

How do you build from your git repo currently? From what I saw, the code-base is littered with hard-codings of HG in Makefiles, e.g http://mxr.mozilla.org/mozilla-central/search?string=MOZ_SOURCE_STAMP
You need to log in before you can comment on or make changes to this bug.