I pushed the proposed change marked as DNM - @Dreamy_Jazz please let me know if this is what we're looking for. I haven't tested it yet, other than just running tests locally.

In c) I meant to skip adding this row entirely, and not add anything other. Yes, sorry, I messed up when copying/pasting. I didn't know is it safe to insert data to other table in such case as I don't know the big picture of what CheckUser is doing.

I see four possible solutions here:

So after investigating the code - We have a RecentChange entry, that contains information about the change in our system. RecentChange has an attribute rc_logid -> which is the log entry associated with given change. It can be 0 -> no log entry set, or a integer -> then it's a foreign_key to logging table.

@CDanis could you elaborate a little bit on this one? What do you mean "force a distributed tracing" ?

In T343983#9161191, @Dreamy_Jazz wrote:

@pmiazga I was able to find it (I think) at https://logstash.wikimedia.org/app/dashboards#/doc/logstash-*/logstash-deploy-1-7.0.0-1-2023.09.04?id=xfyrX4oBi6TOjMqmYTZz

I found another log statement exactly from the same line. The previous error comes from fixStuckGlobalRename but the one I found comes from SpamBlacklist: https://logstash.wikimedia.org/app/dashboards#/view/c7013c90-a487-11ec-be91-b3435f0c0c49?_g=h@53fc073&_a=h@8974712 and was triggered on EditPage.

We went through a set of reviews and I'm expecting this work to be merged and this ticket to be closed this week.

Today on codemob we decided to move Telemetry class to includes/libs folder and make it generic. But when doing that, I found that the Telemetry cannot be global singleton, as it uses the MediaWiki config: https://github.com/wikimedia/mediawiki/blob/5405fdccfcb7fec8c0fd9b9c9c2a73ee18d98127/includes/http/Telemetry.php#L60

I cannot reproduce it anymore nor I don't see any similar logs. This error happened twice on on 2023-08-10 after each command:

Sorry for a bit of silence in this ticket, Recently I focused a little bit more on the T344926 issue which caused us lots of trouble due to layering issues (RESTBagOStuff and MultiHttpClient are part of general libs but Telemetry is MediaWiki specific).

We spoke about those changes today on Codemob and we agreed on:

In T344926#9132153, @BPirkle wrote:

Unless CDanis says there's urgency, I vote for skipping the temporary fix, as I'm not convinced the temporary fix is as simple as we'd hoped.

It's pretty straightforward: https://gerrit.wikimedia.org/r/953689 but I agree - I don't like it as it introduces the layering issue - the RESTBagOStuff shouldn't know anything about Telemetry/Tracing - it's the HTTPClient (whatever client we use) responsibility, not the RESTBagOStuff.

After a pairing session with @DAlangi_WMF we decided that the best way to tackle this :

This is caused by MultiHttpClient using raw curl_*() calls instead HttpRequestFactory to get the HTTP client. On Prod env sessions use kask-session object cache, which uses RESTBagOStuff. The RESTBagOStuff could use a specific HTTP client - but we do not specify one via config (https://github.com/wikimedia/operations-mediawiki-config/blob/811b3dad11ea1cb629e06f71d94ab4e7208ccdfd/wmf-config/CommonSettings.php#L596C18-L596C3), therefore it fallback to default MultiHttpClient library to send requests (https://github.com/wikimedia/mediawiki/blob/f071c22a9a3e7e399dcf3256c96a952f15291a69/includes/libs/objectcache/RESTBagOStuff.php#L149). I also noticed it can support only MultiHttpClient and we cannot pass anything that extends MWHttpRequest which supports telemetry headers.

T344926 is a standalone ticket as it's an existing issue. From a quick check, sessions can use RESTBagOStuff to store/retrieve session data. When setting up a RESTBagOStuff we can specify a custom HTTP client, or it will use MultiHttpClient by default.

For quick hacking I'm going to use the OpenTelemetry monorepo - https://github.com/open-telemetry/opentelemetry-php as the one from mszabo (https://github.com/mszabo-wikia/opentelemetry-php) seems a bit outdated.

@Dreamy_Jazz - I quickly checked this code and I found that this is caused by this line

Adding Content-Transform-Team as they are the owners of Proton service. If there are any questions regarding this change I'm happy to help.

I see two tickets here, one to add tracing headers to the response (same as X-Request-ID), and second ticket to modify the extension to show additional fields.

@CDanis Can you confirm that you see traces on prod ?

Recently @daniel raised a very good point about the performance of each solution, and whether it is faster to do instanceof (as it may have to traverse the inheritance tree) instead of calling property_exists() or even maybe trying to switch to $node->value ?? checks. I developed a super simple benchmark to verify which option would be the best pick.

I'll abandon those.

I wouldn't worry about the function call overhead, as it's only a couple extra ops - and IMHO it's more a micro-optimization that we wouldn't be able to notice in the runtime. And I hope that in such cases compiler could optimize the code by just fetching the variable instead of doing a function call. Some languages already do that.

It has to be something related to the CI configs, I'll look for someone who could help me reproduce this problem locally. I followed @Jdforrester-WMF advice and I loaded a specific set of extensions

wfLoadExtension('EventBus');
wfLoadExtension('EventLogging');
wfLoadExtension('EventStreamConfig');
wfLoadExtension( 'WikimediaMessages' );
wfLoadExtension( 'WikiLambda' );

After a bit of research and thinking about this. There are couple of possible ways of getting rid of the tech debt:

@Jdforrester-WMF - can you link me to the tests that fail?

Could be, I didn't have the EventBus extension locally and didn't test it. But yesterday I started working on EventBus patch too, I'll have it ready today

My understanding is that JobRunner uses overrideRequestId() with a null to reset the RequestId - but then after the reset - getting the id will try to get the ID from headers again, and if not present it will regenerate it.

I'll look into it

In T330747#9079207, @Bennylin wrote:

Hmm.. is it possible to use the technology from ws-export.wmcloud.org that are able to compile the contributors (down to the contributors of templates and images that are being used). The alternative is to provide a link to the history page, as an acknowledgement. If it can provide a link "Retrieved from ...so-and-so...", then it shouldn't be hard to add "Click here to see all the contributors of this page"

Thank you for your cooperation in this matter. Ticket closed.

as @TheDJ says - Proton - our PDF renderer disables the Javascript when rendering the page: https://github.com/wikimedia/mediawiki-services-chromium-render/blob/76161ba764e28e0f4f494d903d41d875a64481e6/lib/renderer.js#L157

The current PDF renderer (Proton) provides a way to render Desktop and Mobile versions of PDF article. This is doable by asking Proton to render mobile version of article - which will make fonts much bigger (if we render a full a4 page with the regular font on a mobile device - the text is not readable). IMHO it would partially solve the problem as we would have a way to render a PDF with much bigger fonts for people who are visually impaired.

The current PDF renderer provides two types of output it can render a regular PDF and a Mobile Version of PDF which is much easier to read on mobile devices.

The output of the PDF renderer is cached only for 10 minutes to prevent mass rendering of the same article/regenerating the same content. PDFs are generated by a standalone service called Proton, and the implementation from the beginning didn't listen to purge events.

Currently, the rendered output of "Download as PDF" is cached on Varnish for 10 minutes. I don't think we have a place to store PDF files. Probably, this ticket refers to an old combination of Collection + electron-renderer which is not supported any more.

I'll close this ticket as it refers to a combination of old Collection extension and most likely the electron-render renderer. Right now we use https://www.mediawiki.org/wiki/Proton to generate PDFs and links.

"Download as PDF" uses a Chromium headless browser to save the generated MediaWiki page as PDF. The result is identical to going to the wiki page and hitting print, and then "Destination: Save as PDF". Because the Wiki page does not contain a list of contributors, it's not present in the PDF file.

@Krinkle had a pretty good question regarding the wgAllowExternalReqID guard.

Yesterday I confirmed with @Vgutierrez and @daniel that instead of building/creating the etag, we can just passthrough the LastModified - which btw is a better way. I'm going to update this phab ticket to reflect the current state.