Skip to content

fix(perps): correct CUF trace lifecycle and tags - #33562

Merged
abretonc7s merged 4 commits into
mainfrom
perps-fix-measurement-unmount-trace-0528
Jul 27, 2026
Merged

fix(perps): correct CUF trace lifecycle and tags#33562
abretonc7s merged 4 commits into
mainfrom
perps-fix-measurement-unmount-trace-0528

Conversation

@abretonc7s

@abretonc7s abretonc7s commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Description

usePerpsMeasurement left a manual Sentry trace active when its view unmounted before readiness. This adds a mount-lifetime cleanup that ends only that hook instance's active trace once as success: false with reason unmounted.

It also adds the shared feature and lifecycle_context tags to the live-proven first-price direct/prewarm and first-order-book WebSocket trace starts.

Failed reconnect traces now preserve background_resume until a later successful foreground-settling span, so a replacement reconnect retains the correct lifecycle context.

Changelog

CHANGELOG entry: null

Related issues

Fixes: No linked public issue; isolated H1/H2/H3 telemetry-integrity findings.

Manual testing steps

Feature: Perps CUF telemetry integrity

  Scenario: User leaves market detail before readiness
    Given the iOS dev client is unlocked and Perps is available
    When the user opens BTC market detail and immediately leaves before data is ready
    Then its active manual trace ends exactly once
    And the trace records success as false with reason "unmounted"

  Scenario: First-price WebSocket measurement starts
    Given the user opens the Perps market list
    When the real price stream starts its first-data trace
    Then the trace request includes feature "perps"
    And it includes the current lifecycle context

  Scenario: A failed foreground reconnect is replaced
    Given Perps resumes from the background
    When one forced reconnect fails as disconnected and a second reconnect starts
    Then the failed reconnect does not settle the foreground lifecycle
    And the replacement reconnect retains lifecycle context "background_resume"

Validation performed:

  • H3 before recipe: 10/10 actions passed and reproduced a same-ID start → active unmount with no end.
  • H3 after recipe: 12/12 actions passed and proved a same-ID start → active unmount → success=false reason=unmounted end.
  • H3 focused Jest: 15/15 tests passed; changed executable production-line coverage is 100%.
  • H1 before: two real first-price boundary requests carried empty tags.
  • H1 after: two real first-price boundary requests carried feature=perps and lifecycle_context=warm.
  • H1 focused Jest: 3/3 tests passed; managed Claude full-file run: 120/120.
  • H1 exact diff was independently approved by managed Claude tmux review and Cursor.
  • H2 before recipe: 10/10 actions passed with recovery disabled and proved background_resume → failed/disconnected completion → warm → second start warm.
  • H2 after recipe: the unchanged 10/10 recipe passed with recovery disabled and proved background_resume → failed/disconnected completion → background_resume → second start background_resume.
  • H2 focused Jest: 2/2 tests passed; managed Claude related-file run: 61/61.
  • H2 exact diff was approved by managed Claude tmux review session xreview-mobile-h1-cuf-claude.

Screenshots/Recordings

Before

N/A — these are non-visual telemetry fixes. Real Hermes boundary proof reproduced the leaked H3 trace, empty-tag H1 requests, and premature H2 foreground settlement.

After

N/A — no visible UI change. Real Hermes after-runs proved the H3 abandonment classification, corrected H1 tags, and retained H2 background_resume context across failed reconnect replacement.

Pre-merge author checklist

Performance checks (if applicable)

  • I've tested on Android
    • N/A — these are shared TypeScript paths; real-app proof ran on iOS and focused unit coverage exercises the lifecycle/tag contracts.
  • I've tested with a power user scenario
    • N/A — wallet size does not affect these trace lifecycle or start-tag contracts.
  • I've instrumented key operations with Sentry traces for production performance metrics
    • This PR corrects existing manual Perps Sentry trace lifecycle and shared start tags.

Pre-merge reviewer checklist

  • I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed).
  • I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.

Note

Low Risk
Observability-only changes to Perps trace lifecycle and tags; no auth, payments, or user-facing UI behavior.

Overview
Fixes Perps CUF / Sentry telemetry so spans close correctly and start with consistent tags.

usePerpsMeasurement now runs unmount cleanup: if a manual trace is still open when the hook’s view unmounts, it ends once with success: false and reason: 'unmounted' (no double-end after success or reset). activeTraceName is stored when a trace starts so cleanup uses the right span name.

PerpsStreamManager attaches buildPerpsCufStartTags() (feature, lifecycle_context) when starting first-price (subscribe + prewarm) and first-order-book WebSocket traces.

endPerpsCufTrace only calls settlePerpsForegroundOnSpan when the end payload is not explicitly failed (success !== false), so a failed reconnect keeps background_resume until a later successful foreground-settling span.

Tests cover unmount behavior, WebSocket trace tags, and reconnect success vs failure lifecycle.

Reviewed by Cursor Bugbot for commit 49517da. Bugbot is set up for automated code reviews on this repo. Configure here.

@abretonc7s abretonc7s added type-bug Something isn't working no-changelog no-changelog Indicates no external facing user changes, therefore no changelog documentation needed team-perps Perps team labels Jul 21, 2026
@abretonc7s abretonc7s self-assigned this Jul 21, 2026
@github-actions

Copy link
Copy Markdown
Contributor

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

@github-actions github-actions Bot added size-M and removed size-S labels Jul 21, 2026
@github-actions

github-actions Bot commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

🧪 Flaky unit test detection

Run history flaky detection

View recent run history

Historical failure rate is a hint, not proof — review each suggestion in context. See the flaky-test-detection skill for the full pattern reference and manual audit workflow.

Failures / runs sampled per window:

File 7d 15d 30d
app/components/UI/Perps/providers/PerpsStreamManager.test.tsx 0/173 0/189 0/358

AI-detected flaky patterns

app/components/UI/Perps/providers/PerpsStreamManager.test.tsx

  • J8 — jest.useFakeTimers() combined with waitFor() (polling conflict) (high)
    • beforeEach calls jest.useFakeTimers() (line ~93) while this test and ~10 others use await waitFor() for assertions on subscription/update effects. waitFor's internal real setTimeout polling never fires under fake timers, leading to intermittent timeouts or flaky passes depending on test order and CI load. Matches J8 exactly; history had no signal but pattern is present.
    • Suggested fix in app/components/UI/Perps/providers/PerpsStreamManager.test.tsx:
      -    // Should receive cached data immediately
      -    await waitFor(() => {
      -      expect(onUpdate).toHaveBeenCalledWith({
      -        'BTC-PERP': {
      -          symbol: 'BTC-PERP',
      -          price: '50000',
      -          timestamp: expect.any(Number),
      -          percentChange24h: '5',
      -          bestBid: '49900',
      -          bestAsk: '50100',
      -          spread: '200',
      -          markPrice: '50050',
      -          funding: undefined,
      -          openInterest: undefined,
      -          volume24h: undefined,
      -          isTradable: true,
      -        },
      -      });
      -    });
      -
      +    // Should receive cached data immediately
      +    jest.useRealTimers();
      +    await waitFor(() => {
      +      expect(onUpdate).toHaveBeenCalledWith({
      +        'BTC-PERP': {
      +          symbol: 'BTC-PERP',
      +          price: '50000',
      +          timestamp: expect.any(Number),
      +          percentChange24h: '5',
      +          bestBid: '49900',
      +          bestAsk: '50100',
      +          spread: '200',
      +          markPrice: '50050',
      +          funding: undefined,
      +          openInterest: undefined,
      +          volume24h: undefined,
      +          isTradable: true,
      +        },
      +      });
      +    });
      +    jest.useFakeTimers();
      +
  • J7 — Non-deterministic data: Date.now(), Math.random(), unstubbed network (medium)
    • Test data uses live Date.now() for timestamp inside mockSubscribeToPrices.mockImplementation (and similar in 5+ other tests). Even with fakeTimers this can vary if timers not pinned with setSystemTime, making expectations brittle. Matches J7 non-deterministic data pattern (similar to noted Perps tests in skill). No other J1-J6,J9-J10 patterns matched with concrete snippets in either file.
    • Suggested fix in app/components/UI/Perps/providers/PerpsStreamManager.test.tsx:
      -        const cachedData: PriceUpdate[] = [
      -          {
      -            symbol: 'BTC-PERP',
      -            price: '50000',
      -            percentChange24h: '5',
      -            timestamp: Date.now(),
      -            bestBid: '49900',
      -            bestAsk: '50100',
      -            spread: '200',
      -            markPrice: '50050',
      -            isTradable: true,
      -          },
      -        ];
      +        const cachedData: PriceUpdate[] = [
      +          {
      +            symbol: 'BTC-PERP',
      +            price: '50000',
      +            percentChange24h: '5',
      +            timestamp: 1722470400000,
      +            bestBid: '49900',
      +            bestAsk: '50100',
      +            spread: '200',
      +            markPrice: '50050',
      +            isTradable: true,
      +          },
      +        ];

This check is informational only and does not block merging.

@abretonc7s abretonc7s changed the title fix(perps): end active traces on unmount fix(perps): close unmounted traces and tag websocket spans Jul 21, 2026
@abretonc7s abretonc7s changed the title fix(perps): close unmounted traces and tag websocket spans fix(perps): correct CUF trace lifecycle and tags Jul 21, 2026
@abretonc7s
abretonc7s marked this pull request as ready for review July 27, 2026 05:49
@abretonc7s
abretonc7s requested a review from a team as a code owner July 27, 2026 05:49
@github-actions

Copy link
Copy Markdown
Contributor

🔍 Smart E2E Test Selection

  • Selected E2E tags: SmokePerps, SmokeWalletPlatform, SmokeConfirmations
  • Selected Performance tags: @PerformancePreps
  • Risk Level: medium
  • AI Confidence: 85%
click to see 🤖 AI reasoning details

E2E Test Selection:
All 6 changed files are within the Perps feature directory (app/components/UI/Perps/). The changes are:

  1. usePerpsMeasurement.ts: Adds unmount cleanup to end active traces with success: false, reason: 'unmounted' - prevents orphaned Sentry traces when Perps components unmount unexpectedly.

  2. perpsCufTrace.ts: Bug fix - failed spans (success: false) no longer incorrectly settle the foreground lifecycle state. Only successful spans should mark foreground work as complete.

  3. PerpsStreamManager.tsx: Adds buildPerpsCufStartTags() to WebSocket trace starts (first price, first order book, prewarmed price) for better CUF observability tagging.

  4. Test files: Unit tests covering all the above changes.

These changes are internal to Perps measurement/tracing infrastructure. While they don't change user-visible behavior directly, they touch PerpsStreamManager.tsx which is a core data streaming provider. The tracing changes could theoretically affect component lifecycle if the cleanup logic has bugs.

SmokePerps is selected as the primary tag since all changes are Perps-specific.
SmokeWalletPlatform is required per SmokePerps tag description (Perps is a section inside Trending tab).
SmokeConfirmations is required per SmokePerps tag description (Add Funds deposits are on-chain transactions).

Performance Test Selection:
The changed files directly modify the Perps CUF (Critical User Flow) tracing infrastructure: usePerpsMeasurement.ts (the hook that drives performance measurement), perpsCufTrace.ts (the utility that manages CUF trace lifecycle and foreground settling), and PerpsStreamManager.tsx (adds CUF start tags to WebSocket traces). These are the exact files that @PerformancePreps tests exercise - they measure perps market loading, position management, add funds flow, and order execution. The bug fix in perpsCufTrace.ts (failed spans no longer settling foreground state) and the unmount cleanup in usePerpsMeasurement.ts could affect how performance metrics are recorded and reported.

View GitHub Actions results

@github-actions github-actions Bot added the risk:medium AI analysis: medium risk label Jul 27, 2026
@abretonc7s
abretonc7s enabled auto-merge July 27, 2026 05:53
@sonarqubecloud

Copy link
Copy Markdown

@github-actions

Copy link
Copy Markdown
Contributor

⚡ Performance Test Results

ℹ️ Performance test results are currently non-blocking and will not block this PR.

1 test failed · 2 tests · 1 device

📱 Devices tested (1)

Android: Google Pixel 8 Pro (v14.0)

❌ Failed Tests (1)

@mm-perps-engineering-team

Test Platform Device Reason Recording
Perps open position and close it Android Google Pixel 8 Pro (v14.0) Test error 📹 Watch
✅ Passed Tests (1)
Test Platform Device Duration Team Recording
Perps add funds Android Google Pixel 8 Pro (v14.0) 8.94s @mm-perps-engineering-team 📹 Watch

Branch: perps-fix-measurement-unmount-trace-0528 · Build: Normal · Commit: 07e0bf2 · View full run

@abretonc7s
abretonc7s added this pull request to the merge queue Jul 27, 2026
Merged via the queue into main with commit ba0f19d Jul 27, 2026
207 checks passed
@abretonc7s
abretonc7s deleted the perps-fix-measurement-unmount-trace-0528 branch July 27, 2026 08:23
@github-actions github-actions Bot locked and limited conversation to collaborators Jul 27, 2026
@metamask-ci metamask-ci Bot added the release-8.6.0 Issue or pull request that will be included in release 8.6.0 label Jul 27, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

no-changelog no-changelog Indicates no external facing user changes, therefore no changelog documentation needed release-8.6.0 Issue or pull request that will be included in release 8.6.0 risk:medium AI analysis: medium risk size-M team-perps Perps team type-bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants