-
Notifications
You must be signed in to change notification settings - Fork 394
feat(shared): Capture auth component mounted for all SDK types #6858
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
feat(shared): Capture auth component mounted for all SDK types #6858
Conversation
🦋 Changeset detectedLatest commit: fa91b76 The changes in this PR will be included in the next version bump. This PR includes changesets to release 19 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughAdds a changeset and updates telemetry to use per-component sampling for COMPONENT_MOUNTED events: Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant UI as Auth Component (SignIn/SignUp/Other)
participant Telemetry as Telemetry Events
participant Sampler as Sampling Strategy
User->>UI: Mount component
UI->>Telemetry: createPrebuiltComponentEvent(COMPONENT_MOUNTED, component)
Telemetry->>Sampler: getComponentMountedSamplingRate(component)
alt High-signal component (SignIn/SignUp)
Sampler-->>Telemetry: 1.0
else Other component
Sampler-->>Telemetry: 0.1
end
Telemetry-->>UI: Event emitted with computed sampling rate
sequenceDiagram
autonumber
actor User
participant UI as Auth Component
participant Telemetry as Telemetry Events
User->>UI: Open component
UI->>Telemetry: createPrebuiltComponentEvent(COMPONENT_OPENED, component)
Note right of Telemetry: Uses default sampling (unchanged, 0.1)
Telemetry-->>UI: Event emitted with default sampling rate
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used📓 Path-based instructions (6)**/*.{js,jsx,ts,tsx}📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Files:
**/*.{js,jsx,ts,tsx,json,css,scss,md,yaml,yml}📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Files:
packages/**/*.{ts,tsx}📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Files:
packages/**/*.{ts,tsx,d.ts}📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Files:
**/*.{ts,tsx}📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Files:
**/*.{js,ts,tsx,jsx}📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
Files:
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
🔇 Additional comments (1)
Comment |
c8e9693
to
dd10d55
Compare
@clerk/agent-toolkit
@clerk/astro
@clerk/backend
@clerk/chrome-extension
@clerk/clerk-js
@clerk/dev-cli
@clerk/elements
@clerk/clerk-expo
@clerk/expo-passkeys
@clerk/express
@clerk/fastify
@clerk/localizations
@clerk/nextjs
@clerk/nuxt
@clerk/clerk-react
@clerk/react-router
@clerk/remix
@clerk/shared
@clerk/tanstack-react-start
@clerk/testing
@clerk/themes
@clerk/types
@clerk/upgrade
@clerk/vue
commit: |
38f220c
to
3078caf
Compare
* @internal | ||
*/ | ||
function getComponentMountedSamplingRate(component: string): number { | ||
return AUTH_COMPONENTS.has(component) ? 1 : EVENT_SAMPLING_RATE; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we really need to go from 10% to 100%? Not objecting to it, just want to call out that we can probably get a representative sample at a lower sampling rate.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think if this was telemetry event data we used in isolation we could lower the sampling rate, but we're trying to compare the telemetry event for this between keyless (which has 100% sampling for this event) and all SDKs. I might have that wrong though, and we could lower it. What do you think?
a475718
to
cb6beb5
Compare
Update .changeset/pretty-rings-compare.md Co-authored-by: Dylan Staley <88163+dstaley@users.noreply.github.com>
cb6beb5
to
fa91b76
Compare
Co-authored-by: Dylan Staley <88163+dstaley@users.noreply.github.com>
Description
Enables 100% sampling rate for
SignIn
andSignUp
components for all SDKs. This will enable a more holistic comparison of Auth Component Mounted data between keyless and other SDK types.Checklist
pnpm test
runs as expected.pnpm build
runs as expected.Type of change
Summary by CodeRabbit
New Features
Chores