13 ms·
Logging as a code smell
- rukuu001 5y agoThe real code smell here is importing a third-party library into all of your application classes. Wrap it in a class of your own so the logger (or whatever) can be easily swapped out if you want to use a different logging system, events, or whatever.
- ahelwer 5y agoOne way I've seen this going awry is when real functionality starts being built as event listeners. Data that would ideally be propagated back up the call stack instead just get consumed via published events because then the core logic doesn't have to be touched. It's like setting up an observatory with a parabolic antenna pointed at the mysterious legacy core, recording & processing its emissions to reconstruct the inner workings.
- notacoward 5y ago(1) This seems more than a bit Java-specific. (2) Not all kinds of software can afford to depend on an event-publishing infrastructure. One obvious example is the event-publishing infrastructure itself. Less obviously, all of its transitive dependencies down to storage, networking, and operating systems. (3) After dismissing logging and explaining a more complex event model, the author says in their own project he stripped away a lot of that infrastructure and ... reinvented logging. Similarly, they admit that AST-rewriting magic is a "footgun" and then recommends it. Why keep arguing for one thing and then doing the opposite? (4) Tagging everything you don't like with buzzwords like "code smell" is a bit of a writing smell. It's "considered harmful" for millennials. (5) The swipe at log4j seems to be just a misguided attempt to make the author's opinions seem more topical. The recent log4j vulnerability was not a problem with logging itself, but with a particular implementation that embedded some rather crazy functionality. That functionality could just as easily have been embedded in an event-oriented system, and I'll bet someone somewhere has already done exactly that.
- barbarbar 5y agoI wish I had read this comment before reading the post.
- dimgl 5y agoI'm not sure this article does a good enough job of explaining why it's a code smell. Aside from the event bus code looking slightly better, I don't see any big advantages. One disadvantage is that if you have a big class with lots of different events, you now have to import all of your events rather than just importing the singular logger. Typically I only resort to logging in situations where I don't want to propagate an error but I do want to log that an error happened. I also sometimes get lazy and debug using logs. I'm trying to avoid this as much as possible and instead debug by attaching to a process and using breakpoints. However this article doesn't touch on either of these scenarios. Also, log4j's vulnerability had nothing to do with the actual concept of logging. AFAIK the vulnerability came from log4j's JNDI lookup functionality, which was not doing any kind of input sanitization which allowed attackers to perform remote code execution. I was really looking forward to a post on logging as a code smell, mostly geared towards inexperienced developers. Pretty disappointed with the actual post.
- gregoryl 5y agoAwful hard to attach a debugger to a production system?
- gkhartman 5y agoAgreed. In a perfect world all production bugs can be reproduced on a test instance, but that's often not the case in the real world. Also, I've seen plenty of timing sensitive bugs that get covered up by the delay introduced by the attached debugger. The much feared "Heisenbug".
- rad_gruchalski 5y agoWhat if you don’t have physical access to a production system?
- 908B64B197 5y agoThat's an organizational problem, not a technical one.
- aranchelk 5y ago> Logging is an example of code duplication. Is it? What am I missing here?
- xboxnolifes 5y agoI think the idea is you have: (1) Some code that does a thing (2) Another line of code that logs that the thing happened And, like in-lined documentation, the code can diverge from the text that says what it does.
- aranchelk 5y agoPrescriptive example from the article: userRoles.addMember( p ); RoleAssignedEvent.publish( p, roleName ); Seems to have the exact same risk of divergence. Depending on the language you can (and I have) used decorators/higher order functions to log the invocation of a function with its name and arguments. Zero risk of divergence there, but it was still logging not some kind of event system. The problem of divergence seems to me to be orthogonal.
- seanhunter 5y agoSo the author proposes we replace it with: (1) Some code that does a thing (2) Another line of code that sends an event that the thing happened
- SV_BubbleTime 5y agoI love the idea of an event bus, but all I use is C and embedded. The last time I considered it I backed away from it. Wish there was effort to establish a pattern for this in C.
- gurjeet 5y agoI believe ZeroMQ has libraries/implementation suitable for embedded systems.
- SV_BubbleTime 5y agoChecked it out, no go. Replies extremely heavy on POSIX api and threads. All the cool functionality is with sockets. There is an inproc:// but it doesn’t do much. Lighter to just work it out using RTOS objects like queues and streams.
- kevin_thibedeau 5y agoYou can do this in C without all the enterprisey ceremony. All you need is a way to send messages to a queue and have them dispatched to registered listeners. The "message" can be nothing more than an encoded integer or something more complex. This lets you have a decoupled interface between interacting subsystems.
- johnwheeler 5y agoJust like a Java programmer to tell you to use an event bus for simple diagnostics. I’m a former one, I don’t mean no offense. I go even lower though. Just print(“XXXXXXXXXXX”) print(“XXXXXXXXXXX”) print(amount)
- Spivak 5y agoThat event bus exists whether you call it one or not — print is just a buffered serialized stream of events consumed by the kernel and rendered to strings.
- johnwheeler 5y ago> buffered serialized stream of events So is a sewer though, and I wouldn’t call that an event bus either.
- diogenesjunior 5y agowhen my code is not working how i expect it to, i print a ton of equal signs, then the value on a knew line, then a new row of equal signs. never used loggin before.
- jpgvm 5y agoYou my friend need to experience an interactive debugger.
- zepolen 5y agoYou my friend need to experience a production heisenbug.
- jpgvm 5y agoI think you are taking the comment rather out of context and just took the chance to be snarky. The gentleman I was replying to is clearly new to the gig and he should try out a debugger, it will change his workflow immensely and likely make him a much better developer. Yes, you still want logs (or events or traces, etc) in production to identify when edge cases or heisenbugs have occurred. A debugger doesn't replace those but it does replace 'println` style debugging when developing new code.
- lambdasquirrel 5y agoIn an ideal world, we wouldn't use logging to debug things after-the-fact. We would use traces, monitoring, and yes, events. But we don't always live in an ideal world, and logs are a simple and easy way to determine the sequence of events that happened after-the-fact, especially if there's something going on in a single application (which... happens to be a pretty big chunk of cases). And at the end of the day, few things are more versatile than a big wall of text with timestamps and request ids attached. Saying that logging is a code smell is a little like saying all tech debt is bad.
- Spivak 5y agoI think the author is on the cusp of the right idea but misses it with the new implementation — the only meaningful change is switching fixed strings interpolated with data to fixed classes parametrized with data. It’s not nothing, but it’s basically a funny way to do structured logging. Logging systems are already event streams with the ability for consumers modify, add supplementary data, and route output with strategies, formatters, etc etc. What I would rather see is a new keyword that generalizes the concept of logging in it’s entirety — “emit $object.” For discrete events it would be emit ThingHappened(), for logs it would be emit Log(), and for traces it would be emit OpenSpan() and emit CloseSpan(). You could collapse whole classes of libraries into a few emit calls and a single listener.
- S_A_P 5y agoLogging is one of many ways to debug and capture behavior in large systems. Logging is not inherently bad and isn’t an event bus just another layer or two of indirection of a logger? This post just demonstrated a logger abstraction that is still capturing and logging activities. Sort of anyway. Code smell is when your logs aren’t helpful
- andreskytt 5y agoDebugging complex systems in production is much easier, if you can rely on certain things. Thus, simple logging to files using known configuration eliminates one variable to debug. With an event bus, there’s always the question of “did the event not happen or did some upfckery prevent the message from getting where I can see it?”. While I do agree with the need to simplify code, moving to an event bus transfers the complexity to where we don’t necessarily want it - to system level. Now there’s one more moving thing to be deployed, monitored, scaled, networked. And, by the way, that moving thing is mission critical, as without it you are flying blind, including in terms of infosec. Is it worth it? Maybe in some scenarios. But definitely not to the level of a code smell, that should be something to be avoided universally.
- cryptonector 5y agoLogging is a code-smell. Some logging is needed, naturally. What you want is a good tracing system, like DTrace and the function boundary and user-space static probe providers. In Java that would be BTrace.
- gregors 5y agoI remember using the SciTE editor with AutoIT a long time ago. You could toggle a key and it would "add trace lines" between every single line of code. Yeah, I'd consider that a code smell for sure if you committed that.
- conradev 5y agoLogging is something you do for some future purpose. If I write a string in a log message, the future purpose is usually for me to read it later. The reasons in the post did not convince me on why my log messages should be structured. For example, why would I want to internationalize those messages?
- lawfulSteadfast 5y ago
- seanhunter 5y agoThe event bus in all these examples looks an awful lot like just a different name for a log, and adding an event bus has some minor downsides in that it requires more complex setup (with additional points of failure and points of difference between local and production deployment) and you would need to use special tooling to get the actual messages recorded somewhere and your hands on the contents of the messages when you need to debug. On the flipside the event bus messages may be useful for more than just human debugging. It's a shame the author didn't actually look at logging as a code smell. For example, is a high density of logging statements in code an indication of places which are brittle and hard to figure out and therefore candidates for reworking? That would be an interesting topic I think.
- zaptheimpaler 5y agoIt's a good point but I think the author sort of drove past the key insight. This is called structured logging. Having some kind of consistent schema of log messages is much better than plain strings for analysis later. It's just building a loose schema of events/log types, and picking a serialization format for said schema. JSON is popular because it remains human readable as well. I agree though, it really is very useful, not much harder than logging strings, and it's easy to evolve an existing system to gradually add structure to logs.
- taspeotis 5y agoSerilog (.NET, not Java) has been doing structured logging well for years.
- dboreham 5y agoSo many people don't know what they're talking about in this field, and then act as if they're superior to those who do. Frustrating.
- edsrzf 5y agoI agree logging is a bit of a code smell, but I prefer Mark Seemann's take: https://blog.ploeh.dk/2020/03/23/repeatable-execution/ https://blog.ploeh.dk/2020/03/23/repeatable-execution/ TL;DR: Log impure actions. If you minimize and group side effects, you don't need to log as much. Pure functions can be recomputed any time if you know their inputs. Event buses can be useful, but it's hard to look at an event being published and know what it does and whether it's important. Whereas the importance of a logger call is usually pretty clear. Logs are rarely load-bearing, but an event can mean anything.
- BerislavLopac 5y agoOne big argument in favour of that approach is the oft-forgotten (or at least ignored) fact that logging is, pretty much by definition, a side effect itself.
- kazinator 5y agoThe author is severely confused. What he's calling logs are actually debug traces. You don't want to use events for those. > eases the transition to internationalized log messages You don't internationalize debug traces any more than you internationalize your function names or programming language words. You barely even fix spelling mistakes in trace messages. Often, you build software without them at all: "compile them out". One excellent reason not to use events for trace messages is that you may sometimes want every last trace message leading up to a crash to be recorded, which won't happen if they go through an event system where they sit in a queue which is no longer being serviced since everything died. Oh, and your shiny event system could have issues, debugging which could benefit from trace messages.
- jb_s 5y ago> sLogger.debug( "Not marking nodes down due to local pause" ); vs > NodeMarkingInactiveEvent.publish( localPause ); The second statement is totally not logging because it doesn't have the word "logging" in it. ;-) That being said... conceptually, I do like the generality of publishing an event that describes internal system state, rather than logging a string on some level. OTOH the logging is supposed to be human, rather than machine-readable. But it's also gotta be a heap more work to do it this way and, maybe you'll never know if the error occurred cause the service lost network for some period of time (because it can't write to the event bus either!)