-
-
Notifications
You must be signed in to change notification settings - Fork 4.5k
feat(onboarding): Add metrics onboarding for the Python SDK #102641
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
base: master
Are you sure you want to change the base?
Conversation
❌ 15 Tests Failed:
View the top 3 failed test(s) by shortest run time
To view more test analytics, go to the Test Analytics Dashboard |
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.
Looks good to me 🚀 !
Not approving yet, I want to give the more experienced onboarders a chance to review.
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.
Bug: Undefined valueType triggers unintended filter initialization paths
The condition change from valueType && valueType !== FieldValueType.STRING to fieldDefinition?.valueType !== FieldValueType.STRING causes the code block to execute when fieldDefinition?.valueType is undefined. This previously skipped block now calls getInitialFilterText with undefined parameters, which could lead to unexpected behavior or runtime errors.
static/app/views/dashboards/globalFilter/addFilter.tsx#L130-L131
sentry/static/app/views/dashboards/globalFilter/addFilter.tsx
Lines 130 to 131 in 8787b29
| if (fieldDefinition?.valueType !== FieldValueType.STRING) { |
| description={t( | ||
| 'Structured application metrics for debugging and troubleshooting. Automatically gets associated with errors and traces.' | ||
| )} | ||
| docLink="https://docs.sentry.io/product/explore/metrics/" |
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.
This redirects to span metrics right now. Not sure which page to link, but this should be addressed here or in the docs repo before merging as well.
| <Product | ||
| label={t('Metrics')} | ||
| description={t( | ||
| 'Structured application metrics for debugging and troubleshooting. Automatically gets associated with errors and traces.' |
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.
Open to suggestions, I didn't change much from the description we have for logs.
Closes LOGS-458