-
-
Notifications
You must be signed in to change notification settings - Fork 522
Implement new Sentry.metrics functionality
#2818
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
Conversation
ec2ab74 to
ee24a43
Compare
Sentry.metrics functionalitySentry.metrics functionality
|
ee24a43 to
79f73e3
Compare
79f73e3 to
892aef6
Compare
| # | ||
| # @param telemetry [MetricEvent] | ||
| # @return [MetricEvent] | ||
| def apply_to_telemetry(telemetry) |
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.
logs will also go through this instead of apply_to_event in a subsequent refactor PR
solnic
left a comment
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.
@sl0thentr0py I'm assuming we're gonna consolidate this implementation with Logs as there's way too much duplication now
| "server.address" => configuration.server_name | ||
| }.compact | ||
|
|
||
| @attributes = default_attributes.merge(@attributes) |
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 could optimize this and make @attributes's default value to be the default attributes, then it won't allocate a default empty hash, and create intermediate hash just to merge defaults into the empty one.
This should remain lazy of course so that we're not building up the final attributes until serialization is invoked.
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'm gonna move this logic to scope after unifying with logs
Co-authored-by: Peter Solnica <peter@solnica.online>
Description
Describe your changes:
Issues