search

pact-foundation / pact-reference

Reference implementations for the pact specifications
https://pact.io
MIT License
93 stars 46 forks source link

fix: reduce log level for matching to debug #454

Closed mefellows closed 4 months ago

mefellows commented 4 months ago
Screenshot 2024-07-05 at 12 03 58 AM

The additional noise in the logs at info level is not needed and interferes with standard out (e.g. test output), and is more appropriate for debug level.

YOU54F commented 4 months ago

some other places noted from https://github.com/pact-foundation/pact-js/issues/1114

it’s here

https://github.com/pact-foundation/pact-core-mock-server/blob/febf7974b648aeeefb6af3d69dd988051886243e/pact_mock_server/src/hyper_server.rs#L275

here

https://github.com/pact-foundation/pact-core-mock-server/blob/febf7974b648aeeefb6af3d69dd988051886243e/pact_mock_server/src/hyper_server.rs#L185

The debug values captured for the incoming and outgoing requests are useful and could be captured for sending in case of failure.

and here

https://github.com/pact-foundation/pact-reference/blob/efc54d263e7ea53c9511cf389870e790c0158bc3/rust/pact_matching/src/lib.rs#L1665

There are other instances of info! statements

I think lib authors can suppress log output by piping to a buffer (or file) and reading from there if necessary (fetch_log_buffer_as_json)

mefellows commented 4 months ago

Could capture additional info statements found here, in this PR if you wanted to, although they cross crates

https://github.com/search?q=repo%3Apact-foundation%2Fpact-reference+info%21&type=code

Good shout, they look like decent targets to switch to debug! as well.

Pact JS was one of the earlier adopters and we were toying with various output options. Resolving https://github.com/pact-foundation/pact-js-core/pull/428 will go some way to making this less relevant in the JS use case.

rholshausen commented 4 months ago

The other option is if you don't want to see INFO level logs, then don't set the logging to INFO. Set it to WARN.

YOU54F commented 4 months ago

this is pretty much ready to go https://github.com/pact-foundation/pact-js-core/pull/428

however it is a pain to test as when the ffi lib is started and you set a log sink, you can't change it, so it logs to stdout/buffer or file. it's the cause of noise in the pact-go and js logs about the ffi already being initialised and a log level is already set, so cannot set a new level

Not sure if we can setup multiple?

Also it would actually be nice if we could control the log level per test, but that currently isn't possibly. I would find maybe being able to log per test more useful that a massive log dump in a file.

makes the haystack smaller if something does go wrong, as no-one wants to go sifting through logs unless there is a really good key that you can reliably search on.