Improved logging class

[iloveitaly]

• switched to extending a gem
• translation expansion is related to Persistence #19

[iloveitaly]

@AlessioRocco Any thoughts on this PR? I have some other changes I'd like to submit a PR for that are dependent on this change

[mtylty]

@iloveitaly I think we might need some time to analyze the requirements here... One thing we don't want is to depend on another external gem which doesn't seem widely supported.

I think we should look for other "more robust" gems o just go with a logging class inside Cangaroo.

[iloveitaly]

@mtylty Agreed! I don't like relying on external gems either. I created simple_structured_logger because there wasn't a great existing solution to this problem.

I think Cangaroo:

• Shouldn't contain a custom logging class
• Shouldn't rely on a custom logger which is complex and unmaintained. This will create an additional failure point when upgrading ruby/rails.
• Should use structured logging. Being able to run key=value queries against logs provides a great audit trail

What do you think?

[iloveitaly]

@mtylty Friendly ping on this; any thoughts on my above comments?

[houndci-bot]

Inconsistent indentation detected.

[AlessioRocco]

@iloveitaly Hi, I like the idea of structured logging but I don't want force the end user to use the simple_structured_logger gem, we can let it by default but the user can also choose another library for logging.

The logger class https://github.com/iloveitaly/cangaroo/blob/b56d6d07b03fec830d9a1ad8dd6f76589d07241c/lib/cangaroo/logger.rb seems to much coupled with simple_structured_logger and I don't see an easy way to replace it, to do so we should create an extendable Log class in some way, do you have any idea on how we can do it?

[AlessioRocco]

@iloveitaly I did this PR #44, it's is currently in WIP but it's a starting point to talk about my previous comment but with with some code, I've added a configuration option https://github.com/nebulab/cangaroo/pull/44/files#diff-8de3b2a85ababa7651c32ebc03dee14fR17 to change the logger to use. I'm currently thinking a way to normalize how the logs works, for now I did this

cangaroo/lib/cangaroo/logger.rb

Lines 9 to 16 in 807a54a

	%i(debug info warn error).each do |log_method|
	define_method log_method do |msg, opts = {}|
	begin
	@logger.send(log_method, msg, opts)
	rescue
	@logger.send(:error, "#{msg}: #{opts}")
	end
	end

, I don't like it very much but is the first idea that I had, unfortunately Rails.logger doesn't accept more than 1 param, instead gems like yours simple_structured_logger and semantic_logger accepts two params... let me know what are your thoughts.

[iloveitaly]

@AlessioRocco I like this approach! Allows users to use their favorite structured logging class with a sane + simple fallback.

Catching ArgumentError seems like a sane approach to me although checking the arity of a method could be a better approach.

[AlessioRocco]

@iloveitaly I use ArgumentError because I want that the end user can use the Cangaroo logger with the same arguments of the choose log gem. Checking the arity is more complex and have the same result, instead I just proxy the call to the end log gem, if that call fail I try with the fallback.

I close this PR in favor of #44

[iloveitaly]

@AlessioRocco Makes sense, that works for me!