1278312 - Uptake monitoring failed in graph2

Status: RESOLVED FIXED

(Release Engineering :: Release Automation: Other, defect)

Description

[Rail Aliiev [:rail]]

see http://archive.mozilla.org/pub/firefox/tinderbox-builds/mozilla-release-noarch/release-mozilla-release-firefox_uptake_monitoring-bm74-build1-build0.txt.gz and https://tools.taskcluster.net/task-inspector/#vZqNkb41TW--49fp1My6rA/

looks like partial_versions": null messes the things. Probably the graph2 script doesn't handle these properly...

Comment 1

[Jordan Lund (:jlund)]

as expected, esr45 also depends on having partial_versions defined in graph2: http://archive.mozilla.org/pub/firefox/tinderbox-builds/mozilla-esr45-noarch/release-mozilla-esr45-firefox_uptake_monitoring-bm72-build1-build3.txt.gz

Comment 2

[Mihai Tabara [:mtabara]⌚️GMT]

I'm a bit confused as to how some of those buildprops value are missing. The uptake monitoring script mainly needs four mandatory buildprops to function properly: version, tuxedo_server_url, partial_versions and platforms. Did some debugging and it seems that in both mozilla-release and mozilla-esr45 there are three buidprops args missing:

"partial_versions": null,
"platform": null, (not needed for uptake monitoring but just raising a flag that's also been set wrong)
"platforms": null, 

If it didn't fail in partial_versions, it would have failed anyway when dealing with platforms the next line: http://hg.mozilla.org/mozilla-central/file/tip/testing/mozharness/scripts/release/uptake_monitoring.py#l82

These configs are set just as the other scripts in releasetasks: https://github.com/mozilla/releasetasks/blob/master/releasetasks/templates/firefox/uptake_monitoring.yml.tmpl so I doubt the culprit is here. 

Could it be something missing from https://github.com/mozilla/releasetasks/tree/master/releasetasks/release_configs ? (both release and esr45)

On a separate note, I agree I could also add some more safeguarding when processing these values at http://hg.mozilla.org/mozilla-central/file/tip/testing/mozharness/scripts/release/uptake_monitoring.py#l81 and even add more checks like this http://hg.mozilla.org/mozilla-central/file/tip/testing/mozharness/scripts/release/uptake_monitoring.py#l118

Comment 3

[Rail Aliiev [:rail]]

Looks like the graph2 generator script expects you to pass partials: http://hg.mozilla.org/build/tools/file/tip/buildfarm/release/releasetasks_graph_gen.py#l169

We definitely can put them into https://github.com/mozilla/releasetasks/blob/master/releasetasks/templates/firefox/common_extras.yml.tmpl and read from the script.

Comment 4

[Mihai Tabara [:mtabara]⌚️GMT]

Attachment: releastasks part - fix uptake monitoring for relpro graph2

Comment 5

[Mihai Tabara [:mtabara]⌚️GMT]

Attachment: Bug 1278312 - fix uptake monitoring for relpro graph2.

Review commit: https://reviewboard.mozilla.org/r/61120/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/61120/

Comment 6

[Mihai Tabara [:mtabara]⌚️GMT]

I added PR for releasetasks and build-tools to fix the uptake monitoring on relpro graph2
* partial_versions => stashed in in the common extras and grabbed it later on from the common_task_id
* platforms => similar behavior with final_verify_platforms, read from release configs / branch configs in both graphs

Questions:
1. Do I need to add the "uptake_monitoring_platforms" in jamun/date project branches?
2. Should I file a separate bug to deal with the inevitable cleanup that's imperative in http://hg.mozilla.org/build/tools/file/tip/buildfarm/release/releasetasks_graph_gen.py#l169 ? (not just partials but also build-number, version and revision, etc)

Comment 7

[Rail Aliiev [:rail]]

Comment on attachment 8766076 [details]
Bug 1278312 - fix uptake monitoring for relpro graph2.

https://reviewboard.mozilla.org/r/61120/#review58156

Comment 8

[Rail Aliiev [:rail]]

(In reply to Mihai Tabara [:mtabara] from comment #6)
> I added PR for releasetasks and build-tools to fix the uptake monitoring on
> relpro graph2
> * partial_versions => stashed in in the common extras and grabbed it later
> on from the common_task_id
> * platforms => similar behavior with final_verify_platforms, read from
> release configs / branch configs in both graphs
> 
> Questions:
> 1. Do I need to add the "uptake_monitoring_platforms" in jamun/date project
> branches?

I'm not sure what you mean. I thought you use release_platforms in graph1 and uptake_monitoring_platforms for graph2. The latter is already addressed in your PR (the dev_* configs).


> 2. Should I file a separate bug to deal with the inevitable cleanup that's
> imperative in
> http://hg.mozilla.org/build/tools/file/tip/buildfarm/release/
> releasetasks_graph_gen.py#l169 ? (not just partials but also build-number,
> version and revision, etc)

You mean killing the feature of passing things via cli args vs fetching them from a task? I think we can kill it.

Comment 9

[Mihai Tabara [:mtabara]⌚️GMT]

(In reply to Rail Aliiev [:rail] from comment #8)
> I'm not sure what you mean. I thought you use release_platforms in graph1
> and uptake_monitoring_platforms for graph2. The latter is already addressed
> in your PR (the dev_* configs).

Ah, right. For graph1 I'm reusing branchConfig['release_platforms'] whilst for graph2 I've got the dev_*configs.

> > 2. Should I file a separate bug to deal with the inevitable cleanup that's
> > imperative in
> > http://hg.mozilla.org/build/tools/file/tip/buildfarm/release/
> > releasetasks_graph_gen.py#l169 ? (not just partials but also build-number,
> > version and revision, etc)
> 
> You mean killing the feature of passing things via cli args vs fetching them
> from a task? I think we can kill it.

Yeah. But again, not just the partials argument, but also the other three which are most likely grabbed from common task id.

Comment 10

[Mihai Tabara [:mtabara]⌚️GMT]

Merged the PR - https://github.com/mozilla/releasetasks/commit/d690c5e58840feead7ebb73edf11d00aad6ef76e
Landed the tools changes as well - http://hg.mozilla.org/build/tools/rev/58243ff61d9e

We'll have to wait for 48.0 release to test and eventual close the bug.

Comment 11

[Mihai Tabara [:mtabara]⌚️GMT]

Attachment: Some quick code cleanup to rely on common task

Had to tweak the common extras whilst fixing this bug last week. I stashed in common extras more data, with respect to the partials and platforms as well.Since most of the stuff is taken from common extras, I thought it'd be better to entirely rely on that, rather than the command line args.

Not sure though what to do with mozharness revision. Are we to default it always to mozilla_revision or should I stash it in the common extras as well since we're here?

Comment 12

[Jordan Lund (:jlund)]

Comment on attachment 8767750 [details] [review]
Some quick code cleanup to rely on common task

this is fine if you feel strongly. I commented in the PR as to why I have it in there.

Comment 13

[Jordan Lund (:jlund)]

if the revision is defined in common extras, that seems fine. it be nice to let the graph 2 generation use a different mozharness rev in case we need to like we can with graph1

Comment 14

[Mihai Tabara [:mtabara]⌚️GMT]

@jlund: Thanks for the info and explanations. Didn't know the context, TIL something - will drop the patch and close the PR.

Note to self: coming back to the main topic of the bug:
* before closing this bug, we'll have to wait for 48.0 to test the uptake monitoring fixup for a release
* currently, bug 1278312 blocks this one. We'll have to wait for 48.b6 to confirm the fix works as expected

Comment 15

[Mihai Tabara [:mtabara]⌚️GMT]

48.0 RC went smoothly http://archive.mozilla.org/pub/firefox/tinderbox-builds/mozilla-release-noarch/release-mozilla-release-firefox_uptake_monitoring-bm77-build1-build0.txt.gz

Closing this