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

Appending info to payload should happen in/around config, not in controller. #94

Closed
smudge opened this issue Nov 19, 2014 · 5 comments
Closed

Comments

@smudge
Copy link
Contributor

smudge commented Nov 19, 2014

Something about the way I have to append additional info to the payload (i.e. overriding append_info_to_payload inside of my ApplicationController) feels messy. It requires defining the same custom options in 2 different locations, very far apart. This is something I'd much prefer to do right next to where I configure lograge, or at least in an initializer, so that when I (or a team member) come back to this in 6 months it isn't totally confusing.

For now, I have an initializer (lograge.rb) set up that looks like this (>= ruby 2.0):

module LogrageControllerOverride
  def append_info_to_payload(payload)
    super
    payload[:host] = request.host
    payload[:fwd] = request.remote_ip
    payload[:user_id] = current_user.try(:id)
  end
end

# should probably do this only if Rails.env is production
ActionController::Base.send :prepend, LogrageControllerOverride

And then in config/environments/production.rb I have this:

config.lograge.enabled = true
config.lograge.custom_options = lambda do |event|
  {
    time: event.time.to_i,
    host: event.payload[:host], # see: config/initializers/lograge.rb
    fwd: event.payload[:fwd].try(:inspect),
    user_id: event.payload[:user_id]
  }
end

But I'd still prefer to be able to do all of this from config/environments/production.rb along with the above config code. As in, specify a proc which will be called in the context of the controller, so I'd have direct access to the usual things like request and current_user. Does that make sense?

@smudge
Copy link
Contributor Author

smudge commented Dec 9, 2014

If this issue is worth pursuing, I can come up with a draft and send a pull request. I've recently done something similar as part of the configuration for a gem I am working on. (Let me know if there is any interest in this.)

@benlovell
Copy link
Collaborator

Hey @smudge. Somehow I missed your initial issue. I'd definitely entertain anything you'd come up with. 👍

@pxlpnk
Copy link
Collaborator

pxlpnk commented Feb 21, 2015

@smudge did you have some time to play around with this?

@benlovell
Copy link
Collaborator

@smudge?

@smudge
Copy link
Contributor Author

smudge commented Jun 18, 2015

(Bueller...)

@pxlpnk pxlpnk closed this as completed Aug 4, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants