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

Adding a hook to allow custom data to the rage line #4

Merged
merged 4 commits into from Mar 28, 2012
Merged

Adding a hook to allow custom data to the rage line #4

merged 4 commits into from Mar 28, 2012

Conversation

adamcooper
Copy link
Contributor

This adds a new hook that allows someone to add additional logging onto the one line log.

I realize that this is pretty opinionated gem but I figured there are a couple cases where putting a bit of extra information may be nice. The use case I need it for was to put a bit of information about GC stats run and I wanted to be able to easily grep and look at this data from the log file.

Here is how you would add an extra bit of information to the log line.

# config/environments/staging.rb
MyApp::Application.configure do
  config.lograge.enabled = true
  config.lograge.extra = lambda do |event|
    # some custom details
    " custom data like GC stats"
  end
end

This patch also includes specs and doc updates.

BTW, There is another hidden feature in here and that is a fix to issue #1. At least in development mode. It should also work in the case where rack_cache == false. This is in it's own commit so you can take one without other.

@roidrage
Copy link
Owner

This is great, thanks for the pull request. I wanted to add custom options at some point too, so this is neat!

I have two suggestions:

  • I think the extra info should be required to be a hash, so that it falls in line with the rest of code and allows for easy extension of a log line with any kind of information. It could then just be turned into key=value pairs too.
  • The name extra could be improved. I'm at a loss for more appropriate names currently, something like optional or added comes to mind, but in general I'd prefer the name to be a bit more expressive. Any ideas?

@airblade
Copy link

Excuse me for interrupting...

@MatMatt Regarding your second point, why not call it custom_options (the language you used yourself at the start of your comment) or custom_details (the language used in the code comment in the original comment)?

I'll be quiet now.

@adamcooper
Copy link
Contributor Author

@mattmatt - Just to be clear, you are suggesting the return value from the lambda be hash. Correct? Not that the lambda itself should be hash. Although, when I was writing it I had the desire to support multiple custom_options such as Hash and something that responds to call.

I think the hash will help enforce the output of the lograge line.

@airblade - I welcome the intrusion because I was really at a loss for a name too which is why it ended up as extra. I like custom_options.

@roidrage
Copy link
Owner

@adamcooper yeah, the lambda return value should be a hash, not the extra or whatever it's going to be called, that'd make things a bit less dynamic :)

Regarding the name, custom_options sounds good to me, thanks for the suggestion, @airblade!

@adamcooper
Copy link
Contributor Author

This should be good to go now.

  • Changed the name to custom_options
  • Changed return format to a Hash
  • Updated specs and docs
  • I also silenced a couple other action_controller subscriptions methods.

@roidrage
Copy link
Owner

Looks awesome, thanks so much!

roidrage added a commit that referenced this pull request Mar 28, 2012
Adding a hook to allow custom data to the rage line
@roidrage roidrage merged commit ee23117 into roidrage:master Mar 28, 2012
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.

None yet

3 participants