Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MNTOR-3814 - use context to fetch experiment data from Cirrus #5440

Draft
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

rhelmer
Copy link
Collaborator

@rhelmer rhelmer commented Dec 20, 2024

References:

Jira: MNTOR-3814

Description

This PR introduces an ExperimentsContext with a provider and a useExperiments() hook that can be called by any client component that needs it. I implemented it just for Glean so far (per the linked Jira ticket) but if this approach is OK I can file a followup to move from prop-drilling to this more generally for features that need access to experiment data (which features are activated, what the active branch is, etc)

How to test

I've been using the Glean debug dashboard to inspect pings. Note that local pings have -1 for most Enrollments values, you need to use Cirrus (or something that mocks it). I'll see if I can mock this to add some automated tests.

An easy way to inspect Glean pings is to run this in the browser devtools console:

Glean.setLogPings(true)

Which should log:

Pings will be logged to the console until this tab is closed.

Checklist (Definition of Done)

  • Commits in this PR are minimal and have descriptive commit messages.
  • All acceptance criteria are met.
  • Jira ticket has been updated (if needed) to match changes made during the development process.
  • Jira ticket has been updated (if needed) with suggestions for QA when this PR is deployed to stage.

@rhelmer rhelmer self-assigned this Dec 20, 2024
@rhelmer rhelmer marked this pull request as draft December 20, 2024 05:32
Copy link

src/app/hooks/useGlean.ts Show resolved Hide resolved
src/app/hooks/useGlean.ts Outdated Show resolved Hide resolved
Comment on lines +59 to +76
nimbus_user_id:
description: Nimbus user ID
type: string
nimbus_app_id:
description: Nimbus application ID
type: string
nimbus_experiment:
description: Nimbus experiment name
type: string
nimbus_branch:
description: Nimbus branch
type: string
nimbus_experiment_type:
description: Nimbus experiment type
type: string
nimbus_is_preview:
description: Nimbus preview mode enabled
type: string
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would we need to check in with a data steward to see if these metrics are still covered by our initial data review?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, so we may not be able to land this until that's done.

@rhelmer rhelmer changed the title attempt to use context for experiment data and glean MNTOR-3814 - use context to fetch experiment data from Cirrus Dec 20, 2024
@rhelmer rhelmer requested a review from flozia December 20, 2024 21:59
Copy link
Collaborator

@flozia flozia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving with the caveat that we are waiting on directions from a data steward. The code looks good to me and when giving this a spin locally it works as expected.

src/middleware.ts Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants