Improve logging documentation of configs

[paweljakubas]

Issue Number

#961

Overview

• I have replicated (tried to do it) monitoring config needed to retrieve what happens when just defaultConfigStdout from iohk-monitoring-framework is called
• I have replicated (tried to do it) monitoring config needed to retrieve what happens when initTracer with Nothing from cli is called
• I have updated CLI Manual

Comments

In order to test do :

$ stack test --ta '-m "initTracer"' cardano-wallet-cli:unit

[paweljakubas]

this is something I cannot go through :
so when I put hasPrometheus: or when I omit it at all to specify this option I end up with:

cgBindAddrPrometheus = Just ("127.0.0.1",12799)

When I hasPrometheus: ["",0] I end up with

cgBindAddrPrometheus = Just ("",0)

which is wrong (it should be validated). And it is expected to have cgBindAddrPrometheus = Nothing
BTW. it is inconsistent with example configuration specified here : https://github.com/input-output-hk/iohk-monitoring-framework/blob/bea0e079fc32ed316ce352d17d14199a680e3f6c/iohk-monitoring/test/config.yaml
When we put hasPrometheus: 12799 we end up with parsing error

[KtorZ]

I'd suggest to open an issue on the iohk-monitoring-framework. Fixing this is not part of our scope, but reporting it is :)

[paweljakubas]

This is needed in order to have set :

cgMapSubtrace = fromList [("#messagecounters.monitoring",NoTrace),("#messagecounters.ekgview",NoTrace),("#messagecounters.aggregation",NoTrace),("#messagecounters.graylog",NoTrace),("#messagecounters.katip",NoTrace),("#messagecounters.switchboard",NoTrace)]

but it also sets :

cgOptions = fromList [("mapSubtrace",fromList [("#messagecounters.monitoring",Object (fromList [("subtrace",String "NoTrace")])),("#messagecounters.ekgview",Object (fromList [("subtrace",String "NoTrace")])),("#messagecounters.aggregation",Object (fromList [("subtrace",String "NoTrace")])),("#messagecounters.graylog",Object (fromList [("subtrace",String "NoTrace")])),("#messagecounters.katip",Object (fromList [("subtrace",String "NoTrace")])),("#messagecounters.switchboard",Object (fromList [("subtrace",String "NoTrace")]))])],

which is maybe as it should be. BUT, it is expected in empty configuration that cgOptions = fromList [] So it is inconsistent in such a sense, That empty is set like : https://github.com/input-output-hk/iohk-monitoring-framework/blob/master/iohk-monitoring/src/Cardano/BM/Configuration/Model.lhs#L543
and then on top of it is https://github.com/input-output-hk/iohk-monitoring-framework/blob/1ebe57df3a76a8ae4e3b5a31a9b85132f71534fc/iohk-monitoring/src/Cardano/BM/Configuration/Static.lhs#L26
So it behaves differently when yaml setting and in the code.

[rvl]

It seems like it will be difficult for us to test and document the entire log config format, and that is really a job for the iohk-monitoring team.

The main use for this --logging-config option would probably be as a way to enable logging to file with rotation by log file size.

[paweljakubas]

@rvl @KtorZ exactly, we probably should resign from giving user possibility of setting whole logging config, but expose some features like rotation, etc.

Nevertheless, the above findings may be of interest to iohk-monitoring-framework team

[KtorZ]

[change required]

Use (</>) from System.FilePath instead of hard-coded separators.

Why

Hard-coded path don't play well with cross-compiled code :s, so, always rely on platform-agnostic combinator like (</>) to combine paths.

[KtorZ]

I'd suggest to open an issue on the iohk-monitoring-framework. Fixing this is not part of our scope, but reporting it is :)

[KtorZ]

Hmmm. @paweljakubas , the goal for #961 I believe is simply to get some degree of documentation on our side. Pointing to some existing documentation / examples on the iohk-monitoring-framework would probably be sufficient.
For testing, I concur @rvl's remark, it's not our scope to check whether the framework works as intended. We already have a few test cases regarding logging and it's quite okay for the wallet backend already.

[paweljakubas]

closing in favor of #1253