Closed Bug 1884795 Opened 11 months ago Closed 11 months ago

Enable new Glean App `accounts_cirrus`

Categories

(Data Platform and Tools :: Glean Platform, task)

task

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: clouserw, Assigned: mwilliams, NeedInfo)

Details

Attachments

(1 file)

I think I'm doing this right, but please don't assume I know what I'm doing. My goal is to connect Mozilla Accounts with Nimbus and I'm following these directions. Mozilla Accounts was originally added to Glean in this patch.


Application ID*: accounts_cirrus
Application Canonical Name: Mozilla Accounts (Cirrus)
Description: Mozilla accounts is Mozilla's authentication solution for account-based end-user services and features.
Data-review response link:
Repository URL: https://github.com/mozilla/fxa
Locations of metrics.yaml files (can be many):

  • packages/fxa-shared/metrics/glean/fxa-ui-metrics.yaml
  • packages/fxa-shared/metrics/glean/fxa-backend-metrics.yaml
    Locations of pings.yaml files (can be many):
  • packages/fxa-shared/metrics/glean/fxa-ui-pings.yaml
  • packages/fxa-shared/metrics/glean/fxa-backend-pings.yaml

Dependencies**:

  • glean-core

Retention Days***: 760

%23%23 Notes and guidelines

* This is the identifier used to initialize Glean (or the id used on the store on Android and Apple devices).

** Dependencies can be found in the Glean repositories. Each dependency must be listed explicitly. For example, the default Glean probes will only be included if glean itself is a dependency.

*** Number of days that raw data will be retained. A good default is 400. We can change this later to accommodate longer retention periods, though we cannot recover data that is past the retention period (for example, we cannot recover data that is 200 days old if your retention period is 180 days).

%23%23 Need additional help%3F
If you need new dependencies, please file new bugs for them, separately from this one. For any questions, ask in the %23glean channel.

%23 To be filled by the Glean team
Application friendly name: my_app_name

Hey Wil,

looks like all the metrics files you are trying to add were already added to the accounts_frontend and accounts_backend, so there's nothing new to add as part of this bug.

To make sure we're able to help you, we need to understand a bit more what you're trying to achieve. You linked the Glean docs to enable a new product, but you're also mentioning Cirrus. What documentation are you following to enable Cirrus? Who is supporting you on the Nimbus side?

Flags: needinfo?(wclouser)

Thanks for the reply. I have a checklist I'm following and I probably misunderstood something on it. @yashika is supporting from the Cirrus side.

I have a link to an example PR of what I'm looking for: https://github.com/mozilla/probe-scraper/pull/617/files

It doesn't look like they link to any yaml files at all there though, so, maybe I shouldn't have added those. ni?yashika for clarity. Thank you.

Flags: needinfo?(wclouser) → needinfo?(ykhurana)

(In reply to Wil Clouser [:clouserw] from comment #2)

Thanks for the reply. I have a checklist I'm following and I probably misunderstood something on it. @yashika is supporting from the Cirrus side.

I have a link to an example PR of what I'm looking for: https://github.com/mozilla/probe-scraper/pull/617/files

Thanks! I think the checklist needs to be tweaked a bit (you should not be dealing with the PRs, that's an internal implementation detail of the ecosystem). This is what the checklist should say for this (for when Yashika will read this):

  1. File a bug following the Glean docs (like you did!)
  2. Pick the application id as described by the checklist already (that was good!)
  3. Call out that this is for a Nimbus / Cirrus Sidecar
  4. Have the Glean app depend on "glean-core" and "nimbus-cirrus" (to be noted in the dependencies section when filing a bug).
  5. Don't add any metrics or ping files, as this is not required for the sidecar (the right dependencies are already included)

It doesn't look like they link to any yaml files at all there though, so, maybe I shouldn't have added those. ni?yashika for clarity. Thank you.

Nope, they're good, the correct files are included because of the things they depend on.

Thanks for bearing with me, this bug is now actionable and will be acted upon when triaging.

Hey, I'll be working on this request and just wanted to clarify the changes to the original comment based on this discussion. I believe the following is correct because this is a Cirrus app, but I'm not clear if the metrics/pings files should be included based on Alessio's last comment.

Locations of metrics.yaml files (can be many):

  • (should be empty)

Locations of pings.yaml files (can be many):

  • (should be empty)

Dependencies:

  • glean-core
  • nimbus-cirrus (added)
Flags: needinfo?(alessio.placitelli)

One other question, this time I think for :clouserw -- the Data-review response link is blank here, was there a data review approval?

Flags: needinfo?(wclouser)

(In reply to Mike Williams [:mwilliams] from comment #4)

Hey, I'll be working on this request and just wanted to clarify the changes to the original comment based on this discussion. I believe the following is correct because this is a Cirrus app, but I'm not clear if the metrics/pings files should be included based on Alessio's last comment.

Locations of metrics.yaml files (can be many):

  • (should be empty)

Locations of pings.yaml files (can be many):

  • (should be empty)

Dependencies:

  • glean-core
  • nimbus-cirrus (added)

Yes, this is correct. I amended the Confluence runbook this morning to add these details there as well. Thank you for doing this!

Flags: needinfo?(alessio.placitelli)

We got data-review approvals for the original glean metrics. We haven't done any additional for Nimbus - I don't think they are needed as we're not collecting anything new though?

Flags: needinfo?(wclouser)

(In reply to Wil Clouser [:clouserw] from comment #7)

We got data-review approvals for the original glean metrics. We haven't done any additional for Nimbus - I don't think they are needed as we're not collecting anything new though?

You are (via Cirrus). Charlie asked for data-review in bug 1848664 comment 5

I've created a (draft PR)[https://github.com/mozilla/probe-scraper/pull/709] for this request. Once I get confirmation of data review approval the PR will be ready for review.

Understood, thanks. I'm attaching the review form here and requesting review from @tlong who has reviewed the other accounts data reviews in the recent months

Flags: needinfo?(tlong)
Flags: needinfo?(tlong)
Attachment #9391794 - Flags: data-review?(tlong)

Comment on attachment 9391794 [details]
Accounts Nimbus Data Collection Review Form.md

Data Review

  1. Is there or will there be documentation that describes the schema for the ultimate data set in a public, complete, and accurate way?

Yes, through the metrics.yaml file and the Glean Dictionary.

  1. Is there a control mechanism that allows the user to turn the data collection on and off?

Yes, through the data preferences in the application settings.

  1. If the request is for permanent data collection, is there someone who will monitor the data over time?

Permanent collection of this data to be monitored by Vesta Zare

  1. Using the category system of data types on the Mozilla wiki, what collection type of data do the requested measurements fall under?

Category 2, Interaction data

  1. Is the data collection request for default-on or default-off?

Default-on

  1. Does the instrumentation include the addition of any new identifiers (whether anonymous or otherwise; e.g., username, random IDs, etc. See the appendix for more details)?

No

  1. Is the data collection covered by the existing Firefox privacy notice?

Yes

  1. Does the data collection use a third-party collection tool?

No

Result

data-review+

Attachment #9391794 - Flags: data-review?(tlong) → data-review+

Data-review is something of a formality in this case, but it's standard practice when adding existing, reviewed metrics to a new population.

Assignee: nobody → mwilliams
Status: NEW → ASSIGNED

Thanks! I marked the PR as ready for review, and will confirm that everything is working properly once that's merged.

Alright, everything appears to be in order now. I enabled the Looker explores for accounts_cirrus (PR: https://github.com/mozilla/looker-spoke-default/pull/786), so you should be all set!

Status: ASSIGNED → RESOLVED
Closed: 11 months ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: