1760255 - Events pings for rally glean studies missing metrics.uuid.rally_id

Status: RESOLVED FIXED

(Data Platform and Tools :: General, defect)

Description

[Wesley Dawson [:whd]]

The most recent schemas deploy appears to have added a generic events ping for some glean products: https://github.com/mozilla-services/mozilla-pipeline-schemas/commit/5515deb59a818d58473dcd640108519ccb2b31b7

This includes the rally ones, which are missing metrics.uuid.rally_id. I'm not sure which part of the stack is generating these ping definitions, but the rally ones are required to have metrics.uuid.rally_id for certain views to be created for deletion requests. I would also guess that for rally studies we may not want these standard/auto-generated pings at all, but I'm not sure.

I'm CCing a couple of people who might be more knowledgeable about where this got added and how we might either (a) ensure rally studies get metrics.uuid.rally_id added, (b) disable these pings for rally studies, or (c) something else. (b) would probably require entries be added to https://github.com/mozilla/mozilla-schema-generator/blob/main/incompatibility-allowlist, but the schemas deployment failed in stage so we won't need to delete/recreate any prod tables.

Comment 1

[Alessio Placitelli [:Dexter]]

The schemas are generated because the latest version of the Glean JS SDK added a feature that was missing, the ability to send an events ping.

The schema changes for the Glean part are correct and these pings must not be disabled for Rally products (because they can't and we don't want to add any special flavour to it). Moreover, aside from the schema changes breaking deployment (which I believe they shouldn't), Rally is not using the most recent version of the Glean JS SDK (because we didn't cut a new release yet) and, as such, it can't be sending such "events" ping yet even if it wanted.

I believe the right course of action here is making sure that whenever the "rally.id" metric is defined in a product, e.g. core addon, the markup then the Rally team makes sure to also add "events" to the "send_in_pings" of the "rally.id" definition.

@Bea, what do you think?
@Ted/@jepstein , this is mostly to make you aware.

Comment 2

[Beatriz Rizental [:brizental]]

I agree with everything Alessio said. I have two more things to note:

1. Maybe the moz-pipeline-schemas should be fetching commits from the release branch on the mozilla/glean.js repository so that changes which are not released yet don't break anything. I am happy to make that change if it is possible / makes sense. Let me know.

2. With regards to:

I believe the right course of action here is making sure that whenever the "rally.id" metric is defined in a product, e.g. core addon, the markup then the Rally team makes sure to also add "events" to the "send_in_pings" of the "rally.id" definition.

This is a good way to move forward, for now. I'd say it might be beneficial to go ahead and include both events and default in the send_in_pings list there, because then in case we ever add a metrics-like ping to the Glean JavaScript SDK, Rally is already prepared for it and we don't get errors like this again.

This is not really a future proof solution, though. The rally-id seems like another good candidate for a potental all-pings metric. This is not possible with the Glean SDKs as of yet, but there are ongoing discussions to expose this feature. I'll link to that bug.

Comment 3

[Wesley Dawson [:whd]]

Thanks for the thoughts on how to proceed. I'm going to stop the bigquery deployment pipeline for rally until this bug is resolved.

Comment 4

[Jonathan Epstein]

(In reply to Alessio Placitelli [:Dexter] from comment #1)

I believe the right course of action here is making sure that whenever the "rally.id" metric is defined in a product, e.g. core addon, the markup then the Rally team makes sure to also add "events" to the "send_in_pings" of the "rally.id" definition.

Thanks for the @. We can take care of this.

Comment 5

[Alessio Placitelli [:Dexter]]

(In reply to Jonathan Epstein from comment #4)

(In reply to Alessio Placitelli [:Dexter] from comment #1)

I believe the right course of action here is making sure that whenever the "rally.id" metric is defined in a product, e.g. core addon, the markup then the Rally team makes sure to also add "events" to the "send_in_pings" of the "rally.id" definition.

Thanks for the @. We can take care of this.

Thanks. Please let us know once you've addressed this so that we can resume schema deployments for Rally products.

Comment 6

[Jonathan Epstein]

It should be noted that none of our studies are currently sending pings using the event type

Comment 7

[Robert Helmer [:rhelmer]]

I've opened PRs for these:
https://github.com/mozilla-rally/rally-core-addon/pull/804
https://github.com/mozilla-rally/facebook-pixel-hunt/pull/119

And also for this upcoming study that does want to use the events type (so comment #1 is helpful thanks):
https://github.com/mozilla-rally/search-engine-usage-study/pull/77

Comment 8

[Alessio Placitelli [:Dexter]]

Great, thanks Rob! Note that this is not about the event type, but rather about the events ping.

The difference being that event metrics can go in any ping, but their default transport is the events ping (unless Glean is told otherwise). Products using the Glean SDK could send event(s) in custom pings before Glean 1.0, but with Glean 1.0 we now have feature parity and so the JS SDK also offers a default transport for them.

Comment 9

[Wesley Dawson [:whd]]

The schemas changes appear to have landed in https://github.com/mozilla-services/mozilla-pipeline-schemas/commit/c9236f5adba6aa5cf56c0cb32bdcc5f995dd49a1.

However, this issue affects rally-study-zero-one as well, since an events ping was added automatically there. Schemas deployments are therefore still failing in stage, and rally-study-zero-one's schemas will need the corresponding update.

Comment 10

[Alessio Placitelli [:Dexter]]

Hey Rob,

would you kindly also address :whd's previous comment?

Comment 11

[Robert Helmer [:rhelmer]]

(In reply to Alessio Placitelli [:Dexter] from comment #10)

Hey Rob,

would you kindly also address :whd's previous comment?

OK I've opened a PR for this: https://github.com/mozilla-rally/rally-study-01/pull/137

The others from comment 7 have landed in the meantime, please let me know if we've missed anything. Thanks for the heads up!

Comment 12

[Wesley Dawson [:whd]]

https://github.com/mozilla-rally/rally-study-01/pull/137 landed today and schemas have been deployed.

Comment 13

[Alessio Placitelli [:Dexter]]

(In reply to Wesley Dawson [:whd] from comment #12)

https://github.com/mozilla-rally/rally-study-01/pull/137 landed today and schemas have been deployed.

Thank you, Wesley