input-output-hk / jormungandr

privacy voting blockchain node
https://input-output-hk.github.io/jormungandr/
Apache License 2.0
364 stars 132 forks source link

Logging structures refactoring #589

Open CodeSandwich opened 5 years ago

CodeSandwich commented 5 years ago

Context

Currently the logging infrastructure is based on passing around Logger objects. Some of them contain some context information, others not.

Problem

It's extremely hard to track, what context information is outputted when using particular Logger.

Also Logger does not have any means of changing existing information. For example when Logger with task: network is derived and somebody tries to overwrite it with task: server, the child logger ends up with both of them causing mayhem in output.

Solution

Create a group of structures of which every could be used instead of Logger.

Logging is performed using macros. Macros are duck-typed, so as long as logging structures have the same API as Logger (log method), they can be used as replacement.

The logging structures will have methods for explicit conversions between some of them. For example NetworkLogger -> ServerLogger. This will make altering their behavior in different contexts possible and easy to track.

mzabaluev commented 5 years ago

I'm not sure the added strictness is worth the effort, we could just stick to some structure and keywords (e.g. using a hierarchical microformat in task).

mzabaluev commented 5 years ago

when Logger with task: network is derived and somebody tries to overwrite it with task: server, the child logger ends up with both of them

I thought this is not how it works, the child values override the parent's if using the same key?

CodeSandwich commented 5 years ago

@mzabaluev It doesn't and it's a mess: https://github.com/slog-rs/slog/issues/211

mzabaluev commented 5 years ago

Oh. Maybe then we should treat our keys more hierarchically, for example:

It's unfortunate that the drains cannot do a good job on this, but maybe we shouldn't fight the system, too.

NicolasDP commented 5 years ago

I don't see how this is a problem. The context here says that this is:

task: network, task server. I get this is a task regarding server/network. This is not too bad. The problem is more that we have been utilising key without really understanding the meaning of them.

The problem is more that we don't have any good specification of what we need to provide in term of contextual information and what they should mean.

For example task here could be renamed service because that is what it is in the code, the network service has spawn a task server. This will need to be addressed by providing a list of meaningful keys, what they mean and how to use them.