Does The Vision™, as currently stated, obviate the need for this @Sfaci ?

In T408186#11319843, @JVanderhoop-WMF wrote:

Does The Vision™, as currently stated, obviate the need for this @Sfaci ?

I'm sorry for answering a bit late. I didn't realize!

I would say that we still need this, even considering The Vision™. We are talking here about experiments being able to use their own combination of contextual attributes as instruments can do thanks to T398480: [Epic] Update instruments configuration for xLab where we defined that a stream could have more than one instrument. This task aims to enable experiments to do that. And this also would prevent teams from creating custom streams only to be able to use a different combination of contextual attributes than the one that base schema provides.

Regarding the xLab UI part, it would die pretty soon because of GrowthBook but the relevant changes in MetricsPlatform and client libraries would survive

the analogous work for instruments: T398480: [Epic] Update instruments configuration for xLab

T409913: Create PHP equivalent of mw.testKitchen.getInstrument() is doing the SDK side of this work, which will simplify this task to be UI specific.

Data will be unified and put into a single iceberg table as part of SDS2.2.6

This is no longer a requirement for the iceberg hypothesis work, but it is a huge source of friction for our users.

cjming opened https://gitlab.wikimedia.org/repos/data-engineering/test-kitchen/-/merge_requests/298

Draft: Configure experiments with stream, schema and contextual attributes

hi @Sfaci @phuedx some questions about experiment endpoints, forms, etc:

• Should stream_name, schema_title, contextual_attributes be included in the api response for all variations? i.e. varnish, mediawiki, analytics, default like they are for https://test-kitchen.wikimedia.org/api/v1/instruments ?
• In the case of custom streams, if they are based off of web base or app base schemas (which we don't currently know/check if they are - do we want to check this? i believe this is not a feature for instruments), presumably we also want to be able to customize contextual attributes for custom streams? This is how the current instrument form works fwiw and afaict
  • This question relates to the previous one in that if we are not checking if custom streams' schemas include web/app base as fragments, and we are outputting contextual attributes in the api response, my gut says we need to check this somewhere:
    • Should it be before we output data in the endpoint?
    • Should it be in the validation of the experiment/instrument forms where if a custom stream is selected, we hide the contextual attributes field (lofi)? Or is there a way to figure out if the custom stream's schema uses a web/app base fragment in the action API (is this over-complicating/engineering the issue?)?
    • Does TK extension need to discern if contextual attributes are relevant for the custom stream when it reads configs?

Should stream_name, schema_title, contextual_attributes be included in the api response for all variations? i.e. varnish, mediawiki, analytics, default like they are for https://test-kitchen.wikimedia.org/api/v1/instruments ?

I was wondering if we only should include stream_name and contextual_attributes and also use /api/v1/experiments (without query parameters) for this purpose:

• schema_title is now included in the response for instruments but it's unused. TestKitchen is setting BASE_SCHEMA by default and SDKs provides setSchema to developers as a way to set a custom one.
• /api/v1/experiments is for now unused (that also means that we have the opportunity to change it if needed) and we could start using it now to fetch all those details for logged-in and logged-out experiments instead of making two different requests with different authorities (varnish + mediawiki endpoints). Varnish doesn't need stream_name and so on and, once the enrollment has been set in wgTestKitchenUserExperiments, at some point we will need to use these configuration details for all the experiments, regardless of they are logged-in or logged-out. I guess we will need to fetch those details when instantiating instruments/experiment in their corresponding InstrumentManager / ExperimentManager (getInstrument() and getExperiment methods). We are already doing something like that for instruments where we need to call configsFetcher->getInstrumentConfigs to know stream_name and contextual_attributes as part of the instrument config.

In the case of custom streams, if they are based off of web base or app base schemas (which we don't currently know/check if they are - do we want to check this? i believe this is not a feature for instruments), presumably we also want to be able to customize contextual attributes for custom streams? This is how the current instrument form works fwiw and afaict

This is not being done right now for instruments, right? I would say it's like a manual check we do when defining/starting to work on the experiment with product teams, right? I would say Mikhail is the one who checks all this when talking with teams during the first phases of the experimentation work. I think your ideas to automate all this are really great but maybe this is not included in the scope of this task and we could consider it as an improvement we can work on later with both instruments are experiment in mind. It seems like same approach would work for both.

Anyway, it doesn't make sense to use an non-app/web based schema for experiments, right?

And what about taking the opportunity to include a field to collect schema_id instead of schema_title for experiments?. schema_title is being collected and included in the instrument configs but, in reality, it's unused (right?) and I would say that schema_id would an interesting field to fill. Developers that use a custom stream and/or a specific schema_id need always to call setSchema but I guess schema_id is pretty defined since the beginning and it's not going to change while the experiment is running, right? That would be something that developers wouldn't have to worry about once the experiment was defined in Test Kitchen UI

Is there a specific reason to avoid using schema_id in Test Kitchen UI? (not sure about this)

thanks @Sfaci ! some responses inline

what about taking the opportunity to include a field to collect schema_id instead of schema_title for experiments?

to confirm - you're talking about in the base /api/v1/experiments response right (not forms)? sounds good to me to output schema_id in lieu of schema_title - seems more useful given the methods in the SDKs. I'll update MR accordingly to include only the new fields in the base response

Is there a specific reason to avoid using schema_id in Test Kitchen UI?

not that I can think of - but are you proposing to include a field in the TK UI? because in theory we already know the schema ids once the experiment owner selects a stream

we could consider it as an improvement we can work on later with both instruments are experiment in mind.

sounds good - i'll file a ticket for the backlog

it doesn't make sense to use an non-app/web based schema for experiments, right?

I can imagine a scenario where we support custom experiments that use schemas based off of the web/app base schemas (for whatever reason needing to collect more data than what is offered in web/app base) - that is what I'm alluding to in my questions -- but as you say, maybe for now we just support experiments using web/app base schemas

to confirm - you're talking about in the base /api/v1/experiments response right (not forms)? sounds good to me to output schema_id in lieu of schema_title - seems more useful given the methods in the SDKs. I'll update MR accordingly to include only the new fields in the base response

Yes! I mean /api/v1/experiments as a way to have all the details about all experiment in only one endpoint. So far we didn't know to fetch logged-out experiment configs from Test Kitchen API but now TestKitchen extension will need to fetch those details configure them "more automatically" with all the details provided by experiment owners via Test Kitchen UI.

I'm also wondering if we should just add a new query parameter to mean this new case, to take the opportunity to create a smaller response for this work. /api/v1/experiments response is pretty verbose and I would say there are more details than the ones we need, right? What about https://test-kitchen.wikimedia.org/api/v1/experiments?format=config&authority=varnish,mediawiki?

not that I can think of - but are you proposing to include a field in the TK UI? because in theory we already know the schema ids once the experiment owner selects a stream

Once a stream is selected we only know the schema_title (for example: analytics/product_metrics/web/base) but not the schema_id (for example: analytics/product_metrics/web/base/2.0.0, where the exact version is also included). So schema_id = schema_title + version

That's why schema_title (the one included in the API response for instruments) is not being used. The useful one is the schema_id because it includes also the version of the corresponding schema_title. It's what instruments/experiments use effectively to express which schema version will validate their events

If you take a look at product_metrics.web_base stream definition you will see what I mean

I can imagine a scenario where we support custom experiments that use schemas based off of the web/app base schemas (for whatever reason needing to collect more data than what is offered in web/app base) - that is what I'm alluding to in my questions -- but as you say, maybe for now we just support experiments using web/app base schemas

I see!. Like the ones are used for instruments because base schemas seem to be insufficient for some reason. You are right! Anyway, I would say it's also part of the improvement mentioned before, right?

lol - i was in the middle of re-writing my responses when you responded - thanks @Sfaci

re: capturing schema_id

As a counterpoint to my initial response, my hesitation with this is validation -- how do we make sure the experiment owner provides a valid schema id? Can we just trust that the experiment owner will input a valid version number for the schema title (which we get automatically from the stream name via action api as you pointed out)?

Maybe there's a way to grab valid schema ids - seems complicated but probably worth investigating. For a lofi iteration, we could just provide an optional text field that allows an experiment owner to provide a specified version number or else it just defaults to latest/current if the value is empty or invalid?

For reference - this is the meta api endpoint that the action api uses - https://meta.wikimedia.org/w/api.php?action=streamconfigs&format=json&formatversion=2&all_settings=1&constraints=destination_event_service%3Deventgate-analytics-external << this only provides schema_title.

Do we also want to capture/provide schema_id or schema_version for the instrument form too?

For a simpler, more expedient approach, we could leave the stream name field as is since it already maps to the schema title and assume it's the most current version? Since the SDKs provide setStream and setSchema methods on the experiment object (are these methods temporary or stable? i guess it depends on this), if experiment owners need to specify an alternative schema version number, they could do it in their feature code right? This pre-empts us having to mess with the component, field validation, etc.

re: query param for new experiments endpoint

bikeshedding opp: what about https://test-kitchen.wikimedia.org/api/v1/experiments?format=config&authority=web ?

re: future improvements

agree agree -- it's getting scope-creepy in here

re: capturing schema_id

Yes, you are right! Validation would be something to consider as a non-easy thing. Maybe we are increasing again the scope here and we should focus on what we have for instruments, at least for now? And, why not?, start investigating/considering how to deal with schema_id for instruments and experiments as a future improvement. So far there were no issues/questions related to that so maybe it's not a critical feature

re: query param for new experiments endpoint

After thinking a bit more about the API endpoint, I was wondering if we shouldn't use the authority query parameter to just mean "fetch all experiment configs". In the end, we are not dealing with enrollment, we only want to fetch experiment configs to do something different. I think we should only use the authority query parameter for its specific purpose. I'm considering again my initial proposal that was to use /api/v1/experiments (In fact it's the equivalent API endpoint we use to fetch instrument configs. It would make sense to do something similar for experiments). Maybe we can simplify the response by removing all the non-technical details like descriptions and so on, and use it as a way to fetch all experiment configs for purposes like the one we are talking about

Maybe there's a way to grab valid schema ids

Hm, not schema_ids directly, but you could construct from
https://schema.wikimedia.org/repositories/secondary/jsonschema/analytics/product_metrics/web/base/

Either by iterating over each file and prefixing the schema_title (a little hacky), or requesting each file (e.g. https://schema.wikimedia.org/repositories/secondary/jsonschema/analytics/product_metrics/web/base/1.0.0) and extracting the $id field.

Or you could (regularly? on deploy) include clones of the schema repos in your service and just fetch them from the filesystem.
https://wikitech.wikimedia.org/wiki/Event_Platform/Schemas

WMF has chosen to distribute schemas using Git. This allows us to do development, CI, versioning and deployment for schemas the same way we do any code project

Or you could (regularly? on deploy) include clones of the schema repos in your service and just fetch them from the filesystem.

https://wikitech.wikimedia.org/wiki/Event_Platform/Schemas

Yes, this is something I have been thinking about, we could do this by adding secondary schemas repo as a submodule of Test Kitchen UI's. That's what we do for experiment-analytics-config and canonical-data where the cadence of updates is pretty low, and having to deploy a new release when needed is not a big deal. I guess that something like that allow us to check whether a specific schema_id exists and it is doable (but maybe out of scope for this ticket). Although, to keep that submodule updated, we would need to be aware of updating the corresponding submodule manually and deploying a new release every time a team has defined a new schema that is going to be used by an upcoming instrument/experiment. We are already having that "issue" and that's why we filed T406604: Test Kitchen UI: include updating submodules as part of CI to automate a bit that at some point for the two submodules that Test Kitchen is already using. That would simplify the specific deployments

I think allowing instrument and experiment owners to specify schema ID (title + version) would be an improvement overall but it needs thought from various perspectives. It could reduce friction during the initial setup of an instrument. It could increase friction during iteration though (a developer tweaks the instrument and has to remember to update the config in Test Kitchen UI to use the latest version).

I agree that we should do it in the same way that we handle the experiment-analytics-config repo – make the repo a submodule, update it regularly, and process it on startup, extracting the information that we need.

Given the additional thought and engineering work, I'd say that this should be a separate task for now.

In T408186#11643459, @cjming wrote:

• Should stream_name, schema_title, contextual_attributes be included in the api response for all variations? i.e. varnish, mediawiki, analytics, default like they are for https://test-kitchen.wikimedia.org/api/v1/instruments ?

The TestKitchen extension and Automated Analytics system are resilient to new fields being added. They are also owned and maintained by us.

The Varnish config fetcher is less resilient to new fields being added and is not owned or maintained by us.

I would suggest that the format and authority parameters don't quite make sense any more. This is because we're going to have to do additional work in MediaWiki on behalf of the Varnish enrollment authority – we're going to merge enrollment information from Varnish with stream and contextual attributes configuration from TKUI. That is, MediaWiki is going to have to know about all experiments but won't be the authority for all of them.

@Sfaci mentioned updating /api/v1/experiments to include stream_name and contextual_attributes above and I think that's a good first step. We would need to update the TestKitchen extension to use that endpoint too, i.e.

Change #1250108 had a related patch set uploaded (by Clare Ming; author: Clare Ming):

[operations/deployment-charts@master] Test Kitchen UI: Deploy v1.2.5 release to staging

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

Change #1250109 had a related patch set uploaded (by Clare Ming; author: Clare Ming):

[operations/deployment-charts@master] Test Kitchen UI: Deploy v1.2.5 release to production

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

Change #1250108 merged by jenkins-bot:

[operations/deployment-charts@master] Test Kitchen UI: Deploy v1.2.5 release to staging

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

Change #1250109 merged by jenkins-bot:

[operations/deployment-charts@master] Test Kitchen UI: Deploy v1.2.5 release to production

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

Change #1251216 had a related patch set uploaded (by Clare Ming; author: Clare Ming):

[mediawiki/extensions/TestKitchen@master] Send experiment events with specified config from Test Kitchen

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

hi @Sfaci @phuedx -- I think the schema_id issue needs to be resolved sooner than later. In the patch for the TK extension using the new endpoint to build the enrollment results, I'm running into the same problem of how to pass in the proper schema_id for experiment configs.

I wrote out my question in Gerrit and then figured it's probably easier to settle on an approach here in Phab. This is the gist of the issue:

ExperimentManager::getExperiment() calls ExperimentManager::getExperimentConfig() which I updated to include schema_id as a key. Since Experiment::send() needs the schema id from experiment configs, it seemed like it needed to be included in ExperimentManager but we don't actually have a proper way to grab the schema id from StreamConfigs.

The schema_id in the patch's experiment configs is actually just the schema_title derived from a new method I also added called StreamConfigs::getSchemaIdForStream() which grabs the schema_title from meta's api.

Andrew posted some options above (T408186#11651079) as did Santi (T408186#11651618) of how to get the actual schema id and I'm thinking we should do this to simplify TK extension and to actually provide the schema id in experiment configs.

If we're able to pass in schema_id in the json response, then we don't have to do any processing of stream configs in TK extension at all.

Anyway if there are other ideas on how to do this, i'm all ears. But my gut says it's better for it to come thru TKUI's api endpoint rather than TK extension having to figure out how to match the stream name with the schema id.

I'm inclined to update TKUI with Santi's approach unless I'm missing that there is an expedient way to grab schema ids by stream name in TK extension. Thoughts?

@cjming: I've responded on Gerrit as well as leaving some other comments to think about. In short, we currently have Experiment#setSchemaID(), which should be enough to get the initial implementation of this deployed. The title of this task may well include "schema" but I'm not sure that we understood what would be involved in implementing that when we estimated this task a 5. Let's discuss the options as a team during standup?

cjming opened https://gitlab.wikimedia.org/repos/data-engineering/test-kitchen/-/merge_requests/309

Add user_identifier_type to base experiment api response

sfaci merged https://gitlab.wikimedia.org/repos/data-engineering/test-kitchen/-/merge_requests/309

Add user_identifier_type to base experiment api response

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

[operations/deployment-charts@master] Test Kitchen UI: Deploy v1.2.7 release to staging

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

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

[operations/deployment-charts@master] Test Kitchen UI: Deploy v1.2.7 release to production

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

Change #1259850 merged by jenkins-bot:

[operations/deployment-charts@master] Test Kitchen UI: Deploy v1.2.7 release to staging

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

Change #1259851 merged by jenkins-bot:

[operations/deployment-charts@master] Test Kitchen UI: Deploy v1.2.7 release to production

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

Change #1251216 merged by jenkins-bot:

[mediawiki/extensions/TestKitchen@master] Send experiment events with specified config from Test Kitchen

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

Change #1265525 had a related patch set uploaded (by Clare Ming; author: Clare Ming):

[operations/puppet@production] Update the Test Kitchen maintenance script to target testwiki

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