Skip to content

feat(passkey): Add Glean events for the wrap lifecycle - #21151

Open
nshirley wants to merge 1 commit into
mainfrom
FXA-14156
Open

feat(passkey): Add Glean events for the wrap lifecycle#21151
nshirley wants to merge 1 commit into
mainfrom
FXA-14156

Conversation

@nshirley

@nshirley nshirley commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Because:

  • Wrap operations have statsd counters but no attributed metrics, so enrolment and passwordless sign-in cannot be measured per service, flow or user.
  • A passkey whose wrap never stored can never unseal Sync keys, and nothing today shows how often that happens.

This commit:

  • Adds passkey.wrap_create_complete and wrap_create_failure to POST /passkey/wraps.
  • Adds passkey.wrap_get_complete and wrap_get_failure to GET /passkey/wraps/{credentialId}.
  • Reports a reason of not_bound, conflict, stale, wrap_not_found, credential_not_found or unexpected on the failure events.
  • Threads glean into passkeyWrapsRoutes.

Closes #FXA-14156

Checklist

Put an x in the boxes that apply

  • My commit is GPG signed.
  • If applicable, I have modified or added tests which pass locally.
  • I have added necessary documentation (if appropriate).
  • I have verified that my changes render correctly in RTL (if appropriate).
  • I have manually reviewed all AI generated code.

How to review (Optional)

  • Key files/areas to focus on:
  • Suggested review order:
  • Risky or complex parts:

Screenshots (Optional)

Please attach the screenshots of the changes made in case of change in user interface.

Other information (Optional)

Any other information that is important to this pull request.

@nshirley
nshirley requested a review from a team as a code owner September 4, 2026 20:36
Copilot AI balanced review requested due to automatic review settings September 4, 2026 20:36
reason:
description: |
Why the envelope was withheld. One of `not_bound` (the MFA token was
minted for a different credential), `stale` (the envelope seals a kB

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Since we've been talking about it - @vpomerleau this includes the stale which is currently withheld. But if we pivot on that then this will also need to be updated. Just noting for so two of us are aware 😃

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Metric descriptions need correction and generated bindings must be refreshed.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Adds attributed Glean telemetry for passkey wrap creation and retrieval.

Changes:

  • Defines wrap completion and failure events with categorized reasons.
  • Emits telemetry from passkey wrap routes.
  • Adds route tests and generated Glean bindings.

Review comments:

  • Correct misleading metric descriptions that label successful events as denominators.
  • Use “fallback” rather than “fall back” and regenerate server_events.ts.
  • Document that not_bound covers both missing and mismatched credential bindings.
File summaries
File Description
packages/fxa-shared/metrics/glean/fxa-backend-metrics.yaml Defines wrap lifecycle metrics.
packages/fxa-auth-server/lib/routes/passkey-wraps.ts Emits wrap success and failure events.
packages/fxa-auth-server/lib/routes/passkey-wraps.spec.ts Tests event emission and reason mapping.
packages/fxa-auth-server/lib/routes/index.js Injects Glean into wrap routes.
packages/fxa-auth-server/lib/metrics/glean/server_events.ts Adds generated event loggers.
packages/fxa-auth-server/lib/metrics/glean/index.ts Exposes wrap event functions.
packages/fxa-auth-server/lib/metrics/glean/index.spec.ts Adds generated logger mocks.
Review details

Suppressed comments (3)

packages/fxa-shared/metrics/glean/fxa-backend-metrics.yaml:1927

  • The documented reason taxonomy does not match the route: a missing credential binding is also emitted as not_bound, and any unrecognized error is emitted as unexpected. Omitting these meanings makes valid telemetry appear outside the metric contract.
          Why the envelope was withheld. One of `not_bound` (the MFA token was
          minted for a different credential), `stale` (the envelope seals a kB
          the account has since replaced, so the passkey needs re-enrolling),
          `wrap_not_found` (the passkey was never given an envelope), or
          `credential_not_found`.

packages/fxa-shared/metrics/glean/fxa-backend-metrics.yaml:1905

  • Use the noun “fallback” here; “fall back” is the verb form. Regenerate server_events.ts after correcting the source metric description.
      for the user, in a silent fall back to signing in with a password, so the

packages/fxa-shared/metrics/glean/fxa-backend-metrics.yaml:1880

  • The documented not_bound meaning excludes the missing-cid case, but isBoundTo returns false for both a missing binding and a mismatched credential, and both are recorded with this reason. Document both cases so analyses do not interpret every event as a cross-credential mismatch.
          Why the write was refused. One of `not_bound` (the MFA token was
          minted for a different credential), `conflict` (a different envelope
          is already stored), `credential_not_found`, or `unexpected`.
  • Files reviewed: 7/7 changed files
  • Comments generated: 1
  • Review effort level: Balanced

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread packages/fxa-shared/metrics/glean/fxa-backend-metrics.yaml Outdated
Because:
  - Wrap operations have statsd counters but no attributed metrics, so
    enrolment and passwordless sign-in cannot be measured per service,
    flow or user.
  - A passkey whose wrap never stored can never unseal Sync keys, and
    nothing today shows how often that happens.

This commit:
  - Adds passkey.wrap_create_complete and wrap_create_failure to
    POST /passkey/wraps.
  - Adds passkey.wrap_get_complete and wrap_get_failure to
    GET /passkey/wraps/{credentialId}.
  - Reports a reason of not_bound, conflict, stale, wrap_not_found,
    credential_not_found or unexpected on the failure events.
  - Threads glean into passkeyWrapsRoutes.

Closes #FXA-14156
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