[go: up one dir, main page]

Page MenuHomePhabricator

[beta cluster] Enable new Kartographer Nearby on minerva
Closed, ResolvedPublic3 Estimated Story Points

Description

To test the new nearby feature on mobile, we want to enable for the minerva skin on beta. This means creating a separate feature flag to skip the guard condition to limit nearby to the desktop skin. Do not enable on pilot wikis.

New feature flag: $wgKartographerNearbyOnMobile

Demo link: https://en.m.wikipedia.beta.wmflabs.org/wiki/Maptests#/map/4

Event Timeline

ECohen_WMDE renamed this task from Test nearby in mobile skin on beta to Enable nearby in minerva on beta only.Oct 13 2022, 10:54 AM

Could the description of this ticket be expanded? Is this about https://en.m.wikipedia.org/wiki/Special:Nearby (this is accessible in Minerva) ?

Bump!

For more context, if this is a new feature I would insist on a performance review before exposing this feature to mobile users. We have had a lot of performance problems with map related features in the past. In addition to this there are many untriaged Kartographer bugs relating to JS errors in production that I would consider blockers to rolling out such a feature at a greater scale. Happy to share those if helpful.

awight renamed this task from Enable nearby in minerva on beta only to [beta cluster] Enable new Kartographer Nearby on minerva.Oct 19 2022, 8:38 AM
awight updated the task description. (Show Details)

@Jdlrobson This is about a new "nearby" layer in Kartographer full-screen maps, I've linked the task description to our public project page for more information.

There certainly could be performance issues, but my feeling is that the feature is sufficiently well-hidden and needs to be explicitly enabled by the user each time they want to see the layer, so we don't expect there to be any impact on the general readership. At this phase of our feature, we're only adding approximately 50 points when the layer is turned on, which is a small overhead relative to the rest of Kartographer. I completely agree that we should be wary of this extension, so please do continue to keep an eye on it!

@Jdlrobson This is about a new "nearby" layer in Kartographer full-screen maps, I've linked the task description to our public project page for more information.

Thanks for expanding! This is all useful context!

I'm assuming that tile server performance will not be a problem, so mostly focusing on the mobile side of things.

  • Could you point me to the code that loads the feature? Is it limited to a user preference? What does the bootstrap code look like? How does it impact first paint? What dependencies does it add to page startup?
  • How many bytes do you expect this change will add to the critical path?
  • Is this logged in only or it available to anonymous users?
  • What percentage of pages will have the feature available?
  • How much usage does the feature get now, and how much will that increase by?
  • There are various production errors for this extension. Most notably, T297848 and T297519 remain unopened and triaged as high priority and are mostly occurring on mobile. If those volumes increase drastically, what is your plan for rolling back? Have you allocated time to address Kartographer production errors?
  • It seems there have been a lot of additions built in OOUI for this feature. What are the plans for migrating this to Vue/Codex?
  • Could you point me to the code that loads the feature?

The bulk of the functionality is implemented by nearby.js, which is
loaded as part of the ext.kartographer.dialog module.

What does the bootstrap code look like? How does it impact first paint? What dependencies does it add to page startup? How many bytes do you expect this change will add to the critical path?

Nothing new is added to the critical path, these modules are only bootstrapped when a map is opened in full-screen mode, by a call to mw.loader.using: https://gerrit.wikimedia.org/r/plugins/gitiles/mediawiki/extensions/Kartographer/+/refs/heads/master/modules/box/Map.js#530 .

Example of full-screen mode: https://en.wikipedia.beta.wmflabs.org/wiki/User:Adamw/sandbox#/map/3

Our new feature is embedded within existing code, we've simply added a new packageFile to support the "Show nearby articles" button.

Is it limited to a user preference?

No, it's enabled by a temporary server configuration feature flag $wgKartographerNearby.

  • Is this logged in only or it available to anonymous users?

All users can access the feature.

  • What percentage of pages will have the feature available?

We have detailed breakdowns of the prevalence of mapframes, I'm inviting you to an internal background document. Roughly 0.3% of enwiki pages include a Kartographer map, and 10% of enwikivoyage pages.

All mapframes on a wiki where we have enabled the feature flag will have the nearby feature available.

  • How much usage does the feature get now, and how much will that increase by?

We're working on a Superset dashboard, please follow progress in T318928.

  • There are various production errors for this extension. Most notably, T297848 and T297519 remain unopened and triaged as high priority and are mostly occurring on mobile. If those volumes increase drastically, what is your plan for rolling back? Have you allocated time to address Kartographer production errors?

We can simply disable the "nearby" feature flag if we run into problems. Maps will gracefully degrade back to their previous state. Nothing can be built on top of the nearby feature so such a rollback has low impact and there are no complications.

Our team only has a mandate to work on Kartographer for two more months. We've spent some time looking at errors, and have already reduced the number of kartotherian server errors several-fold. Unfortunately, the service is quite fail-y—in T317766 we found something around a 5-15% error rate. I would love to see more investment in fixing the many different classes of error, but we can't commit to it. Please feel free to contact me privately if you want to talk more about this aspect!

  • It seems there have been a lot of additions built in OOUI for this feature. What are the plans for migrating this to Vue/Codex?

Due to the lack of resources hinted at above, there are currently no plans to migrate to Codex. Creating this epic (T321387: Migrate Kartographer UI to Codex) would be a good next step, and we should link all the tasks which talk about "convert *to* OOUI" which are now outdated.

It seems there have been a lot of additions built in OOUI for this feature. What are the plans for migrating this to Vue/Codex?

I'm confused, have there ? Isn't this entire functionality just one more OOUI button within all the other pre-existing OOUI elements of Kartographer ?

The only other element being pogs and (pre-existing) popups which both are extensions to the leaflet library and UI ? It's not like we would ever convert leaflet UI to Codex and/or vue, right ?

It seems there have been a lot of additions built in OOUI for this feature. What are the plans for migrating this to Vue/Codex?

I'm confused, have there ? Isn't this entire functionality just one more OOUI button within all the other pre-existing OOUI elements of Kartographer ?

I took this question as referring to Kartographer in general, but I think you're right to emphasize that the subject of this task is a much smaller "nearby" feature. What we want to know here is whether "nearby" will further drag down an already heavyweight maps interface.

The only other element being pogs and (pre-existing) popups which both are extensions to the leaflet library and UI ? It's not like we would ever convert leaflet UI to Codex and/or vue, right ?

I've linked a few tasks under the new epic (T321387: Migrate Kartographer UI to Codex), but yes the topic has come up of integrating POG and leaflet behind a bit more of a standardized wiki GUI. It seems like a good idea, not to port the entire thing but to better harmonize the external features as we normally do. I can also imagine direct reuse of components such as T239935: Add a coordinate picker to Codex.

Thanks for the internal background document.

Nothing new is added to the critical path, these modules are only bootstrapped when a map is opened in full-screen mode, by a call to mw.loader.using: https://gerrit.wikimedia.org/r/plugins/gitiles/mediawiki/extensions/Kartographer/+/refs/heads/master/modules/box/Map.js#530 .

and that's great to hear!

As far I can see Kartographer was responsible for 6430 JavaScript errors in the last 7 days, https://logstash.wikimedia.org/goto/d475b3a68f52bd0eacddbb6826be3856 (known bugs are T269376,T265919,T297519,T297848). I'd appreciate if you keep an eye on those numbers as you roll out the feature to further places and make sure they only modestly increase as otherwise my team will be getting hourly alerting emails.

Change 875294 had a related patch set uploaded (by Awight; author: Awight):

[mediawiki/extensions/Kartographer@master] Feature flag for Nearby on mobile

https://gerrit.wikimedia.org/r/875294

Change 875294 merged by jenkins-bot:

[mediawiki/extensions/Kartographer@master] Feature flag for Nearby on mobile

https://gerrit.wikimedia.org/r/875294

Change 875460 had a related patch set uploaded (by Awight; author: Awight):

[operations/mediawiki-config@master] [beta] Enable Kartographer "nearby" on mobile skin

https://gerrit.wikimedia.org/r/875460

Change 875460 merged by jenkins-bot:

[operations/mediawiki-config@master] [beta] Enable Kartographer "nearby" on mobile skin

https://gerrit.wikimedia.org/r/875460

awight removed awight as the assignee of this task.Jan 5 2023, 8:23 AM
awight updated the task description. (Show Details)
awight moved this task from Tech Review to Demo on the WMDE-TechWish-Sprint-2023-01-04 board.