Skip to content
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

Feature to calculate db_payload_size and response_payload_size #3

Closed
wants to merge 37 commits into from

Conversation

prog-supdex
Copy link

@prog-supdex prog-supdex commented Mar 30, 2022

Hello, @DmitryTsepelev !

#1

Here I add railties for adding logs about Db payload size and response payload size. And we`ll see in logs like this

Completed 200 OK in 114ms (Views: 44.4ms | ActiveRecord: 13.8ms | DB Payload: 69.3kb | Body Payload: 29.3kb | Allocations: 12382)

Also, this information (db_payload_size and response_payload_size) added to 'process_action.action_controller' of ActiveNotification

I patched ActiveRecord::Relation and ActiveRecord::Calculations, for calculate bytes of result (AR extension)

DB payload size

For response_body I added railtie ControllerResponsePayloadSize and calculate response_body before render

For calculating I used Marchal.dump(some_object).size
If result contains AR object (#<Transaction id: ..., token: ....>), that means more allocated memory, and we will calculate this, not only rows data.
Difference between (for example)

Transaction.select(:token)

and

Transaction.pluck(:token)

Before, I also tried to use ObjectSpace.memsize_of, but this is not performance, because we need to find all related objects and caculate them

if ratio less than threshold, will send notificaion (of log, dependencies configuration) with message 'I/O to response payload ratio is ratio, while threshold is threshold'

Also, we can configure own notifications, by using add_notification, for example

IoToResponsePayloadRatio.configure do |config|
  config.add_notification(slack: SomeClass::Slack)
end

For monitoring data, include to controller

class TransactionsController < ApplicationController
  include IoToResponsePayloadRatio::Controller
end

And, I am ready for fixing code, if needed)

…name method reset_allocated_memory to reset_db_payload_size
…sponse payload size before convert to json in render
Copy link
Owner

@DmitryTsepelev DmitryTsepelev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! I decided to merge #4 because I reviewed it earlier, your approaches are almost same 🙂 I listed you as a solution author on the Cult and will put it to the readme and changelog now.

May I ask you to port your code related to #add_notification?

@prog-supdex
Copy link
Author

LGTM! I decided to merge #4 because I reviewed it earlier, your approaches are almost same slightly_smiling_face I listed you as a solution author on the Cult and will put it to the readme and changelog now.

May I ask you to port your code related to #add_notification?

Yeah, of course, I will create new PR and will integrate #add_notification

@prog-supdex
Copy link
Author

@DmitryTsepelev
oh, I see, that in current version of gem You can set custom publisher(notification) and my add_notification, unfortunately, doesn't make sense

@prog-supdex prog-supdex closed this Apr 1, 2022
@DmitryTsepelev
Copy link
Owner

Got it! Thanks for the hard work again, and please let me know if you have any other ideas for the lib!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants