832007 - include revision url to help match Histograms.json with packet

Status: RESOLVED FIXED

(Toolkit :: Telemetry, defect)

Description

[Harsha [:harsha]]

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

Comment 1

[Lawrence Mandel [:lmandel] (use needinfo)]

Vlad, Nathan - I'm pretty confident that this information exists. From where can this information be pulled?

Comment 2

[Vladan Djeric (:vladan)]

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

Comment 3

[Harsha [:harsha]]

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

[(dormant account)]

Discussed this with Vladan. This info is already in about:buildconfig, Vladan will expose it in the ping.

Comment 5

[Vladan Djeric (:vladan)]

Attachment: Report HG revision of Histograms.json file in Telemetry ping

Comment 6

[Vladan Djeric (:vladan)]

(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

[(dormant account)]

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

Comment 8

[Nathan Froyd [:froydnj]]

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.

Comment 9

[Vladan Djeric (:vladan)]

Attachment: 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

Comment 10

[Nathan Froyd [:froydnj]]

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.

Comment 11

[Vladan Djeric (:vladan)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/2c9557cd695f

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/2c9557cd695f

Comment 13

[Gregory Szorc [:gps]]

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.

Comment 14

[Mike Hommey [:glandium]]

And we have make functions to deal with these since bug 746277.

Comment 15

[Matt Brubeck (:mbrubeck)]

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://[e-mail]/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://[e-mail]/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

Comment 16

[Vladan Djeric (:vladan)]

(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