General file

[richfitz]

With apologies, there is quite a bit in here, and more to come if you would like (see issues)

• Added very basic unit tests so I could tell if I broke anything (f0effe1)
• Support for removing logging (required for testing creading many different logging targets during testing) (e1675b4)
• Generalise the log_file function to support connections and 3rd party interfaces (77e347f)
• Minor tweak to log_functions.R to allow arbitrary additional parameters to be passed in (3e4b3d3)

I've broken those across a series of commits that should make it easier to decide what, if anything, you want to keep. I've lumped it all together as one PR so you can see the end result though.

The third commit is the biggest: it

• generalises the approach by providing log_file and log_connection as two alternative ways to start logging (note that logging to a connection will likely be significantly faster than logging by appending to a file, even with flushing after each write).
• generalises the writing approach to allow different "types" of destination to have different writing functions.
• fixes some extra whitespace that was being added to log messages
• adds a subscriptions argument to log_file to help with programmatically setting up logging (see issue)

This also required renaming the option loggr_files to loggr_objects (as they may not all be files).

There's a subtle change of behaviour; I kept the behaviour of "console" the same (it runs through cat(..., file="") but "stdout" now runs through cat(..., file=stdout()). That has some weirdness with capture.output not working when I really feel that it should be - it's possible that something to do with the signal handling is doing something with the sinks.

Aim:

I want to be able to generate machine readable (e.g., JSON) logs directly into a database (in my case Redis) so that I can log on AWS where I don't expect to have access to the "machines" but do have access to the Redis instance. I'm going to use Redis as a broker for logstash I think.

I've tried to retain complete backward compatability with your interface, but have some questions about the design there. I have changed the customised initialisation string though; it is now the same across all connection types and does not indicate if the file is being started or appended to (though that is fairly obvious in the file itself).

Use with Redis

With the interface changes, I can now write this amount of support code (which I'd be happy to contribute) to set up the Redis inteface (designed to work with my RedisAPI package, but could be tweaked to work with the naked RcppRedis package)

log_redis <- function(con, key, ...,
                      .warning   = TRUE, .error = TRUE, .message = TRUE,
                      .formatter = format_log_entry, subscriptions = NULL) {
  name <- sprintf("%s:%d:%s", con$host, con$port, key)
  subscriptions <- loggr_subscriptions(.warning, .error, .message, ...,
                                       subscriptions=subscriptions)
  obj <- list(name=name, con=con, key=key, write=write_redis)
  loggr_start(obj, subscriptions, .formatter)
}
write_redis <- function(obj, str) {
  obj$con$RPUSH(obj$key, str)
}

Then, to support json writing (this required no tweaks to your formatter interface which was really nice)

format_log_entry_json <- function(event) {
  jsonlite::toJSON(unclass(event))
}

Then in use:

con <- RedisAPI::hiredis()
log_redis(con, "mylog", .formatter=format_log_entry_json)
log_info("hello")
log_info("hello", a=1, b=2)
con$LRANGE("mylog", 0, -1) # fetch all the logs

Ordinarily we'd do a LPOP or BLPOP to pop things off the list.

I've tried to match your style as much as possible; apologies for differences that remain.

[smbache]

Looking forward to looking at this! Thanks so far :)

[smbache]

I looked through your changes and I think it looks promising. I added you as collaborator, so feel free to work on it (even pull this yourself). I won't have much time the next some days to work on it, but excited about getting this more mature.

[richfitz]

OK, thanks! I'm excited about using this for a bunch of long-running simulations, though that won't be for a month or so.

I'll incorporate the changes from the other issues to this PR and give you a bit longer to look over this. Perhaps if you've not had time by the end of the week I can just merge then.

[smbache]

I like it. I have merged with small modifications here and there. But this is much better.

I still wonder:

• could/should suppressed messages/warnings be disabled somehow?
• should we let ... be only regular strings or vectors thereof, and omit the subscription arguments?

PS feel free to add yourself as aut to the DESCRIPTION, as you did a lot of this work!
Thanks again.

[richfitz]

Great! I'm going to be using this a bunch next week so will get more feedback.

I agree with both suggestions (avoiding suppressed messages/warnings and dropping ... for simplicity). The latter is easy the former could be fun.

I'll add myself to the DESCRIPTION soon. Thanks for the merge!

[smbache]

I made an attempt at muffling muffled messages and warnings! It doesn't catch package startup messages though... regular messages and warnings seem to work fine.