editoast: reduce logging level to trace for some verbose events

[leovalais]

Proposition PR.

Are demoted to trace level:

• Query payloads sent to core
• PG connection instrumentation

[codecov-commenter]

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

Attention: Patch coverage is 66.66667% with 3 lines in your changes missing coverage. Please review.

Project coverage is 79.79%. Comparing base (70e728e) to head (8cee0fe).
Report is 8 commits behind head on dev.

Files with missing lines	Patch %	Lines
.../src/db_connection_pool/tracing_instrumentation.rs	66.66%	3 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@            Coverage Diff             @@
##              dev    #9950      +/-   ##
==========================================
- Coverage   79.81%   79.79%   -0.02%     
==========================================
  Files        1052     1052              
  Lines      105439   105439              
  Branches      727      727              
==========================================
- Hits        84152    84139      -13     
- Misses      21245    21258      +13     
  Partials       42       42              

Flag	Coverage Δ
editoast	73.70% <66.66%> (-0.05%)	⬇️
front	89.12% <ø> (ø)
gateway	2.18% <ø> (ø)
osrdyne	3.28% <ø> (ø)
railjson_generator	87.50% <ø> (ø)
tests	87.00% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[woshilapin]

Before reviewing or anything, have you consider using RUST_LOG=editoast_models=none before changing the log level? I often realize that these kinds of PR comes from a problematic situation when debugging something... but for a specific case, which might not apply in all situations. I consider that these logs are about editoast_models... and therefore, everything related to DB interactions have some interest, at least more than trace in my opinion.

And probably the same applies for the core client, if it ends up being moved inside its own crate (but it's already its own module so actually, the argument might still already be valid).

🤓 I'm not saying no to this PR, I'm just asking a bit more to be convinced it's the right move.

[leovalais]

I'm not 100% sure either, but the current situation still feels quite wrong to me. (Hence the "proposition PR", I don't mind closing it.)

Actually, I cracked when I was trying to observe the DB interactions while testing the STDCM request logging feature. I couldn't spot the event of StdcmLog::create because it was drowned under the extensive logging of the DB connection and the core client. Having both Model functions and DB queries at the same log level feels sketchy to me.

Besides:

at least more than trace in my opinion

Then I wonder what goes into the trace level?

My take is that we must not have too much trouble finding debug logs. Otherwise it's not a convenient "debug"ging tool. In other words, if debug logs are too verbose, or too numerous, it hinders the debug ability and thus defeats the purpose of the log. So I'd rather have anything that falls into these categories at the trace level, where we expect to be overflowed by logs.

I'm not saying I'm right or that we should do things this way, but I guess that's where I draw the line between debug and trace.

Wdyt?

[flomonster]

LGTM. I don't have a preference for the default log level.

[woshilapin]

I've been incapable to find a reasonable definition to separate DEBUG and TRACE. In order to move on, and without a good definition, I propose to take the next best choice (or I think it is): let's take the best default (the one that will, in general, be the most reasonable). And even if I had to debug once and those logs were useful, I'd say that in most cases, they're not needed. So let's keep them as TRACE, maybe the future will teach us more anyway.