feat(onboarding): Add metrics onboarding for JS SDKs #102775
Conversation
github-actions
bot
added
the
Scope: Frontend
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Bug: Feature flag gap for METRICS product selection
The getDisabledProducts function doesn't include a feature flag check for ProductSolution.METRICS, even though METRICS is now added to the product selection UI and platform availability lists. This is inconsistent with how other products (SESSION_REPLAY, PERFORMANCE_MONITORING, PROFILING, LOGS) are handled, where each has a corresponding feature flag check. Without this check, users could potentially select METRICS even if their organization doesn't have the feature enabled, leading to broken functionality or misleading UI state.
static/app/components/onboarding/productSelection.tsx#L31-L80
sentry/static/app/components/onboarding/productSelection.tsx
Lines 31 to 80 in 3b702e8
There was a problem hiding this comment.
Bug: Metrics access bypasses feature flag check
The getDisabledProducts function does not check for a metrics feature flag before allowing users to select the Metrics product. The function checks for feature flags for SESSION_REPLAY (session-replay), PERFORMANCE_MONITORING (performance-view), PROFILING (profiling-view), and LOGS (ourlogs-enabled), but is missing a similar check for METRICS. This means users can select Metrics during onboarding even if their organization doesn't have the metrics feature enabled, which is inconsistent with how other products are handled and could lead to unexpected behavior.
static/app/components/onboarding/productSelection.tsx#L30-L77
sentry/static/app/components/onboarding/productSelection.tsx
Lines 30 to 77 in 7eef636
chargome
force-pushed
the
cg-metrics-onboarding-js
branch
from
7eef636 to
532707e
Compare
static/app/gettingStartedDocs/javascript/react-router/onboarding.spec.tsx
Show resolved
Hide resolved
❌ 6 Tests Failed:
View the top 3 failed test(s) by shortest run time
To view more test analytics, go to the Test Analytics Dashboard |
# Conflicts: # static/app/gettingStartedDocs/javascript/gatsby.tsx # static/app/gettingStartedDocs/javascript/react.tsx # static/app/gettingStartedDocs/javascript/svelte.tsx # static/app/gettingStartedDocs/javascript/tanstackstart-react.tsx # static/app/gettingStartedDocs/javascript/vue.tsx
There was a problem hiding this comment.
Bug: Metrics available despite missing feature.
The getDisabledProducts function doesn't check for the custom-metrics feature flag, even though ProductSolution.METRICS is now available for selection on multiple platforms. Users without the metrics feature enabled can still select it during onboarding, leading to broken functionality. The function should check organization.features.includes('custom-metrics') and add ProductSolution.METRICS to disabled products when not available.
static/app/components/onboarding/productSelection.tsx#L29-L80
sentry/static/app/components/onboarding/productSelection.tsx
Lines 29 to 80 in dc1ed28
|
hey @chargome , sorry for the many conflicts I caused. I have 2x more JS split PRs to go and as soon as I merge them, I promise to solve all the conflicts 😉 |
There was a problem hiding this comment.
Bug: Enable Logs for Server Setup
The getSdkServerSetupSnippet function is missing enableLogs configuration when params.isLogsSelected is true. Other full-stack frameworks like Astro include this configuration in their server setup. The function should add enableLogs: true conditionally based on params.isLogsSelected, similar to how it handles performance and profiling configurations.
static/app/gettingStartedDocs/javascript/solidstart/onboarding.tsx#L12-L44
There was a problem hiding this comment.
Bug: Incomplete Remix Onboarding Experience
The nextSteps function was removed but not replaced, leaving the Remix onboarding configuration without any next steps. This breaks consistency with other similar full-stack frameworks and prevents users from seeing the Metrics documentation link when metrics are selected during onboarding.
static/app/gettingStartedDocs/javascript/remix/onboarding.tsx#L88-L90
sentry/static/app/gettingStartedDocs/javascript/remix/onboarding.tsx
Lines 88 to 90 in df73767
There was a problem hiding this comment.
Bug: Inconsistent Remix Onboarding Next Steps
The nextSteps property was removed from the remix onboarding config. According to the OnboardingConfig type definition, nextSteps is optional but when present should be a generator function that returns an array of next steps based on DocsParams. Other similar files in the PR (like nextjs, nuxt, sveltekit) still have nextSteps that returns an empty array when no conditional steps apply. This inconsistency may cause issues if the onboarding renderer expects this property to exist, and it prevents adding metrics-related next steps like other platforms support.
static/app/gettingStartedDocs/javascript/remix/onboarding.tsx#L89-L90
sentry/static/app/gettingStartedDocs/javascript/remix/onboarding.tsx
Lines 89 to 90 in 66f1569
Bug: Nuxt: Inconsistent metrics documentation onboarding.
The nextSteps function doesn't accept params and conditionally add metrics documentation when metrics are selected. Other meta-frameworks in this PR (like astro, solidstart, react, etc.) that receive metricsOnboarding support also update their nextSteps to conditionally include metrics documentation based on params.isMetricsSelected. Nuxt receives metricsOnboarding in its index file but the onboarding config doesn't follow this pattern, creating inconsistency.
static/app/gettingStartedDocs/javascript/nuxt/onboarding.tsx#L87-L95
There was a problem hiding this comment.
Bug: Misleading verification text for metrics.
The verify text only mentions "test log" when isLogsSelected is true, but doesn't account for when isMetricsSelected is true without logs. When metrics are selected (but not logs), the text should mention "test metric" as well, but currently it will only say "trigger a test error" even though the code snippet includes Sentry.metrics.count(). The condition should check both params.isLogsSelected and params.isMetricsSelected to provide accurate user guidance.
static/app/gettingStartedDocs/javascript/angular/onboarding.tsx#L170-L179
sentry/static/app/gettingStartedDocs/javascript/angular/onboarding.tsx
Lines 170 to 179 in 98792e3
| 'node-cloudflare-pages': [ | ||
| ProductSolution.PERFORMANCE_MONITORING, | ||
| ProductSolution.LOGS, | ||
| ProductSolution.METRICS, |
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
resolves https://linear.app/getsentry/issue/LOGS-378/uisdk-metrics-empty-state-project-onboarding --------- Co-authored-by: Priscila Oliveira <[email protected]>
resolves https://linear.app/getsentry/issue/LOGS-378/uisdk-metrics-empty-state-project-onboarding --------- Co-authored-by: Priscila Oliveira <[email protected]>
resolves https://linear.app/getsentry/issue/LOGS-378/uisdk-metrics-empty-state-project-onboarding --------- Co-authored-by: Priscila Oliveira <[email protected]>
resolves https://linear.app/getsentry/issue/LOGS-378/uisdk-metrics-empty-state-project-onboarding