When a request abort happens, Page Previews should not switch into FETCH_COMPLETE state

When a request is aborted, the counter.PagePreviewsApiFailure should not be incremented

In T199482#4439716, @Ryasmeen wrote:

Checked all the acceptance criteria except the last one, and they are passing.

☝️ per T199482#4439716.

I'll triage this as Normal for now but bring it onto the board and into Needs Code Review as there's already a fix to be reviewed /cc @ovasileva.

Thanks for investigating and submitting the intermediate fix, @Jdforrester-WMF. Once the error's dealt with, Readers Web should look at using ContribsPager rather than duplicating it.

In T184793#4433421, @Tbayer wrote:

There may be some followup work once page previews are rolled out to Wikidata (T111231), in case the setup doesn't work out of the box there.

From Web's POV, this epic is now done (all the AC are met and the subtasks are now closed)! Are there any outstanding issues on your side @Nuria/@Tbayer?

The error rate of ~0.00188% is borne out by another two hours of data collected between 2:00 and 3:58 UTC.

We need to decide whether to send multiple events per page or follow an approach similar to https://meta.wikimedia.org/wiki/Schema:MobileWikiAppPageScroll

I captured just over an hours worth of data between 12:22 and 13:29 UTC using [0] and found the following errors:

Excellent! Thanks, @Nuria. I'll try that out now.

@elukey: As discussed, could you grab today's VirtualPageView-related processing errors from eventlog1002 and put them somewhere accessible like @mforns did in T196904#4303746?

@Tbayer: Can this be resolved once T196904: Some VirtualPageView are too long and fail EventLogging processing is?

I can now login to https://commons.m.wikimedia.org and when I do I see the CentralAuth cookies being set with a domain of commons.m.wikimedia.org 👍

Thanks to @pmiazga for deploying the change in my (unexpected!) absence.

I've scheduled ☝️ for deployment during today's European Mid-day SWAT deployment.

In T196904#4422959, @mforns wrote:

So, the errors should be a lot less frequent but not disappear, no?

@elukey was good enough to check the logs on eventlog1002:

I don't have access to eventlog1002 so I don't think that I can sign this off. Over to you for now, @Ottomata!

In T197700#4421743, @phuedx wrote:

• The number of successful API requests has dropped steadily from 400000-600000/min to ~60000/min;

Thanks for the ping, @Krinkle. I didn't address the increase in the API Request Failures in my comment and I should have.

Per T197700#4421696.

Per T198619#4420254.

I'll keep this task bookmarked to refer to if Readers Web sees similar requests in future too. Thanks, both!

💪

Not sure if this is a regression or not. Unless the XPath queries have changed dramatically during refactoring, I don't think that we've been normalising whitespace when searching for paragraphs with content before the infobox /cc @pmiazga.

@Jdlrobson gave additional context for T197729#4410014 in the parent task:

☝️ per T197491#4400205.

In T196904#4403947, @ABorbaWMF wrote:

Thanks, @Ryasmeen

In T197700#4395552, @pmiazga wrote:

IMHO we shouldn't count aborted requests as counter.PagePreviewsApiFailure. This will create a noise in our dashboards. This graph supposed to show an API errors (4xx/5xx responses) but now it will be populated with both errors and aborted HTTP calls which makes it worthless

Sure. I'll poke the engineers on the team to see if they'd be interested in trying this out.

What about using a sprite? The worst case performance impact would be:

In T197932#4391951, @Jdlrobson wrote:

Editor currently works with /^\/editor\/(\d+|all)$/
Isn't consistency more important?

In T188164#4324348, @Pchelolo wrote:

Per discussion on the Audiences-Services sync meeting, the initial step on the Page-Previews side could be to just explicitly set the Accept-Language header to the current wiki language code with no variant. <snip />

^ That's likely a question for @cscott and/or @Jdforrester-WMF and probably best asked in T43716: Support language variant conversion in Parsoid (?). Also, you might want to take a look at T195948: MCS should respect Accept-Language header, T197009: MCS should respect Accept-Language header for MW API requests, and T197792: MCS should proxy Vary and Content-Language headers header it gets from Parsoid.

It is currently difficult to debug EventLogging events. Analysts want a browser developer plugin to view the events they emit.

In T198256#4324763, @Ottomata wrote:

… we need schemas to be predicable, types to be static, and field names to be SQL compatible (no spaces, '.', other weird characters, etc.),

Full JSONSchema spec is too featureful and extensive for easy integration between different systems.

In T198256#4323745, @Ottomata wrote:

Yeah, both protobufs and thrift are options, but neither have the advantages that Avro does, yet many of the same disadvantages.

@ovasileva suggested that switching to Charter (see T181200) might get rid of this issue during today's standup ritual.

In T196904#4319559, @Jdrewniak wrote:

The alternative to that, I think, is to not call makeBeaconUrl from eventLogging, but replace it with our own function that does something similar, but chops off the encoded source_url.

Alright. This LGTM 👍

It should be possible to enter group B on reading web staging or a local machine, but impossible to enter the group on production.

For T193584#4318754:

IIRC I assigned this to myself to leave a couple of notes about our discussion during yesterday's Web Team Task Grooming meeting. So…

Fascinating! Thanks for taking the time to write this up, @Gilles!

In T193584#4312710, @phuedx wrote:

…I'll create a follow on task to capture the follow-on change to the ReadingDepth instrumentation to mark events as "oversampled" (using the terminology from the NavigationTiming schema).

Can I suggest that the issue hash fragment be #/issues/section/{{n}}, which is a little more explicit (and hopefully reads like "show issues for section 1").

In T193584#4314677, @ovasileva wrote:

I'm getting the following behavior and just wanted to check if that's expected.

As discussed in today's standup meeting, I'll create a follow on task to capture the follow-on change to the ReadingDepth instrumentation to mark events as "oversampled" (using the terminology from the NavigationTiming schema).

I approve this request for @Niedzielski and @pmiazga. @dr0ptp4kt will have to approve this request for me.

from the service side - there is some value, but it will not make any considerable difference. 99% of summary requests are stored in varnish and never reach restbase. But for remaining 1% aborting request will reduce the server load (first restbase checks Cassandra cache, and if the summary is not present it calls MCS).

In T193055#4300084, @Jdlrobson wrote:

So it sounds like your proposal is that ABANDON_START should abort any pending API requests? Is that correct?

Upstream described the rubric they use to prioritise bugs and clarified that they consider it low priority: https://github.com/GoogleChrome/puppeteer/issues/1505#issuecomment-393470731

Another possible solution: