refactor: replace go-kit logger with slog

[bernot-dev]

Since Go 1.22, there is a structure logger slog in the standard library. Compared to go-kit/log, the logger we are currently using, it offers several advantages:

1. No external dependency, uses standard library.
2. No error returned by logging calls - no need for linter ignore annotations or discarding returned error.
3. Cleaner interface at call site.

We have configured go-kit to output logs that look like this:

{"caller":"main.go:71","level":"error","msg":"--datasource-uid must be set","ts":"2025-03-06T15:13:11.665648804Z"}

slog outputs logs that look like this, with minimal configuration:

{"time":"2025-03-06T15:13:11.665763574Z","level":"ERROR","source":{"function":"main.main","file":"/app/cmd/datasource-syncer/main.go","line":72},"msg":"--datasource-uid must be set"}

It is possible to modify slog output to match previous logging. However, it seems likely that slog conventions will likely see increased adoption over time. In addition, slog took cues from OpenTelemetry, like the log level values.

When the standard library did not include structured logging, many different logging libraries proliferated, and it was difficult to choose a good solution. Over time, it has become clear that go-kit/log has seen limited adoption. For instance, go-kit has 187 stars on GitHub, compared to 22.7K for zap, 25.1K for logrus, and 11K for zerolog.

[bernot-dev]

There was also a significant effort to migrate Prometheus to slog: prometheus/prometheus#14355

[SuperQ]

I think this can fixed to eliminate the fmt.Sprintf().

Suggested change

	logger.Error(fmt.Sprintf("Failed to update Grafana data source uids: %s", dsErrors))
	logger.Error("Failed to update Grafana data source uids", "error", dsErrors)

[bwplotka]

This make sense code wise, but speaking to the team, we see a need to make sure the output is compatible with the old JSON. Is this feasible to ensure?

Looks like it's mostly time -> back to ts. The source vs caller - this might be lower risk although ppl conversion on logging might fail (e.g. conversion to BigQuery), simple to keep stable for now?

For level being upper case / lower case I think that's fine too.

[SuperQ]

The Prometheus common/promslog package provides a go-kit format slog wrapper.

[bwplotka]

Thanks! LGTM for slog on Go API side. For output, we need to maintain some compatibility though.

Now I recall Prometheus team wrote a compatibility layer utils for this. We can import and use it from here: https://github.com/prometheus/common/blob/main/promslog/slog.go#L242

Should we adopt this? Also I assume we do component by component?

[bwplotka]

The Prometheus common/promslog package provides a go-kit format slog wrapper.

Yea I just found too 🙈 , thanks

[bernot-dev]

We have a diamond dependency problem with the frontend and using common.

$ go mod tidy
go: finding module for package github.com/prometheus/common/promlog
go: github.com/GoogleCloudPlatform/prometheus-engine/cmd/frontend/internal/rule imports
    github.com/prometheus/prometheus/web/api/v1 tested by
    github.com/prometheus/prometheus/web/api/v1.test imports
    github.com/prometheus/common/promlog: module github.com/prometheus/common@latest found (v0.62.0), but does not contain package github.com/prometheus/common/promlog

There is only toil and sadness ahead trying to unwind these dependency issues.

[bwplotka]

Isn't this simple to unwind? Promlog is for go-kit so frontend will have to move to slog to and it's solved, no?

But I agree it's painful, like every change that touches literally every file (logging) 🙈

[SuperQ]

FYI, we wrote a bash script to help automate the conversion from go-kit to slog.

[bernot-dev]

Isn't this simple to unwind? Promlog is for go-kit so frontend will have to move to slog to and it's solved, no?

No. Everything is related.

Just as one example of a tricky situation for us, some places we pass a go-kit logger to prometheus/prometheus. So, we either have to have both loggers or upgrade to v3.

On a related note, this is officially an unforking task now, too.

[bwplotka]

FYI: This is still needed after 0.17 - especially when we upgrade our fork to 3.5