ppy
Unify logging to use ASP.NET default console logger
- [x] Depends on https://github.com/ppy/osu-server-spectator/pull/214 for mergeability
- [x] Ask @ThePooN if he's okay with deploying this
This has been bothering me for a little while. Spectator server logging is a mix of built-in ASP.NET logging to console, direct Console.WriteLine() usage, and (over)use of framework logger. The last of these is especially problematic when deploying to k8s as file-based logs are not how you're supposed to be doing logging on k8s.
So here's a diff that moves everything to the ASP.NET logger and just straight up bans the other methods to keep things in shape.
Code-wise this looks alright. Haven't tested yet.
One question I have is, it seemed that @ThePooN implied we would have a hard time retrieving these logs when they were on the K8s cluster. Now that this is no longer running on K8s, I gather it's not going to be such an issue. But if we do move it back to K8s, are we going to have issues pulling these logs from e.g. a server crash?
As far as I gather (and @ThePooN plz correct me here if wrong) the lowdown is as such:
- When we were on k8s, the file-based logs would be lost when an instance was killed, but stdout from a previous run was being preserved by k8s internals
- Now we're not on k8s anymore, and deploying this would require some extra infra so that the logs could go somewhere persistent
- We may still return to k8s, but that depends on whether the issue causing large overheads in packet handling that were observed on k8s can be fixed (there's a prospective setting that can be tuned but it has not been confirmed yet whether it fixes)
For now we're not on k8s. Log files are not lost on redeploy, and stdout is persisted by systemd with no concern in longevity.
If we were to come back to k8s, log files would be lost on every redeploy and crashes. stdout is the way to go for logging on containerized environments however we have no logging infrastructure yet, and outofthebox kubernetes persists very little logs. with osu-server-spectator throughput, only the last 5mn are persisted. So we'd want to re-enable datadog logging or a self-hosted solution like loki.
tl;dr we should most likely merge this if we come back to k8s but will also require some infra work to accommodate it
with osu-server-spectator throughput, only the last 5mn are persisted
This would also probably get worse after this change btw, what with the number of file-based log calls I changed to print to stdout instead.
Blocked while we figure out a server-side log ingest solution, since it looks like we're moving back to k8s.
@ThePooN Has grafana been deployed now / is this PR now ready to be merged? Looking to reduce my inbox.
it is not deployed yet, i've been catching up since i came back from holidays a week ago and have been focused on higher priority stuff
@ThePooN Has grafana been deployed now / is this PR now ready to be merged? Looking to reduce my inbox.
Has been deployed now and in a state I'm pretty happy with.
We also have the ability to apply regular expressions to infer additional labels. That is most commonly used to add prefixes such as [<component name>], and make independent logging streams for every component.