-
-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
ref(onboardingtasks): Add more instrumentation to help investigate bug #85826
base: master
Are you sure you want to change the base?
ref(onboardingtasks): Add more instrumentation to help investigate bug #85826
Conversation
priscilawebdev
commented
Feb 25, 2025
- Contributes to https://github.com/getsentry/projects/issues/720
sentry_sdk.capture_message( | ||
f"First project task not created for org {organization_id}, cache key {cache_key} set", | ||
level="warning", | ||
) |
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.
removing this as it is super noisy!
@@ -74,14 +76,6 @@ def record(self, organization_id, task, **kwargs): | |||
level="warning", | |||
) | |||
pass | |||
|
|||
# Store marker to prevent running all the time | |||
cache.set(cache_key, 1, 3600) |
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.
We should only set the cache if the task was successfully created
if created and task_id == OnboardingTask.FIRST_PROJECT: | ||
scope = sentry_sdk.get_current_scope() | ||
scope.set_extra("org", organization.ids) | ||
sentry_sdk.capture_message( | ||
f"Onboarding task {task_id} was created unexpectedly. It should have been updated instead.", | ||
level="warning", | ||
) |
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 shouldn't happen since we filter tasks by their status on the frontend, which should be either skipped or complete (see), but let's see what's going on here
Codecov ReportAll modified and coverable lines are covered by tests ✅ ✅ All tests successful. No failed tests found. Additional details and impacted files@@ Coverage Diff @@
## master #85826 +/- ##
==========================================
- Coverage 87.88% 87.88% -0.01%
==========================================
Files 9683 9683
Lines 548959 548962 +3
Branches 21317 21317
==========================================
+ Hits 482477 482479 +2
- Misses 66175 66176 +1
Partials 307 307 |
if created and task_id == OnboardingTask.FIRST_PROJECT: | ||
scope = sentry_sdk.get_current_scope() | ||
scope.set_extra("org", organization.id) | ||
sentry_sdk.capture_message( | ||
f"Onboarding task {task_id} was created unexpectedly. It should have been updated instead.", | ||
level="warning", | ||
) |
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.
If the task should not be create here, but only updated, why are we calling create_or_update in the method?
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 thought about replacing it with update
only but I was afraid of causing issues, so instead I decided to first add the capture_message
and see if this is actually happening - I know it does happen for tests but that is ok