Can we replace `github.com/prometheus/common/log` with `log/slog`?

peimanja / artifactory_exporter

JFrog Artifactory Prometheus Exporter written in Go

Apache License 2.0

140 stars 37 forks source link

Can we replace `github.com/prometheus/common/log` with `log/slog`? #121

Closed KacperPerschke closed 6 months ago

KacperPerschke commented 7 months ago

Logger github.com/prometheus/common/log is already marked as obsolete in version v0.26.0.

The log/slog component is new to the go standard library. It feels light and fast.

Hopefully we could then use it to choke out the github.com/go-kit/log library.

KacperPerschke commented 7 months ago

Static analysis^† of current master versions of imported libraries has not shown that any of these components use internally "github.com/prometheus/common/promlog".

96f1aec289bfec34242831d74d450c3d0089050f "github.com/prometheus/client_golang/"
9b7b6759313a7b41ffba1a65ea31cfbc9ed0b944 "github.com/prometheus/client_model/"
f09436866874ce72e1566783b8ee4a2c05f6bba5 "prometheus/common"

Can we continue the topic of replacing the logger with a lighter and simpler one?

^† I used vim and grep for analysis.

KacperPerschke commented 6 months ago

I have prepared a complete transition to log/slog. Please feel free to review and report any notices to me.

Unfortunately, I have no way to try it at home. I will try the “production” operation of my full proposal in the first week of January 2024.

If you would like the entire change to be moved to “#121”, please let me know.
If you prefer two steps, after possibly approving “#121”, I will open a new pull request to merge the second part of the change.

peimanja commented 6 months ago

Thanks for your contribution. Sounds good to me. I will give this a try as well and we will sort it out soon. We can just go with the complete transition to log/slog and we'll do our testing with that

KacperPerschke commented 6 months ago

[…] We can just go with the complete transition to log/slog […]

I moved the slog branch to this commit in my fork of your repository.

KacperPerschke commented 6 months ago

Test run results.

24 hours with logging level set to ‘debug’

We have set the ‘debug’ logging level and ‘json’ logging format.

The program behaved stably. Also when it comes to CPU and memory usage. In our environment it produced 300MB of logs per hour. At 3 a.m. service processes in the artifactory caused the program to produce over 7GB of logs and the orchestrator stopped the program.

24 hours with logging level set to ‘info’

We have set the ‘info’ logging level and ‘json’ logging format.

The program was as stable as before. In our environment, it produced less than eight log entries per day.

peimanja commented 6 months ago

Thanks for this change and all the testing. Looks good to me. You need to update golang version in here as well and we should be good to go.

sonarcloud[bot] commented 6 months ago

Quality Gate passed

Kudos, no new issues were introduced!

0 New issues
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

KacperPerschke commented 6 months ago

[…] You need to update golang version in here as well […].

Thank you for suggestion! I wasn't inquisitive enough to find it. I do hope that next commit meets the need.