chore(clerk-js,shared): Increase sampling for ui components on keyless #6514
Conversation
🦋 Changeset detectedLatest commit: 19c3bf5 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 Git ↗︎ 1 Skipped Deployment
|
@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: |
📝 WalkthroughWalkthroughAdds two changesets (patch for @clerk/clerk-js, minor for @clerk/shared). Introduces a new TelemetryCollector option perEventSampling and wires it through types and collector initialization (default true). Server-side telemetry sampling logic now respects perEventSampling, and tests were added for sampling interactions. In clerk-js many prebuilt-component flows were standardized to use a local component identifier for preloadHint, mount name, and telemetry event names; telemetry initialization for keyless apps conditionally sets perEventSampling to false (throttling vs per-event sampling). bundlewatch headless size threshold increased from 57KB to 57.1KB. Minor JSDoc and guard/optional-chaining cleanups included. Estimated code review effort🎯 4 (Complex) | ⏱️ ~35 minutes 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
.changeset/bitter-waves-burn.md (1)
5-5: Clarify terminology: sampling vs throttlingThe code implements a per-event sampling rate override (eventSamplingRate), setting it to 1 for keyless. The note says “Use throttling instead of sampling,” which can confuse readers. Suggest clarifying that for keyless, samplingRate is set to 1 (i.e., 100% of events are sent).
Suggested wording:
-Use throttling instead of sampling for telemetry events of UI components on keyless apps. +For keyless apps, send 100% of UI component telemetry by overriding the event sampling rate to 1..changeset/slow-shoes-give.md (1)
5-5: Mention expected range for the new parameterCall out that samplingRate is expected to be between 0 and 1 (inclusive) to guide consumers.
-Update `eventPrebuiltComponentMounted` and `eventPrebuiltComponentOpened` to accept a samplingRate. +Update `eventPrebuiltComponentMounted` and `eventPrebuiltComponentOpened` to accept an optional `samplingRate` (0..1).packages/shared/src/telemetry/events/component-mounted.ts (1)
51-62: Document expected range and default behavior for samplingRateJSDoc mentions the param but not its valid range or default. Add brief guidance.
- * @param samplingRate - The sampling rate for the event. + * @param samplingRate - Optional sampling rate for the event (0..1). Defaults to a library constant.Also applies to: 70-81
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.changeset/bitter-waves-burn.md(1 hunks).changeset/slow-shoes-give.md(1 hunks)packages/clerk-js/src/core/clerk.ts(22 hunks)packages/shared/src/telemetry/events/component-mounted.ts(3 hunks)
🧰 Additional context used
📓 Path-based instructions (8)
.changeset/**
📄 CodeRabbit Inference Engine (.cursor/rules/monorepo.mdc)
Automated releases must use Changesets.
Files:
.changeset/slow-shoes-give.md.changeset/bitter-waves-burn.md
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)
**/*.{js,jsx,ts,tsx}: All code must pass ESLint checks with the project's configuration
Follow established naming conventions (PascalCase for components, camelCase for variables)
Maintain comprehensive JSDoc comments for public APIs
Use dynamic imports for optional features
All public APIs must be documented with JSDoc
Provide meaningful error messages to developers
Include error recovery suggestions where applicable
Log errors appropriately for debugging
Lazy load components and features when possible
Implement proper caching strategies
Use efficient data structures and algorithms
Profile and optimize critical paths
Validate all inputs and sanitize outputs
Implement proper logging with different levels
Files:
packages/clerk-js/src/core/clerk.tspackages/shared/src/telemetry/events/component-mounted.ts
**/*.{js,jsx,ts,tsx,json,css,scss,md,yaml,yml}
📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)
Use Prettier for consistent code formatting
Files:
packages/clerk-js/src/core/clerk.tspackages/shared/src/telemetry/events/component-mounted.ts
packages/**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)
TypeScript is required for all packages
Files:
packages/clerk-js/src/core/clerk.tspackages/shared/src/telemetry/events/component-mounted.ts
packages/**/*.{ts,tsx,d.ts}
📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)
Packages should export TypeScript types alongside runtime code
Files:
packages/clerk-js/src/core/clerk.tspackages/shared/src/telemetry/events/component-mounted.ts
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)
Use proper TypeScript error types
**/*.{ts,tsx}: Always define explicit return types for functions, especially public APIs
Use proper type annotations for variables and parameters where inference isn't clear
Avoidanytype - preferunknownwhen type is uncertain, then narrow with type guards
Useinterfacefor object shapes that might be extended
Usetypefor unions, primitives, and computed types
Preferreadonlyproperties for immutable data structures
Useprivatefor internal implementation details
Useprotectedfor inheritance hierarchies
Usepublicexplicitly for clarity in public APIs
Preferreadonlyfor properties that shouldn't change after construction
Prefer composition and interfaces over deep inheritance chains
Use mixins for shared behavior across unrelated classes
Implement dependency injection for loose coupling
Let TypeScript infer when types are obvious
Useconst assertionsfor literal types:as const
Usesatisfiesoperator for type checking without widening
Use mapped types for transforming object types
Use conditional types for type-level logic
Leverage template literal types for string manipulation
Use ES6 imports/exports consistently
Use default exports sparingly, prefer named exports
Use type-only imports:import type { ... } from ...
Noanytypes without justification
Proper error handling with typed errors
Consistent use ofreadonlyfor immutable data
Proper generic constraints
No unused type parameters
Proper use of utility types instead of manual type construction
Type-only imports where possible
Proper tree-shaking friendly exports
No circular dependencies
Efficient type computations (avoid deep recursion)
Files:
packages/clerk-js/src/core/clerk.tspackages/shared/src/telemetry/events/component-mounted.ts
**/*.{js,ts,tsx,jsx}
📄 CodeRabbit Inference Engine (.cursor/rules/monorepo.mdc)
Support multiple Clerk environment variables (CLERK_, NEXT_PUBLIC_CLERK_, etc.) for configuration.
Files:
packages/clerk-js/src/core/clerk.tspackages/shared/src/telemetry/events/component-mounted.ts
**/*
⚙️ CodeRabbit Configuration File
If there are no tests added or modified as part of the PR, please suggest that tests be added to cover the changes.
Files:
packages/clerk-js/src/core/clerk.tspackages/shared/src/telemetry/events/component-mounted.ts
🧬 Code Graph Analysis (2)
packages/clerk-js/src/core/clerk.ts (1)
packages/shared/src/telemetry/events/component-mounted.ts (2)
eventPrebuiltComponentOpened(74-81)eventPrebuiltComponentMounted(55-62)
packages/shared/src/telemetry/events/component-mounted.ts (1)
packages/types/src/telemetry.ts (1)
TelemetryEventRaw(40-44)
⏰ 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). (5)
- GitHub Check: Formatting | Dedupe | Changeset
- GitHub Check: Build Packages
- GitHub Check: semgrep/ci
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (2)
packages/shared/src/telemetry/events/component-mounted.ts (1)
55-61: LGTM: API changes are minimal and backward-compatibleOptional param is added at the end, return types remain explicit, and event payload remains stable.
Also applies to: 74-81
packages/clerk-js/src/core/clerk.ts (1)
350-353: LGTM: Centralized sampling rate logic for keylessPrivate getter cleanly encapsulates the keyless override (1 or undefined). Call sites remain simple.
brkalow
left a comment
brkalow
left a comment
There was a problem hiding this comment.
Can we disable sampling at the collector level instead of per-event? Something like disableSampling: true as an option to the collector.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
packages/clerk-js/src/core/clerk.ts (2)
660-662: Remove unnecessary template literalThis uses a backtick template literal without interpolation, which violates ESLint's
no-useless-template-literalsrule:this.telemetry?.record( - eventPrebuiltComponentOpened(`UserVerification`, props, undefined, this.telemetrySamplingRate), + eventPrebuiltComponentOpened('UserVerification', props, undefined, this.telemetrySamplingRate), );
730-730: Fix operator precedence in length checksThe expressions on these lines have operator precedence issues that can lead to incorrect evaluation:
Line 730 & 880:
-const additionalData = (props?.customPages?.length || 0) > 0 ? { customPages: true } : undefined; +const additionalData = ((props?.customPages?.length || 0) > 0) ? { customPages: true } : undefined;Line 1061:
-...(props?.customMenuItems?.length || 0 > 0 ? { customItems: true } : undefined), +...((props?.customMenuItems?.length || 0) > 0 ? { customItems: true } : undefined),Without proper parentheses, the expression evaluates as
length || (0 > 0)instead of(length || 0) > 0.Also applies to: 880-880, 1061-1061
🧹 Nitpick comments (1)
packages/clerk-js/src/core/clerk.ts (1)
350-352: Consider improving clarity and terminology consistencyThe PR description mentions switching from "sampling" to "throttling" for keyless applications, but the variable name still uses "samplingRate". Consider either:
- Renaming to reflect throttling semantics (e.g.,
telemetryThrottleRate)- Adding a comment explaining that
1means no sampling (100% of events sent)Also, the conditional could be more explicit:
private get telemetrySamplingRate(): number | undefined { - return this.#options.__internal_keyless_claimKeylessApplicationUrl ? 1 : undefined; + // Return 1 (100% sampling/no throttling) for keyless apps, undefined (use default) for others + return this.#options.__internal_keyless_claimKeylessApplicationUrl ? 1 : undefined; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/clerk-js/src/core/clerk.ts(22 hunks)packages/shared/src/telemetry/events/component-mounted.ts(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/shared/src/telemetry/events/component-mounted.ts
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)
**/*.{js,jsx,ts,tsx}: All code must pass ESLint checks with the project's configuration
Follow established naming conventions (PascalCase for components, camelCase for variables)
Maintain comprehensive JSDoc comments for public APIs
Use dynamic imports for optional features
All public APIs must be documented with JSDoc
Provide meaningful error messages to developers
Include error recovery suggestions where applicable
Log errors appropriately for debugging
Lazy load components and features when possible
Implement proper caching strategies
Use efficient data structures and algorithms
Profile and optimize critical paths
Validate all inputs and sanitize outputs
Implement proper logging with different levels
Files:
packages/clerk-js/src/core/clerk.ts
**/*.{js,jsx,ts,tsx,json,css,scss,md,yaml,yml}
📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)
Use Prettier for consistent code formatting
Files:
packages/clerk-js/src/core/clerk.ts
packages/**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)
TypeScript is required for all packages
Files:
packages/clerk-js/src/core/clerk.ts
packages/**/*.{ts,tsx,d.ts}
📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)
Packages should export TypeScript types alongside runtime code
Files:
packages/clerk-js/src/core/clerk.ts
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)
Use proper TypeScript error types
**/*.{ts,tsx}: Always define explicit return types for functions, especially public APIs
Use proper type annotations for variables and parameters where inference isn't clear
Avoidanytype - preferunknownwhen type is uncertain, then narrow with type guards
Useinterfacefor object shapes that might be extended
Usetypefor unions, primitives, and computed types
Preferreadonlyproperties for immutable data structures
Useprivatefor internal implementation details
Useprotectedfor inheritance hierarchies
Usepublicexplicitly for clarity in public APIs
Preferreadonlyfor properties that shouldn't change after construction
Prefer composition and interfaces over deep inheritance chains
Use mixins for shared behavior across unrelated classes
Implement dependency injection for loose coupling
Let TypeScript infer when types are obvious
Useconst assertionsfor literal types:as const
Usesatisfiesoperator for type checking without widening
Use mapped types for transforming object types
Use conditional types for type-level logic
Leverage template literal types for string manipulation
Use ES6 imports/exports consistently
Use default exports sparingly, prefer named exports
Use type-only imports:import type { ... } from ...
Noanytypes without justification
Proper error handling with typed errors
Consistent use ofreadonlyfor immutable data
Proper generic constraints
No unused type parameters
Proper use of utility types instead of manual type construction
Type-only imports where possible
Proper tree-shaking friendly exports
No circular dependencies
Efficient type computations (avoid deep recursion)
Files:
packages/clerk-js/src/core/clerk.ts
**/*.{js,ts,tsx,jsx}
📄 CodeRabbit Inference Engine (.cursor/rules/monorepo.mdc)
Support multiple Clerk environment variables (CLERK_, NEXT_PUBLIC_CLERK_, etc.) for configuration.
Files:
packages/clerk-js/src/core/clerk.ts
**/*
⚙️ CodeRabbit Configuration File
If there are no tests added or modified as part of the PR, please suggest that tests be added to cover the changes.
Files:
packages/clerk-js/src/core/clerk.ts
🧬 Code Graph Analysis (1)
packages/clerk-js/src/core/clerk.ts (1)
packages/shared/src/telemetry/events/component-mounted.ts (2)
eventPrebuiltComponentOpened(79-86)eventPrebuiltComponentMounted(60-67)
⏰ 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). (4)
- GitHub Check: Build Packages
- GitHub Check: semgrep/ci
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (1)
packages/clerk-js/src/core/clerk.ts (1)
550-551: Consistent propagation of samplingRate to telemetry eventsAll telemetry event calls have been correctly updated to pass
this.telemetrySamplingRateas the 4th parameter, which aligns with the changes in the shared package. This ensures keyless applications will have their telemetry throttled appropriately.Also applies to: 574-575, 626-627, 708-709, 731-733, 763-765, 787-789, 803-804, 824-825, 848-849, 881-882, 921-923, 954-956, 987-997, 1031-1041, 1065-1067, 1086-1087, 1113-1114, 1182-1183, 1220-1222
packages/clerk-js/src/core/clerk.ts
Outdated
| private get telemetrySamplingRate(): number | undefined { | ||
| return this.#options.__internal_keyless_claimKeylessApplicationUrl ? 1 : undefined; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add tests for telemetry sampling rate logic
Consider adding tests to verify:
- The
telemetrySamplingRategetter returns1for keyless apps andundefinedfor regular apps - The sampling rate is correctly passed to all telemetry event methods
- The telemetry behavior differs appropriately between keyless and non-keyless applications
Example test structure:
describe('telemetry sampling', () => {
it('should use full sampling rate for keyless apps', () => {
const clerk = new Clerk(key, {
__internal_keyless_claimKeylessApplicationUrl: 'https://example.com'
});
// Verify telemetry calls receive samplingRate = 1
});
it('should use default sampling for regular apps', () => {
const clerk = new Clerk(key);
// Verify telemetry calls receive samplingRate = undefined
});
});🤖 Prompt for AI Agents
In packages/clerk-js/src/core/clerk.ts around lines 350 to 352, add unit tests
that verify the telemetrySamplingRate getter and its propagation: create a Clerk
instance with __internal_keyless_claimKeylessApplicationUrl set (keyless) and
assert telemetrySamplingRate === 1 and that all telemetry event calls receive
samplingRate = 1 (use spies/mocks on telemetry methods); create a Clerk instance
without that option and assert telemetrySamplingRate === undefined and telemetry
event calls receive samplingRate = undefined; ensure tests cover multiple
telemetry methods and both behaviors so keyless and non-keyless flows are
exercised.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/clerk-js/bundlewatch.config.json (2)
6-6: Tiny budget bump: confirm necessity and keep budgets tight57KB → 57.1KB is modest. Please confirm the measured compressed size for the headless bundle post-change and that this increase is strictly tied to the new telemetry parameter wiring. If we expect more incremental growth soon, consider either a slightly larger buffer or a follow-up cleanup to offset this growth.
1-35: Tests recommendation for telemetry changesThis PR appears to alter telemetry event APIs/usage (samplingRate propagation). If no tests were added, consider adding focused unit/integration tests to cover:
- eventPrebuiltComponentMounted/Open with explicit samplingRate
- defaulting/normalization behavior when samplingRate is undefined/invalid
- keyless flows where throttling vs sampling behavior differs
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/clerk-js/bundlewatch.config.json(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{js,jsx,ts,tsx,json,css,scss,md,yaml,yml}
📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)
Use Prettier for consistent code formatting
Files:
packages/clerk-js/bundlewatch.config.json
**/*
⚙️ CodeRabbit Configuration File
If there are no tests added or modified as part of the PR, please suggest that tests be added to cover the changes.
Files:
packages/clerk-js/bundlewatch.config.json
⏰ 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). (4)
- GitHub Check: Integration Tests (nextjs, chrome, 15)
- GitHub Check: Integration Tests (billing, chrome)
- GitHub Check: Integration Tests (nextjs, chrome, 14)
- GitHub Check: Integration Tests (generic, chrome)
panteliselef
force-pushed
the
elef/user-3116-increase-telemetry-sampling-for-aios-while-on-keyless
branch
from
0ca6c52 to
0bbf54e
Compare
…or-aios-while-on-keyless
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (2)
packages/clerk-js/src/core/clerk.ts (2)
657-658: Avoid useless template literals and align with component variable patternUse plain strings and/or a local component variable, like other flows.
Apply:
- this.telemetry?.record(eventPrebuiltComponentOpened(`UserVerification`, props)); + const component = 'UserVerification'; + this.telemetry?.record(eventPrebuiltComponentOpened(component, props));
1-2897: Replace backtick-quoted component names in telemetry callsWe found one leftover usage of backticks around the component name in a telemetry call. Please update it to use a plain string literal:
• packages/clerk-js/src/core/clerk.ts:657
- this.telemetry?.record(eventPrebuiltComponentOpened(`UserVerification`, props)); + this.telemetry?.record(eventPrebuiltComponentOpened('UserVerification', props));
🧹 Nitpick comments (1)
packages/clerk-js/src/core/clerk.ts (1)
438-446: Wire perEventSampling for keyless initializationSetting perEventSampling to false when __internal_keyless_claimKeylessApplicationUrl is present implements the “throttling instead of per-event sampling” goal. Consider adding a small unit/integration test under clerk-js to verify the wiring for keyless vs non-keyless.
If helpful, I can scaffold a test that asserts TelemetryCollector receives perEventSampling=false for keyless and undefined for non-keyless.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
.changeset/slow-shoes-give.md(1 hunks)packages/clerk-js/src/core/clerk.ts(8 hunks)packages/shared/src/__tests__/telemetry.test.ts(1 hunks)packages/shared/src/telemetry/collector.ts(3 hunks)packages/shared/src/telemetry/events/component-mounted.ts(1 hunks)packages/shared/src/telemetry/types.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/shared/src/telemetry/events/component-mounted.ts
- .changeset/slow-shoes-give.md
🧰 Additional context used
📓 Path-based instructions (9)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)
**/*.{js,jsx,ts,tsx}: All code must pass ESLint checks with the project's configuration
Follow established naming conventions (PascalCase for components, camelCase for variables)
Maintain comprehensive JSDoc comments for public APIs
Use dynamic imports for optional features
All public APIs must be documented with JSDoc
Provide meaningful error messages to developers
Include error recovery suggestions where applicable
Log errors appropriately for debugging
Lazy load components and features when possible
Implement proper caching strategies
Use efficient data structures and algorithms
Profile and optimize critical paths
Validate all inputs and sanitize outputs
Implement proper logging with different levels
Files:
packages/shared/src/telemetry/types.tspackages/shared/src/__tests__/telemetry.test.tspackages/shared/src/telemetry/collector.tspackages/clerk-js/src/core/clerk.ts
**/*.{js,jsx,ts,tsx,json,css,scss,md,yaml,yml}
📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)
Use Prettier for consistent code formatting
Files:
packages/shared/src/telemetry/types.tspackages/shared/src/__tests__/telemetry.test.tspackages/shared/src/telemetry/collector.tspackages/clerk-js/src/core/clerk.ts
packages/**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)
TypeScript is required for all packages
Files:
packages/shared/src/telemetry/types.tspackages/shared/src/__tests__/telemetry.test.tspackages/shared/src/telemetry/collector.tspackages/clerk-js/src/core/clerk.ts
packages/**/*.{ts,tsx,d.ts}
📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)
Packages should export TypeScript types alongside runtime code
Files:
packages/shared/src/telemetry/types.tspackages/shared/src/__tests__/telemetry.test.tspackages/shared/src/telemetry/collector.tspackages/clerk-js/src/core/clerk.ts
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)
Use proper TypeScript error types
**/*.{ts,tsx}: Always define explicit return types for functions, especially public APIs
Use proper type annotations for variables and parameters where inference isn't clear
Avoidanytype - preferunknownwhen type is uncertain, then narrow with type guards
Useinterfacefor object shapes that might be extended
Usetypefor unions, primitives, and computed types
Preferreadonlyproperties for immutable data structures
Useprivatefor internal implementation details
Useprotectedfor inheritance hierarchies
Usepublicexplicitly for clarity in public APIs
Preferreadonlyfor properties that shouldn't change after construction
Prefer composition and interfaces over deep inheritance chains
Use mixins for shared behavior across unrelated classes
Implement dependency injection for loose coupling
Let TypeScript infer when types are obvious
Useconst assertionsfor literal types:as const
Usesatisfiesoperator for type checking without widening
Use mapped types for transforming object types
Use conditional types for type-level logic
Leverage template literal types for string manipulation
Use ES6 imports/exports consistently
Use default exports sparingly, prefer named exports
Use type-only imports:import type { ... } from ...
Noanytypes without justification
Proper error handling with typed errors
Consistent use ofreadonlyfor immutable data
Proper generic constraints
No unused type parameters
Proper use of utility types instead of manual type construction
Type-only imports where possible
Proper tree-shaking friendly exports
No circular dependencies
Efficient type computations (avoid deep recursion)
Files:
packages/shared/src/telemetry/types.tspackages/shared/src/__tests__/telemetry.test.tspackages/shared/src/telemetry/collector.tspackages/clerk-js/src/core/clerk.ts
**/*.{js,ts,tsx,jsx}
📄 CodeRabbit Inference Engine (.cursor/rules/monorepo.mdc)
Support multiple Clerk environment variables (CLERK_, NEXT_PUBLIC_CLERK_, etc.) for configuration.
Files:
packages/shared/src/telemetry/types.tspackages/shared/src/__tests__/telemetry.test.tspackages/shared/src/telemetry/collector.tspackages/clerk-js/src/core/clerk.ts
**/*
⚙️ CodeRabbit Configuration File
If there are no tests added or modified as part of the PR, please suggest that tests be added to cover the changes.
Files:
packages/shared/src/telemetry/types.tspackages/shared/src/__tests__/telemetry.test.tspackages/shared/src/telemetry/collector.tspackages/clerk-js/src/core/clerk.ts
packages/**/*.{test,spec}.{js,jsx,ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/monorepo.mdc)
Unit tests should use Jest or Vitest as the test runner.
Files:
packages/shared/src/__tests__/telemetry.test.ts
**/__tests__/**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/typescript.mdc)
**/__tests__/**/*.{ts,tsx}: Create type-safe test builders/factories
Use branded types for test isolation
Implement proper mock types that match interfaces
Files:
packages/shared/src/__tests__/telemetry.test.ts
🧬 Code Graph Analysis (2)
packages/shared/src/__tests__/telemetry.test.ts (1)
packages/shared/src/telemetry/collector.ts (1)
TelemetryCollector(72-310)
packages/clerk-js/src/core/clerk.ts (1)
packages/shared/src/telemetry/events/component-mounted.ts (2)
eventPrebuiltComponentOpened(70-76)eventPrebuiltComponentMounted(53-59)
⏰ 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). (23)
- GitHub Check: Integration Tests (react-router, chrome)
- GitHub Check: Integration Tests (nextjs, chrome, 15)
- GitHub Check: Integration Tests (billing, chrome)
- GitHub Check: Integration Tests (nuxt, chrome)
- GitHub Check: Integration Tests (nextjs, chrome, 14)
- GitHub Check: Integration Tests (expo-web, chrome)
- GitHub Check: Integration Tests (vue, chrome)
- GitHub Check: Integration Tests (tanstack-react-router, chrome)
- GitHub Check: Integration Tests (localhost, chrome)
- GitHub Check: Integration Tests (quickstart, chrome)
- GitHub Check: Integration Tests (tanstack-react-start, chrome)
- GitHub Check: Integration Tests (sessions, chrome)
- GitHub Check: Integration Tests (ap-flows, chrome)
- GitHub Check: Integration Tests (generic, chrome)
- GitHub Check: Integration Tests (elements, chrome)
- GitHub Check: Integration Tests (astro, chrome)
- GitHub Check: Integration Tests (express, chrome)
- GitHub Check: Publish with pkg-pr-new
- GitHub Check: Unit Tests (18, --filter=@clerk/astro --filter=@clerk/backend --filter=@clerk/express --filter=@c...
- GitHub Check: Unit Tests (22, **)
- GitHub Check: Static analysis
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (14)
packages/shared/src/telemetry/types.ts (2)
11-13: JSDoc punctuation fix looks goodMinor doc polish; no functional impact.
15-19: New public option perEventSampling?: boolean — API shape LGTMClear semantics and default noted. This pairs well with collector defaulting to true and conditional overrides.
packages/shared/src/telemetry/collector.ts (3)
47-52: Expose perEventSampling in TelemetryCollectorConfigType update correctly surfaces the flag for initialization.
80-87: Default perEventSampling to true in constructorMatches the public JSDoc default. Good defensive defaulting.
169-174: Sampling gate correctly short-circuits per-event rates when perEventSampling is falseOrder of checks is correct: global sampling, then per-event if enabled, then throttling. Tests cover both paths.
packages/shared/src/__tests__/telemetry.test.ts (2)
204-224: Test: perEventSampling=false bypasses per-event rateGood coverage. Server-path forcing via window undefined and random seed mocking is appropriate.
226-245: Test: perEventSampling=true enforces per-event rateComplements the previous test; ensures default behavior is preserved.
packages/clerk-js/src/core/clerk.ts (7)
541-548: Standardize component identifier for GoogleOneTapConsistent use of a local component variable for preloadHint and telemetry. Good.
565-572: SignIn open flow: consistent component naming and additionalDataLocal component name improves consistency; additionalData correctly captures withSignUp state.
618-624: PlanDetails open flow: component variable + telemetryLooks consistent with the new pattern.
725-726: Operator precedence fix for customPages lengthThe explicit comparison avoids precedence pitfalls. Good catch.
802-814: SignIn mount flow: component variable across preloadHint, name, and telemetryConsistent and clear.
827-838: SignUp mount flow normalizationGood standardization.
859-871: UserProfile mount: component variable + explicit customPages checkMatches the new conventions and fixes the earlier precedence issue.
…or-aios-while-on-keyless
panteliselef
deleted the
elef/user-3116-increase-telemetry-sampling-for-aios-while-on-keyless
branch
Description
Switch to throttling instead of sampling for telemetry on keyless applications to help us better troubleshoot keyless issues.
Checklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change
Summary by CodeRabbit
New Features
Tests
Chores