Closed Bug 1472627 Opened 6 years ago Closed 6 years ago

[Probe Scraper] The probe scraper is failing today 02/07/2018

Categories

(Toolkit :: Telemetry, defect, P1)

Product:
Toolkit
Component:
Telemetry
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Tracking Flags:
Tracking Status
firefox63 --- affected

People

(Reporter: Dexter, Assigned: Dexter)

References

Details

Attachments

(2 files)

Bug 1472627 - Enable the deferred error handling for event parsing. r?janerik,chutten
46 bytes, text/x-phabricator-request
janerik
: review+
chutten
: review+
Details | Review
Bug 1472627 - Fix event name and field name lengths in Events.yaml
59 bytes, text/x-review-board-request
miker
: review+
Details 
The error from the logs:

> + python probe_scraper/runner.py --out-dir probe_data --cache-dir probe_cache
> Traceback (most recent call last):
>   File "probe_scraper/runner.py", line 240, in <module>
>     args.dry_run)
>   File "probe_scraper/runner.py", line 203, in main
>     load_moz_central_probes(cache_dir, out_dir)
>   File "probe_scraper/runner.py", line 119, in load_moz_central_probes
>     results = PARSERS[probe_type].parse(paths, details["version"])
>   File "/mnt/analyses/probe-scraper/probe_scraper/parsers/events.py", line 69, in parse
>     events = parse_events.load_events(filenames[0], strict_type_checks=False)
>   File "/mnt/analyses/probe-scraper/probe_scraper/parsers/third_party/parse_events.py", line 339, in load_events
>     regex=IDENTIFIER_PATTERN)
>   File "/mnt/analyses/probe-scraper/probe_scraper/parsers/third_party/parse_events.py", line 150, in string_check
>     (identifier, value, field, max_length))
> ValueError: devtools.main: value 'pause_behaviour_changed' for field event name is greater than maximum length of 20

This is odd. The event landed 23 hours ago (bug 1470432), but it should have busted the build on m-c before getting to the aggregator.
Assignee: nobody → alessio.placitelli
Priority: -- → P1
Hi Mike! The events you landed yesterday have names that are a bit longer than expected, longer than the maximum allowed size.
Normally, building locally would have caught this problem. However, looks like there was a bug on our end and problems were not being reported for events.

> devtools.main: Value 'pause_behaviour_changed' for field event name is greater than maximum length of 20.
> devtools.main#pause_behaviour_changed: Value 'pause_behaviour_changed' for field methods is greater than maximum length of 20.
> devtools.main#pause_behaviour_changed: Value 'caught_exceptions' for field extra_keys is greater than maximum length of 15.
> devtools.main#pause: Value 'collapsed_callstacks' for field extra_keys is greater than maximum length of 15.

I think we should change these to match the stricter length requirements.
Blocks: 1470432
Flags: needinfo?(mratcliffe)
While ParseError was used throughout the codebase, errors were detected
but not reported due to the missing |atexit| handler. This also makes
string checking only run when strict checks are enabled.
I forgot to mention in the previous comment that this bug will take care of fixing the event parsing. When this happens, the m-c build will fail due to the events from comment 1.
Comment on attachment 8989124 [details]
Bug 1472627 - Enable the deferred error handling for event parsing. r?janerik,chutten

Jan-Erik Rediger [:janerik] has approved the revision.

https://phabricator.services.mozilla.com/D1910
Attachment #8989124 - Flags: review+
I'll shorten the event names and upload the patch to this bug (r=me)
Flags: needinfo?(mratcliffe)
Comment on attachment 8989148 [details]
Bug 1472627 - Fix event name and field name lengths in Events.yaml

https://reviewboard.mozilla.org/r/254216/#review260990

Just an event name change
Attachment #8989148 - Flags: review?(mratcliffe) → review+
So I renamed pause_behaviour_changed to pause_on_exceptions.

Hope that works for you.
(In reply to Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe] from comment #8)
> So I renamed pause_behaviour_changed to pause_on_exceptions.
> 
> Hope that works for you.

Thanks! But please note that this was not the only error thrown:  comment 1 lists a few of them (all should be fixed)
Flags: needinfo?(mratcliffe)
(In reply to Alessio Placitelli [:Dexter] from comment #9)
> (In reply to Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe] from
> comment #8)
> > So I renamed pause_behaviour_changed to pause_on_exceptions.
> > 
> > Hope that works for you.
> 
> Thanks! But please note that this was not the only error thrown:  comment 1
> lists a few of them (all should be fixed)

My apologies... that should be better.
Flags: needinfo?(mratcliffe)
I am strangely getting no results for events in about:telemetry and nothing is logged in the browser console.
See bug 1470493. We had to remove displaying events recently when we switched to the events ping. The mentioned bug will add it back.
For now you can inspect events in the web console, when you're on about:telemetry like this:

    Telemetry.snapshotEvents(Ci.nsITelemetry.DATASET_RELEASE_CHANNEL_OPTIN, false)
(In reply to Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe] from comment #12)
> I am strangely getting no results for events in about:telemetry and nothing
> is logged in the browser console.

This is fine, it should be because of bug 1470493. Events were moved away from the main ping and are now in their own ping (see bug 1460595). You should have an "event" ping in the archived pings and your data should be there.
(In reply to Alessio Placitelli [:Dexter] from comment #14)
> (In reply to Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe] from
> comment #12)
> > I am strangely getting no results for events in about:telemetry and nothing
> > is logged in the browser console.
> 
> This is fine, it should be because of bug 1470493. Events were moved away
> from the main ping and are now in their own ping (see bug 1460595). You
> should have an "event" ping in the archived pings and your data should be
> there.

Ah, thanks for explaining.
Hey Mike, sorry, last question :) Can I land your patch? I'll land mine right after.
Flags: needinfo?(mratcliffe)
Keywords: leave-open
(In reply to Alessio Placitelli [:Dexter] from comment #16)
> Hey Mike, sorry, last question :) Can I land your patch? I'll land mine
> right after.

Of course.
Flags: needinfo?(mratcliffe)
Pushed by alessio.placitelli@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/75e064680911
Fix event name and field name lengths in Events.yaml r=miker
Comment on attachment 8989124 [details]
Bug 1472627 - Enable the deferred error handling for event parsing. r?janerik,chutten

Chris H-C :chutten has approved the revision.

https://phabricator.services.mozilla.com/D1910
Attachment #8989124 - Flags: review+
Pushed by jrediger@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/a8eecd3c2697
Enable the deferred error handling for event parsing. r=janerik,chutten
The final piece of this puzzle is this PR: https://github.com/mozilla/probe-scraper/pull/54 (currently in review). I will close this bugs once the PR is merged and the job runs again.
Looks like probe scraper is now up and running! Closing this.
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: