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

Rails controller runtime logging #4446

Merged
merged 4 commits into from Nov 27, 2018
Merged

Rails controller runtime logging #4446

merged 4 commits into from Nov 27, 2018

Conversation

doooby
Copy link
Contributor

@doooby doooby commented Sep 8, 2017

Hi there, I'm bringing back an issue about logging mongoid runtime per rails controller action.

there was #3885 that failed to get into mongoid, not sure why, but now that code is incompatible.

the log message Completed 200 OK in 52ms (Views: 0.2ms | ActiveRecord: 0.4ms | MongoDB: 0.1ms) is nice but marginal feature.

we needed to collect some metrics about our rails application; this allows us to use standard rails notification:

ActiveSupport::Notifications.subscribe 'process_action.action_controller' do |*args|
  data = args.extract_options!
  data[:mongo_runtime] # accessible here
end

and please, would it be possible to put it in older versions too? (we're currently on mongoid 5.1)

@estolfo
Copy link
Contributor

estolfo commented Sep 29, 2017

Hi @doooby
Thank you very much from your pull request! Would you mind adding a few lines of documentation in the code and to our tutorial?
And a few tests would be great as well.

Thanks!
Emily

@doooby
Copy link
Contributor Author

doooby commented Oct 24, 2017

sure, when i have the time, i'll do.


protected

attr_internal :mongo_runtime
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this should be either "mongoid_runtime" or "mongodb_runtime"

@gnumarcelo
Copy link

Any chance this being merged soon?

@estolfo
Copy link
Contributor

estolfo commented Mar 13, 2018

I'd like some documentation and tests before this is merged. Maybe @doooby has some time?

@doooby
Copy link
Contributor Author

doooby commented Mar 19, 2018

Sorry, I hadn't had the time for this. And for more, the reasons of the previous Travis fail goes over my understanding. But here, I've polished it a bit, added some test that made sense to me and added a mention to the doc.

also merged master from upstream.

@p-mongo p-mongo self-assigned this Jul 11, 2018
@p-mongo p-mongo removed their assignment Aug 14, 2018
@p-mongo
Copy link
Contributor

p-mongo commented Oct 21, 2018

Tracking ticket: https://jira.mongodb.org/browse/MONGOID-4631

@p-mongo
Copy link
Contributor

p-mongo commented Nov 26, 2018

The diff references ::Mongoid::Railties::ControllerRuntime::Instrument which does not appear to be defined anywhere, and the code that references doesn't seem to be covered by tests either.

@p-mongo p-mongo requested a review from saghm November 26, 2018 23:50
@p-mongo p-mongo merged commit 6b35ed5 into mongodb:master Nov 27, 2018
p-mongo pushed a commit that referenced this pull request Nov 27, 2018
* Rails controller runtime logging

* Rails instrumentation of Mongoid runtime during action processing

* Tweak documentation wording

* Repair the code and expand documentation
p-mongo pushed a commit that referenced this pull request Nov 27, 2018
* Rails controller runtime logging

* Rails instrumentation of Mongoid runtime during action processing

* Tweak documentation wording

* Repair the code and expand documentation
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
7 participants