In T365473#10084886, @cjming wrote:

Renaming contextual_attributes and propagating that change throughout all the schemas and client libraries and mpic and current instruments in prod feels like adding to an already full plate atm which is also why I'm fine with leaving it as is and giving it a clear definition in docs. But maybe there's never a good time to do a name change like this and better now than later when we have dozens more instruments in prod.

Thanks, @Sfaci. I'll rebase and finish the TODOs on !39

Moving to Code Review / Tech Input for code review of MR 42.

While I originally settled on contextual attributes, my preferences are: 1) context fields; and 2) context values. I have a slight preference for 1 because it matches the verbiage when talking about events and event schemas (events have fields, event schemas have fields).

Are they still called contextual attributes in the current version of Metrics Platform?

• plus performance, and scalability evaluation to the best of our ability.

MR 39 is blocked on MR 42, which is now ready for review (thanks for the ping @Sfaci!). I'll update the task description…

So the only way that we ensure consistency of assignment output sans backend store is by locking all the inputs.

In T372368#10069353, @Aklapper wrote:

See https://www.mediawiki.org/wiki/Phabricator/Project_management#Types_of_Projects for our taxonomy.

• Can we successfully bucket users based on traffic?

In T372368#10061580, @Jdforrester-WMF wrote:

In T372368#10060455, @Aklapper wrote:

https://phabricator.wikimedia.org/project/profile/5324/ already exists, so I'd rather call this #mediawiki-extensions-MetricsPlatform to avoid confusion?

I think maybe that project is wrongly-configured? It appears to be a team backlog, but it's got the icon and colour of a component.

Usecases examined

After a little more thought, we can combine the initial approach (fetching and caching the config in a per-DC cache on a loop) with the above approach to ensure that we don't contact Statsig whilst serving the page.

@cjming: I've added a bunch of comments and taken the liberty of updating the example CTR instrument. Would it be helpful to pull out all of the open questions into a separate section so that they can be addressed (or converted into Phab tasks, emails, etc)?

I think that this could be moved to done and any further deployment/responding to points in the Application Security Review can be done in other tasks /cc @VirginiaPoundstone @WDoranWMF

In T369847#10057294, @phuedx wrote:

We introduce a private (uncacheable), URL-loadable RL module, which: generates an ID, uses it to bootstrap the client SDK, and sets the ID in a cookie for reuse. The ext.metricsPlatform RL module, which is public (cacheable) and URL-loadable, would depend on this module.

Up until now, I've assumed that minimize requests to the server is paramount. If I relax that assumption a little, I can see an alternative that's more flexible (it uses the remote-evaluation SDK, which has fewer cons) with the same control over what gets sent where and the same separation from Statsig:

In T369847#10048270, @phuedx wrote:

I'm going to attempt to use the new SDK and see how far I can get with this.

☝️ I've hit some snags (plural).

In T366802#10025085, @VirginiaPoundstone wrote:

This task needs more detail ;)

In T369687#10043350, @mforns wrote:

In any case, we would have to solve the modification_pct issue, since we don't have any numeric field in the common fragment...
@phuedx Do you think we could add one?

I've reviewed the doc 👍 The note about GrowthBook Proxy was great.

In T369847#10009180, @mpopov wrote:

(The file attachment – a diagram I suspect? – is missing.)

@mpopov: I'm currently thinking about how to wire up the Statsig SDK and a general mechanism for T368326: Update Metrics Platform Client Libraries to accept experiment membership and would appreciate your input.

1. I've had to use the maintenance version of the JavaScript SDK because:
   1. It allows you to inject the Statsig config fetched by the PHP SDKs directly rather than fetching it itself. During testing I found that this wasn't the case for the newer SDK
   2. The new SDK requires that you use a specific build to do local on-device evaluation. Per https://docs.statsig.com/client/js-on-device-eval-client, you require a Enterprise or Pro license to do on-device evaluation with this SDK

• A RESTful API is up and running that receive events and proxies these to a Statsig instance

A search for Next.js download SWC (from download-swc.js in that stacktrace you provided yielded: https://nextjs.org/docs/messages/failed-loading-swc

functional tests

In T368543#9926759, @Lucas_Werkmeister_WMDE wrote:

I’m not really sure how this code could ever have worked:

includes/JsonSchemaContent.php

	public function objectRow( $key, $val ) {
		if ( $key === '$ref' ) {
			$valParts = explode( '/', $val, 2 );
			if ( !isset( $valParts[1] ) ) {
				// Don't store or inject service objects in Content objects
				// as that breaks serialization (T286610).
				$services = MediaWikiServices::getInstance();
				$revId = (int)$valParts[1];
				$revRecord = $services->getRevisionLookup()->getRevisionById( $revId );
				$title = $revRecord->getPageAsLinkTarget();

We’ve already checked that $valParts[1] isn’t set, but then we still cast it to an int, and try to load a revision record from that ID? Is the if condition just flipped from what it should be?

I also don’t see any recent changes to the code, which is confusing.

The problem appears to have been highlighted by this edit to the Schema schema.

In T355055#9976963, @Agamyasamuel wrote:

@phuedx Could you please elaborate the issue little more. would love to work on it.
~ Thanks.

In T365839#9969612, @BTullis wrote:

I suggest that we evaluate FerretDB with a PostgreSQL backend for this project. We can create a PostgreSQL database on our existing bare-metal PostgreSQL servers (an-db100[1-2]) for now, although we could actually leverage our work on the cloudnativepg operator in the near future, if we wish. If there are any issues, we could even use SQLite within a persistent volume for this initial setup period, although I would prefer to use PostgreSQL if possible.

No need for a RegEx. A little DOMDocument magic in BannerRenderer should do:

In T353817#9961942, @Ottomata wrote:

@Joe, to not squat w/beacon.php, in case there are other usages of /beacon that we'd want to route, would this perhaps be better at w/beacon/event.php ?

In T363685#9956753, @ngkountas wrote:

I'm moving this task to "Waiting for deployment" column. @KCVelaga_WMF do you think you could QA this task once the PR is merged and deployed?

In T366627#9950681, @Ottomata wrote:

It would be nice to generate some fake data and run some test queries on it to get an idea for latency added on by filtering on discriminator. I have not had time to do this yet.