Skip to content
This repository has been archived by the owner on Mar 29, 2020. It is now read-only.

Implement proper logging #64

Open
ClaudiuCeia opened this issue Feb 4, 2018 · 6 comments
Open

Implement proper logging #64

ClaudiuCeia opened this issue Feb 4, 2018 · 6 comments
Labels
enhancement Self explanatory help wanted Liber la Pull Requests nice to have Optional, de livrat dupa urmatorul deploy

Comments

@ClaudiuCeia
Copy link
Member

https://github.com/Seldaek/monolog or something

@ClaudiuCeia ClaudiuCeia added enhancement Self explanatory help wanted Liber la Pull Requests labels Feb 4, 2018
@ClaudiuCeia ClaudiuCeia added this to the v 1.0 backend milestone Feb 4, 2018
@ClaudiuCeia ClaudiuCeia added this to To Do in fiipregatit.ro via automation Feb 4, 2018
@raduvoinea
Copy link

What problem are you trying to solve? :)

@ClaudiuCeia
Copy link
Member Author

The problem of shit hitting the fan and not knowing what went down. Never sub-estimate proper logging. We can have a chat on this subject any time you want.

@ClaudiuCeia
Copy link
Member Author

Ok, catching a break so I'll elaborate:

The default logs we have (error.log and access.log) are not very insightful for the most common case, troubleshooting.

For the error log, while you do get a stacktrace, it's not always helpful by itself for two main reasons:

  • Tendency of suppressing exceptions at every level, both by the language itself (PHP) and by programmer choice. That means that a lot of what you see is just noise.
  • Even when you do get an accurate stacktrace, you don't get context. It's true we don't have complex state to be worried about since every visitor hits the same code path, but we may deal with issues coming from WordPress core state (the global $post variable for example, it actually happened during dev. on this project). So having context being logged along with the stacktrace is more than beneficial.

Additionally, the content managers can have issues too. If there are problems in any of the dependencies we have (WordPress, plugins) I want to be able to see them as soon as possible, maybe even before the beneficiary realizes something went wrong. Having stacktraces with context can mean avoiding a 2 hour remote debugging session where you're trying to replicate the state the user had while encountering the issue they're reporting. This is even more important for regular visitor use cases.

There could be a broken link on the site leading folks to a broken error page without us even knowing, and with no channels for them to easily communicate this it can take ages until we fix it. The use cases are countless. Having a structured log is a good start. Having a visualization tool on top for monitoring is even better, but that can wait for now. Ideally we'd have something like Grafana to centralize logs across all CivicTech projects, but not just yet.

Logging isn't only useful for debugging purposes, but having a better structured error log is a great start, the rest can come after. I hope this clears things up a bit.

@ClaudiuCeia ClaudiuCeia added the nice to have Optional, de livrat dupa urmatorul deploy label Feb 10, 2018
@raduvoinea
Copy link

I know how logging works and understand the benefits of proper logging. I was asking why talk about Monolog (or similar) because:

  • it will not solve your context problem automatically (right?), might be as useless as error log
  • since we don't have a Dependency Injection Container, using that logger might prove to be a burden (config, instantiations, etc)
  • logging (and monitoring) is usually used in business critical areas; what would those critical areas be in our case? How do you know something goes wrong? You need something like if (value is wrong) or if (result seems incorrect) then Log this error with this context. We don't really have this cases.

What we could do is create a static Logger class (more static, yay, \o/) that will log in error_log (or whatever).
Later if this proves to be a bottle-neck, said Logger class can remain a wrapper around a more powerful logging mechanism, without modifying code everywhere it is used.

Anyway, I see where you want to go with this and it is a really good idea. With Grafana on top even cooler.

@ClaudiuCeia
Copy link
Member Author

Yeah, we don't have DI, but we can use PHP's erorr handler for this case: http://php.net/manual/en/function.set-error-handler.php. It would probably be a singleton that handles it all, but whatever, such is life.

So it would address logging automatically with the condition that we don't shy away from throwing exceptions, which I would argue is the way to go anyway. They provide a stacktrace by default and yell at you when things are broken, unlike silencing exceptions and counting on the programmer to handle every error state that could possible occur.

Regarding business critical areas - I think there are actually several types of logging / or several use cases. I think we're still at the very first level since all we have with regards to logging is the default error_log. At least we have a rotation strategy in place that's not too bad.

I don't think we need to address a certain a certain area in this first instance as much as follow some guidelines in our custom code, for example:

  • To stop null propagation where an exception would be the case (ie: I expect a value from the database but even if I don't get it I'll just propagate null and program the application layer to handle that case as well - bad). Those exception don't show up in production but they do get handled by the error handler which allows us to take action sooner.

We don't really have that much custom stuff for this project, but what we do here we'll probably use (at least as a principle) in other projects, so I want us to do good.

I only suggested Monolog after a brief research based on wide support from the community and good 3rd party integrations: https://seldaek.github.io/monolog/doc/02-handlers-formatters-processors.html

Feel free to suggest something else, I haven't really done anything vanilla PHP in a while and I'm not sure what/if there are better options out there.

@ClaudiuCeia ClaudiuCeia removed this from To Do in fiipregatit.ro Feb 22, 2018
@ClaudiuCeia ClaudiuCeia removed this from the v 1.0 backend milestone Feb 26, 2018
@dsecareanu
Copy link
Collaborator

dsecareanu commented May 2, 2018

De fapt nu e despre monitorizare ci despre logging...
https://help.ubuntu.com/community/LinuxLogFiles
Si astea pot fi automatizate cu un script de bash care sa le salveze pe un backup server si sa le stearga periodic.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement Self explanatory help wanted Liber la Pull Requests nice to have Optional, de livrat dupa urmatorul deploy
Projects
None yet
Development

No branches or pull requests

3 participants