-
-
Notifications
You must be signed in to change notification settings - Fork 4k
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(metrics): first custom metric sent signal #71096
Conversation
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 need to register this event like this?
from sentry import analytics |
Also, that event is in amplitude visible as "Sent First Replay". When I searched sentry repos, I found this mapping: |
👍 |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #71096 +/- ##
==========================================
- Coverage 77.87% 77.87% -0.01%
==========================================
Files 6529 6529
Lines 290909 290912 +3
Branches 50338 50339 +1
==========================================
- Hits 226550 226544 -6
- Misses 58120 58125 +5
- Partials 6239 6243 +4
|
|
||
# We assume that the flag update is reflected in the cache, so that upcoming calls will get the up-to- | ||
# date project with the `has_custom_metrics` flag set to true. | ||
project.update(flags=F("flags").bitor(Project.flags.has_custom_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.
If we don't update this flag anymore, we will run the amplitude event for every incoming custom metric of a project that never set that flag before. Do we want that?
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.
Missed the other code, nvm.
@@ -318,6 +320,31 @@ def record_first_cron_checkin(project, monitor_id, **kwargs): | |||
) | |||
|
|||
|
|||
@first_custom_metric_received.connect(weak=False) | |||
def record_first_custom_metric(project, **kwargs): | |||
project.update(flags=F("flags").bitor(Project.flags.has_custom_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.
@iambriccardo we update the flag here
Suspect IssuesThis pull request was deployed and Sentry observed the following issues:
Did you find this useful? React with a 👍 or 👎 |
Moves project flag update logic and event recording from metrics billing consumer into the signal receiver for consistency with other project flags. Records analytics event and temporarily keeps
sentry_sdk.capture_message