Comfy-Org / Comfy-Org/Comfy-Desktop

No monitor on sign_in_failed and Desktop RUM is dark, so auth regressions cannot alert

Open
#1,447 1 comment 0 reactions 1 assignee Claimed by @benceruleanlu View on GitHub
Dominant language
TypeScript
Stars
458
Forks
59
Avg merge
22h 18m
Merged PRs (30d)
45

Description

A regression that cost roughly 620 sign-ins a day ran from 8 August until it was found by hand two weeks later. Nothing fired, and with the current setup nothing could have.

## Two separate gaps

**1. There is no monitor on `sign_in_failed`.**

`comfy.desktop.auth.sign_in_failed` is explicitly allow-listed for mirroring to Datadog in `src/shared/datadogMirroredEvents.ts`, and the comment on that list says adding a name to it means someone wants a monitor on that event. No monitor was ever created. The event has been mirroring into a void.

**2. Desktop's Datadog RUM application is effectively empty.**

It carries roughly 23 sessions in 30 days, against ~34 million events for the Cloud frontend. This is the more dangerous of the two, because it fails quietly in the wrong direction: anyone investigating a Desktop auth problem in Datadog gets a handful of rows back and concludes things are fine. Every number in the 8 August analysis had to come from PostHog for this reason.

## Asks

1. Create a Datadog monitor on `comfy.desktop.auth.sign_in_failed` with a rate-based threshold, not a count-based one, so it survives volume swings. The event-level failure rate sat at a stable 6% before 8 August and a stable 10.2% after, so a rate alert with a modest tolerance would have fired within a day.
2. Work out why Desktop RUM is dark and either fix it or write down that it is not a usable data source, so the next person does not draw a false conclusion from it. `sessionSampleRate` is set to 100 in `rendererBootstrap.ts`, so the sampling config is not the obvious cause.
3. Consider a per-person completion alert rather than an event-level failure alert. The failure rate moved 6% -> 10% across the regression, but per-person completion moved 70% -> 42%. The per-person metric is roughly seven times more sensitive here, because people retry and retries flatter the per-click rate.

Point 3 is the one that generalises. Failure rate per click is the metric we happen to have, but it consistently understates user-visible damage on any flow where retrying is cheap.

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.