caas-team / sparrow

A monitoring tool to gather infrastructure network information
Apache License 2.0
6 stars 4 forks source link

feat: add optional text handler for the logger #118

Closed lvlcn-t closed 3 months ago

lvlcn-t commented 4 months ago

Motivation

We currently only offer an ugly json handler for the logger.

Since this is really annoying when debugging, I've took some time and improved our logger so we can opt into a pretty text logger.

Changes

I've introduced a prettier text handler for the logger. This enables us to choose the text logging format over the default JSON format by setting the environment variable LOG_FORMAT=text.

This enhances readability and usability for log analysis when we're debugging.

For additional information look at the commits.

Tests done

I've provided some unit tests for the handler creation.

The pretty logs look like this (colors aren't displayed here, for more info on that please refer to charmbracelet/log):

$ go run main.go run --config .tmp/sparrow.yaml 
Using config file: .tmp/sparrow.yaml
01:44:01 INFO <cmd/run.go:81> Running sparrow
01:44:01 INFO <targets/gitlab.go:77> Starting global gitlabTargetManager reconciler
01:44:01 INFO <api/api.go:76> Serving Api addr=:8080
01:44:11 ERRO <gitlab/gitlab.go:344> Failed to post file status="400 Bad Request"
01:44:11 ERRO <targets/gitlab.go:180> Failed to register global gitlabTargetManager error="request failed, status is 400 Bad Request"
01:44:11 WARN <targets/gitlab.go:104> Failed to register self as global target error="request failed, status is 400 Bad Request"

TODO

puffitos commented 4 months ago

Slog already has a text logger, doesn't it? I don't see the necessity to import a dependency we do not actually need.

lvlcn-t commented 4 months ago

@puffitos But it's prettier 🥺 Nevertheless I can change it to the slog.TextHandler if you think we shouldn't introduce another dependency.

puffitos commented 4 months ago

I'll let the others decide as well @y-eight @niklastreml, this should be a team decision 😊

niklastreml commented 3 months ago

I like the charmbracelet logger, but since this is mostly a debugging thing, I don't think we need to introduce a dependency for this. I'm fine as long as there is some alternative to the JSON logger, as I find myself building one every time I work on a feature anyways, only to throw it away before posting the pr

lvlcn-t commented 3 months ago

The default slog text handlers output is now like this:

❯ go run main.go run --config .tmp/sparrow.yaml
Using config file: .tmp/sparrow.yaml
time=01:17:02 level=INFO source=/home/tom/dev/sparrow/cmd/run.go:81 msg="Running sparrow"
time=01:17:02 level=INFO source=/home/tom/dev/sparrow/pkg/sparrow/targets/gitlab.go:77 msg="Starting global gitlabTargetManager reconciler"
time=01:17:02 level=INFO source=/home/tom/dev/sparrow/pkg/api/api.go:76 msg="Serving Api" addr=:8080
time=01:17:12 level=ERROR source=/home/tom/dev/sparrow/pkg/sparrow/gitlab/gitlab.go:344 msg="Failed to post file" status="400 Bad Request"
time=01:17:12 level=ERROR source=/home/tom/dev/sparrow/pkg/sparrow/targets/gitlab.go:180 msg="Failed to register global gitlabTargetManager" error="request failed, status is 400 Bad Request"
time=01:17:12 level=WARN source=/home/tom/dev/sparrow/pkg/sparrow/targets/gitlab.go:104 msg="Failed to register self as global target" error="request failed, status is 400 Bad Request"
time=01:17:23 level=ERROR source=/home/tom/dev/sparrow/pkg/sparrow/gitlab/gitlab.go:344 msg="Failed to post file" status="400 Bad Request"
time=01:17:23 level=ERROR source=/home/tom/dev/sparrow/pkg/sparrow/targets/gitlab.go:180 msg="Failed to register global gitlabTargetManager" error="request failed, status is 400 Bad Request"
time=01:17:23 level=WARN source=/home/tom/dev/sparrow/pkg/sparrow/targets/gitlab.go:104 msg="Failed to register self as global target" error="request failed, status is 400 Bad Request"
puffitos commented 3 months ago

Please add a few lines in the Readme as well, to document this new feature. Eventually, this should also be part of the chart values.