Skip to content
This repository

Adding new channel_logger plugin as a plugin example. #118

Closed
wants to merge 1 commit into from

2 participants

David Campbell Dominik Honnef
David Campbell
dacamp commented

I'd like to actually fix the core of the issue in the Logger(s) classes, but right now I don't have time. This will have to do as an example on opening a log for each channel the bot is in. It also creates one log for all private messages. By design connection logging still goes to Logger.

Thanks!

Dominik Honnef
Owner

This could already be implemented with a custom logger, so I'm not sure what you'd want to fix in the core Logger class.

Dominik Honnef dominikh commented on the diff
examples/plugins/channel_logger.rb
... ... @@ -0,0 +1,100 @@
  1 +# This plugin logs private messages and each channel seperately.
  2 +# Intended use would be to pipe Cinch::Loggers STDOUT/STDERR to a
  3 +# debug log and ChannelLogger mail to will handle the rest.
1
Dominik Honnef Owner
dominikh added a note

mail to?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Dominik Honnef dominikh commented on the diff
examples/plugins/channel_logger.rb
... ... @@ -0,0 +1,100 @@
  1 +# This plugin logs private messages and each channel seperately.
  2 +# Intended use would be to pipe Cinch::Loggers STDOUT/STDERR to a
  3 +# debug log and ChannelLogger mail to will handle the rest.
  4 +# I'm sure fixing up the Cinch::Logger(s)? class is the solution,
  5 +# but this is a good temporary fix.
  6 +
  7 +require 'cinch'
  8 +
  9 +# Message instance variable message needs to be writeable
  10 +module Cinch; class Message; attr_accessor :message ; end; end
1
Dominik Honnef Owner
dominikh added a note

I definitely will not include an example that modifies the core classes of Cinch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Dominik Honnef dominikh commented on the diff
examples/plugins/channel_logger.rb
((26 lines not shown))
  26 + @logs = { :privmsg =>File.open(config[:logfile], "a") }
  27 + @logs[:privmsg].sync = true
  28 + @logdir = File.dirname(config[:logfile])
  29 + @timeformat = config[:timeformat] || "%Y/%m/%d %H:%M:%S"
  30 + @logformat = config[:format] || "[%{time}] %{channel} %{nick}: %{msg}"
  31 + @last_time_check = Time.now
  32 +
  33 + bot.debug("Opened message logfile at #{config[:logfile]}")
  34 + end
  35 +
  36 + def handle_notice(msg, *args)
  37 + h = msg.raw.strip.split(/\s+/)[1]
  38 + if h =~ /JOIN/ and msg.channel
  39 + open_log(msg.channel.name)
  40 + end
  41 + msg.message = "-!- [#{h.to_s}] #{msg.user.nick}"
1
Dominik Honnef Owner
dominikh added a note

So all plugins that run after this one get a butchered message?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
David Campbell dacamp closed this
David Campbell
dacamp commented

Yeah overwriting the attribute was dumb and entirely over complicated. I'm going to blame whiskey brain, but in reality I just wasn't thinking... Actually now that I look back at your Logger class I see that it's totally possible to achieve my goal with a custom formatted logger (or not) and your current classes... So, suffice to say this entire plugin is pretty much garbage, I'll close this request. I'll go repent and attempt to cleanse myself of sin. I do think a good example of channel logging is important, I'll try and work on it sometime this week.

It would be nice though to be able to name a Logger though... How opposed would you be to LoggerList becoming a hash rather than an array? It seems like with your current methods, backwards compatibility could still be accomplished. I'm obviously not as familiar with your gem though.

David Campbell dacamp deleted the branch
Dominik Honnef
Owner

If LoggerList becomes a hash, the behaviour of iteration will change, << wouldn't work anymore unless we make up a name, etc. So that seems rather backwards incompatible.

What would you need the name for?

David Campbell
dacamp commented

I suppose it's different schools of thought is all. I started looking into this because I thought "What would you need duplicated logging for?" among many other things.

Imagine yourself trying to dig through logs for nearly any piece of information... You would end up in the debug log every time, at which point any other logs are irrelevant. This isn't very DRY:

loggers.first.level = :debug # Now you're getting all levels (:debug) sent to STDERR, cause what would you ever need that changed for?
loggers[1].level    = :log   # Duplicates everything above except for debug
loggers[2].level    = :info  # Duplicates everything above except for debug and log
#  And so on

You're also limiting the total number of logs you'd ever need to open (which should still be considered, but only for heavy use cases will it matter). Since logs can only print out the information set in each level, you'd never create more than LevelOrder.size I/O streams, any further logging is just duplicated information (Albeit maybe formatted differently. Yay colors!). Before you reply with anymore snarky remarks, I realize formatting does change information.

There is so much more to logging than just 'mmm is my code broken?!' Being able to audit IRC logs in the real world is completely different. It forces accountability, so when ~dipshitdev@yolo says "hey I'm gunna push the big red button" you can easily track it down when, where, and hopefully why it was done. During a production outage, post-mordem logs are invaluable.

You wouldn't really lose any of the 'features' your current class has except for "<<" of course. If that is absolutely necessary, it could be written as its own method. As far as iterations go, adding "|k," isn't really so hard is it? Come on man, you gotta see the glass as half full sometimes!

I do think you would gain enough to make it worth it.

  • You'd no longer have to iterate over n Logger objects during each LoggerList#log call.
# (see Logger#log)                                                          
    def log(messages, event = :debug, level = event)
      self[event].log(messages,event,level)
    end

Auto assign your LoggerLevel to an I/O variable if that level isn't assigned it's own yet. The level of the individual logger would determine if it should actually get logged.

 1.9.3p392 :001 > f = Hash.new($stdout)
  => {} 
 1.9.3p392 :002 > f[:log].puts "I'm a little log bot"
 I'm a little log bot
* NOTE: This could create bloat, if someone decided to pass 'event' param symbols willy nilly... but then again it wouldn't really be cinch's fault since the first time the symbol was instantiated would have been by a user.
  • Plugins could have their own specific logs, which would be helpful for debugging.
    loggers[:myplugin].level = :debug # wtf is wrong with my shit!!!

  • Channels could have their own logs.
    Especially helpful when you can ask the bot to join your channel and start logging.

  • Log rotation could now happen more aggressively on debug logging, while still maintaining the integrity of any individual logs you'd need to keep around.

There's plenty more, but I'm sure I've worn out my welcome already.

So, rant aside, that's the beauty of open source I suppose. I'll just continue to develop on my own fork.

Dominik Honnef
Owner

The reason we can't lose << is because I do not want to release Cinch 3.0, and breaking backwards compatibility in a minor release is out of the question. So unless you make << work, it's not an option. Same thing goes for iteration, even though that part could actually be made backwards compatible by redefining #each. But then you also got to do that for all the methods of Array, including [], and all the methods of Enumerable that currently return arrays.

This really isn't about me thinking the current design is superior, it's about not breaking backwards compatibility.

There is one thing where I do think the current design is superior, however, and that is being able to the same level to multiple files. Logging isn't just about adding/not adding colours.

  • The FormattedLogger is mainly used to inspect your bot, and requires to include as many information as possible to debug errors in your plugins as well as Cinch itself.
  • Then there's also the ZcbotLogger, which generates logs in a format that is understood by pisg, a tool to generate statistics from IRC logs. It requires some of the levels to be set that the FormattedLogger requires as well.

I would never accept a mix of FormattedLogger and ZcbotLogger log files to debug a problem.

As for plugins having their own level: This could easily be made possible with the current design, we'd just need to make Logger.LevelOrder configurable. By using Logger#log you can then specify whatever level you wish.

I agree with the per-channel logging, that was an oversight in the design. It's still possible to implement this with the current API, it's just not neat because you would need to parse raw messages in your logger.

As a final, snarky response: The glass is neither half full nor half empty, it's twice as big as it needs to be.

I'll gladly consider this should it ever come to a Cinch 3.0, but until then you might be required to maintain your own fork indeed. Or implement it as a plugin that builds on top of the current infrastructure.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Showing 1 unique commit by 1 author.

May 23, 2013
David Campbell dacamp Adding new channel_logger plugin as a plugin example. b0cb925
This page is out of date. Refresh to see the latest.

Showing 1 changed file with 100 additions and 0 deletions. Show diff stats Hide diff stats

  1. +100 0 examples/plugins/channel_logger.rb
100 examples/plugins/channel_logger.rb
... ... @@ -0,0 +1,100 @@
  1 +# This plugin logs private messages and each channel seperately.
  2 +# Intended use would be to pipe Cinch::Loggers STDOUT/STDERR to a
  3 +# debug log and ChannelLogger mail to will handle the rest.
  4 +# I'm sure fixing up the Cinch::Logger(s)? class is the solution,
  5 +# but this is a good temporary fix.
  6 +
  7 +require 'cinch'
  8 +
  9 +# Message instance variable message needs to be writeable
  10 +module Cinch; class Message; attr_accessor :message ; end; end
  11 +
  12 +class ChannelLogger
  13 + include Cinch::Plugin
  14 +
  15 + set :required_options, [:logfile]
  16 +
  17 + listen_to :connect, :method => :setup
  18 + listen_to :disconnect, :method => :cleanup
  19 +
  20 + listen_to :channel, :method => :log_public_message
  21 + listen_to :private, :method => :log_private_message
  22 + listen_to :error, :action,
  23 + :join, :leaving, :method => :handle_notice
  24 +
  25 + def setup(*args)
  26 + @logs = { :privmsg =>File.open(config[:logfile], "a") }
  27 + @logs[:privmsg].sync = true
  28 + @logdir = File.dirname(config[:logfile])
  29 + @timeformat = config[:timeformat] || "%Y/%m/%d %H:%M:%S"
  30 + @logformat = config[:format] || "[%{time}] %{channel} %{nick}: %{msg}"
  31 + @last_time_check = Time.now
  32 +
  33 + bot.debug("Opened message logfile at #{config[:logfile]}")
  34 + end
  35 +
  36 + def handle_notice(msg, *args)
  37 + h = msg.raw.strip.split(/\s+/)[1]
  38 + if h =~ /JOIN/ and msg.channel
  39 + open_log(msg.channel.name)
  40 + end
  41 + msg.message = "-!- [#{h.to_s}] #{msg.user.nick}"
  42 + log_public_message(msg)
  43 + end
  44 +
  45 + def log_public_message(msg)
  46 + return unless @logs # Connection messages will still go to logger
  47 + log_msg(msg)
  48 + end
  49 +
  50 + def log_private_message(msg)
  51 + return unless msg.respond_to?(:user)
  52 + log_public_message(msg)
  53 + end
  54 +
  55 + def cleanup(*)
  56 + @logs.each_pair{ |key, fh|
  57 + fh.puts(sprintf(@logformat,
  58 + :time => Time.now.strftime(@timeformat),
  59 + :channel => key.to_s,
  60 + :nick => bot.nick,
  61 + :msg => "-!- [QUIT] #{bot.nick}"))
  62 +
  63 + fh.close
  64 + }
  65 + bot.debug("Closed message logfiles.")
  66 + end
  67 +
  68 + private
  69 + def log_msg(msg)
  70 + chl = msg.channel || :privmsg
  71 + @logs[chl].puts(sprintf(@logformat,
  72 + :time => Time.now.strftime(@timeformat),
  73 + :channel => chl.to_s,
  74 + :nick => msg.user.nick,
  75 + :msg => msg.message ))
  76 + end
  77 +
  78 + def open_log(chl)
  79 + @logs[chl] ||=
  80 + File.open("#{@logdir}/#{chl}.log", "a")
  81 + @logs[chl].sync ||= true
  82 + end
  83 +
  84 +end
  85 +
  86 +bot = Cinch::Bot.new do
  87 + configure do |c|
  88 + c.nick = "channel_logger"
  89 + c.server = "irc.freenode.org"
  90 + c.channels = ["#cinch-bots"]
  91 +
  92 + c.plugins.options[ChannelLogger] = {
  93 + :logfile => '/tmp/private_msgs.log'
  94 + }
  95 +
  96 + c.plugins.plugins = [ChannelLogger]
  97 + end
  98 +end
  99 +
  100 +bot.start

Tip: You can add notes to lines in a file. Hover to the left of a line to make a note

Something went wrong with that request. Please try again.