+1 to using Markers, and to Josh's & Benedict's points. Logs have defined semantics, redefining them is confusing. To be completely honest, I wasn't even consciously aware of the convention that DEBUG would be used for background logs. I don't expect most users would be either.
On 2024/07/31 17:10:06 Aleksey Yeshchenko wrote: > I reckon we could start using slf4j/logback Markers to tag individual logging > statements as foreground/background (or, ideally, with more granularity than > that - separate out repair, compaction, schema, reads/wrties, etc.). > > > On 24 Jul 2024, at 16:02, Josh McKenzie <jmcken...@apache.org> wrote: > > > > I argued this point 6 years ago and I'll continue to argue it today. > > Relying on assertions in production code by default, having debug live in > > production code by default, these are code and culture smells to me. Yes we > > work on complex distributed systems. No we should not be relying on > > crutches like this in production environments to make up for the fact that > > our property and fuzz-based testing has historically been inadequate. And > > don't get me started on it being 2024 and us not having nightly perf > > regression testing. > > > > Let me quote Benedict and strongly +1 his words here: > > > >> Debug logging can be enabled for production clusters, but it should not be > >> on by default. > > > >> DEBUG should not be taken to mean “background activities” and we should > >> stamp this out wherever this has been occurring > > > >> DEBUG means verbose logging for use debugging system behaviour. It should > >> be safe to enable in production, but it should not be on by default. > > > >> We should anyway not be codifying something just because a handful of > >> people have been doing it. > > > > Doing our own thing w/regards to logging as a project is a pretty big > > unforced error IMO. > > > > A brief glance at cockroach's logging channels > > <https://www.cockroachlabs.com/docs/stable/logging-overview#logging-channels> > > and sinks or postgres' various severity levels > > <https://www.postgresql.org/docs/current/runtime-config-logging.html#RUNTIME-CONFIG-SEVERITY-LEVELS> > > etc. just reinforces for me how our approach to logging and debug feels > > ad-hoc, reactive, and organic to me. There's a lot of prior art we could > > learn from here. > > > > So I'm in my glass house here chucking stones, because of course I don't > > have the bandwidth to help address any of this. I'm with you on this Mick: > >> What's your proposal to change it to how it should be? And who will do > >> that work ? Along with better naming of log files, I wholeheartedly > >> support a more intuitive approach to using separate loggers in the code, > >> e.g. with markers. > >> > >> My proposal is only in lieu of such an agreement and change. > > > > On Wed, Jul 24, 2024, at 4:59 AM, Berenguer Blasi wrote: > >> I wouldn't forbid isDebugEnabled. > >> > >> Guarding heavy debug params with it makes perfect sense per se, you avoid > >> a perf penalty with 1 loc. If in C* production clusters are > >> expected/preferred to be run with debug logging on so be it but being an > >> OSS project we're unaware of all tools, side-projects and any other > >> consumers of our code. They could be benefiting from not running in debug. > >> > >> Parsing the use of isDebugEnabled as making debug not meant for production > >> is sthg I am not totally following. Debug is, imo, is perfectly fine in > >> production for it's intended uses and with it's know impact. If we want to > >> signal we want C* to be ran in debug in production then let's add a big > >> warning in the logback xml files on even log that as a WARN or ERROR. But > >> I wouldn't push down that concern into the code style. > >> > >> Besides, imo, if we remove/forbid it getting it back if we changed our > >> minds wouldn't be an easy task. Whereas if it's there it doesn't hurt. > >> > >> My 2cts > >> > >> On 23/7/24 16:12, Mick Semb Wever wrote: > >>> > >>> > >>> On Tue, 23 Jul 2024 at 15:27, J. D. Jordan <jeremiah.jor...@gmail.com > >>> <mailto:jeremiah.jor...@gmail.com>> wrote: > >>> I don’t know that I agree we should remove use if isDebugEnabled, but we > >>> should be assuming debug is enabled and not doing anything too crazy > >>> there. > >>> > >>> > >>> The problem is that the use of isDebugEnabled, as trivial as it is, leads > >>> to the typical assumption that debug is not meant for production. This > >>> has tripped us up in the past. It's important that debug logging does > >>> not impact production performance. Any use of isDebugEnabled is > >>> counter-intuitive to this. Maybe restoring the logging guidelines is > >>> enough, but I'd argue that we can further simplify things (at least > >>> against our current state of affairs) by just disallowing the use of > >>> isDebugEnabled altogether. > >>> > >>> And yes, debug equalling background tasks is not entirely accurate, my > >>> bad (the doc provides a far more accurate description to what might be > >>> found in system vs debug log files). And there's for example read/write > >>> tracing, that if enabled, might only be found in the debug.log . This > >>> has been brought up previously, and in the ticket and old ml thread > >>> there's mention in keeping system.log clean to read/writes/hotpaths. The > >>> logging guideline was written (by Paulo I believe) as a result of 10241 > >>> and that old ml thread. > >>> We have an existing logging guideline doc, we need debug to be performant > >>> in production – it is enabled by default, and is designed and > >>> recommended to be used in production. > >
