Page MenuHomePhabricator

Community Updates module impressions lack experiment and variant information
Closed, ResolvedPublic5 Estimated Story PointsBUG REPORT

Description

Steps to replicate the issue (include links if applicable):

  • Navigate to Special:Homepage on a wiki with a community update set up (eg: eswiki beta, testwiki, eswiki)
  • Make sure to be in the community-updates-module experiment variant ( can be forced using ge.utils.setUserVariant('community-updates-module') from JS console)
  • Inspect http request and lookup for analytics requests with meta.stream: mediawiki.product_metrics.homepage_module_interaction
  • Inspect the event payload

What happens?:
The event contains an experiments property but it's empty, eg: { experiments: assigned: {}, enrolled: [] }

What should have happened instead?:
The event contains an experiments property with correct information, eg:

{
	"experiments": {
		"assigned": {
			"growth-experiments": "community-updates-module"
		},
		"enrolled": [
			"growth-experiments"
		]
	}
}

Software version (on Special:Version page; skip for WMF-hosted wikis like Wikipedia):

Other information (browser name/version, screenshots, etc.):
The experiment variant data is correctly added to other stream events. This only affects the Metrics Platform client instantiated in ext.growthExperiments.Homepage.Logger/useInstrument.js.

This has impacted the analysis in T378251, luckily there was a fallback instrument for the same relevant actions so the analysis should be doable.

Event Timeline

Change #1101840 had a related patch set uploaded (by Phuedx; author: Phuedx):

[operations/mediawiki-config@master] Beta Cluster: Enable MetricsPlatform extension on all wikis

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

Sgs triaged this task as Medium priority.Dec 10 2024, 4:36 PM

Change #1101840 merged by jenkins-bot:

[operations/mediawiki-config@master] Beta Cluster: Enable MetricsPlatform extension on all wikis

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

Mentioned in SAL (#wikimedia-operations) [2024-12-10T21:48:31Z] <cjming@deploy2002> Started scap sync-world: Backport for [[gerrit:1101840|Beta Cluster: Enable MetricsPlatform extension on all wikis (T381849 T381853)]]

Mentioned in SAL (#wikimedia-operations) [2024-12-10T21:52:45Z] <cjming@deploy2002> cjming, phuedx: Backport for [[gerrit:1101840|Beta Cluster: Enable MetricsPlatform extension on all wikis (T381849 T381853)]] synced to the testservers (https://wikitech.wikimedia.org/wiki/Mwdebug)

Mentioned in SAL (#wikimedia-operations) [2024-12-10T21:59:22Z] <cjming@deploy2002> Finished scap sync-world: Backport for [[gerrit:1101840|Beta Cluster: Enable MetricsPlatform extension on all wikis (T381849 T381853)]] (duration: 10m 50s)

On closer inspection, the extension not being enabled everywhere was a red herring. The bug is here – the Metrics Platform Client is overriding the experiment enrollment information that you're providing with our own empty ("null") enrollment information. This should be a quick fix :)

@cjming @Sfaci: Looking at the instrument code that @Sgs has linked to, I propose that we add a temporary, unstable API to the Metrics Platform JavaScript Client like:

interface MetricsClient {

    /**
     * Adds experiment enrollment information to all events submitted to the given stream.
     * 
     * The enrollment information will be merged into the enrollment information managed by the
     * Metrics Platform.
     *
     * @unstable
     */
    setExperimentEnrollment(
        experimentName: string,
        featureName: string,
        featureValue: any
    );
}

interface Instrument {

    /**
     * Adds experiment enrollment information to all events.
     *
     * The enrollment information will be merged into the enrollment information managed by the
     * Metrics Platform.
     *
     * @unstable
     */
    setExperimentEnrollment(
        experimentName: string,
        featureName: string,
        featureValue: any
    );
}

The MetricsClient class would then fold the given information into the return value of Integration#getCurrentUserExperiments(). What do you think?

… alternatively, in the spirit of keeping things simple, we could update the Metrics Platform Client to check if there's experiment enrollment information in the event and merge it with the return value of Integration#getCurrentUserExpeirments() if so?

alternatively, in the spirit of keeping things simple, we could update the Metrics Platform Client to check if there's experiment enrollment information in the event and merge it with the return value of Integration#getCurrentUserExpeirments() if so?

@phuedx I like that way. It would be really simple. We only should add a check to know if there is already an experiment fragment within the action data to avoid calling getCurrentUserExperiment and use that data. Considering this change as temporary, it would be also transparent regarding adding and remove it.

concur - simple fix sounds good to me too

The simple fix implementation and a related unit test case to test it is ready in the following MR: https://gitlab.wikimedia.org/repos/data-engineering/metrics-platform/-/merge_requests/78

Change #1102840 had a related patch set uploaded (by Santiago Faci; author: Santiago Faci):

[mediawiki/extensions/EventLogging@master] lib: Update lib/metrics-platform to 491eba5916f7

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

Milimetric set the point value for this task to 5.Dec 19 2024, 4:20 PM

@phuedx do we need to backport this change to 1.44.0-wmf.8 ? I can do this on Monday 12/23 if so

Change #1102840 merged by jenkins-bot:

[mediawiki/extensions/EventLogging@master] lib: Update lib/metrics-platform to 491eba5916f7

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