Skip to content

chore!: adopt log/slog, drop go-kit/log #1311

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Oct 26, 2024

Conversation

tjhop
Copy link
Contributor

@tjhop tjhop commented Oct 25, 2024

The bulk of this change set was automated by the following script which is being used to aid in converting the various exporters/projects to use slog:

https://gist.github.com/tjhop/49f96fb7ebbe55b12deee0b0312d8434

Other changes included:

  • upgrade go in ci to 1.23
  • update client_golang, common, exporter-toolkit

The bulk of this change set was automated by the following script which
is being used to aid in converting the various exporters/projects to use
slog:

https://gist.github.com/tjhop/49f96fb7ebbe55b12deee0b0312d8434

Other changes included:
- upgrade go in ci to 1.23
- update client_golang, common, exporter-toolkit

Signed-off-by: TJ Hoplock <[email protected]>
@tjhop
Copy link
Contributor Author

tjhop commented Oct 25, 2024

cc: @SuperQ

@tjhop
Copy link
Contributor Author

tjhop commented Oct 25, 2024

This one needed a bit of extra love to convert because of the scrapeLogger config. It's been ported to an slog handler that tees logs to both the underlying logger and a logger that writes to a buffer for use with displaying scrape logs to users.

Ex of it working with the new implementation:

~ -> curl -sL "http://localhost:9115/probe?target=prometheus.io&module=http_2xx&debug=true" | head -25
Logs for the probe:
time=2024-10-25T00:10:17.560-04:00 level=INFO source=handler.go:122 msg="Beginning probe" module=http_2xx target=prometheus.io probe=http timeout_seconds=119.5
time=2024-10-25T00:10:17.560-04:00 level=INFO source=utils.go:61 msg="Resolving target address" module=http_2xx target=prometheus.io target=prometheus.io ip_protocol=ip4
time=2024-10-25T00:10:17.561-04:00 level=INFO source=utils.go:96 msg="Resolved target address" module=http_2xx target=prometheus.io target=prometheus.io ip=172.67.201.240
time=2024-10-25T00:10:17.561-04:00 level=INFO source=http.go:153 msg="Making HTTP request" module=http_2xx target=prometheus.io url=http://172.67.201.240 host=prometheus.io
time=2024-10-25T00:10:17.711-04:00 level=INFO source=http.go:381 msg="Received redirect" module=http_2xx target=prometheus.io location=https://prometheus.io/
time=2024-10-25T00:10:17.711-04:00 level=INFO source=http.go:153 msg="Making HTTP request" module=http_2xx target=prometheus.io url=https://prometheus.io/ host=""
time=2024-10-25T00:10:17.711-04:00 level=INFO source=http.go:169 msg="Address does not match first address, not sending TLS ServerName" module=http_2xx target=prometheus.io first=172.67.201.240 address=prometheus.io
time=2024-10-25T00:10:17.823-04:00 level=INFO source=http.go:479 msg="Received HTTP response" module=http_2xx target=prometheus.io status_code=200
time=2024-10-25T00:10:17.826-04:00 level=INFO source=http.go:601 msg="Response timings for roundtrip" module=http_2xx target=prometheus.io roundtrip=0 start=2024-10-25T00:10:17.562-04:00 dnsDone=2024-10-25T00:10:17.562-04:00 connectDone=2024-10-25T00:10:17.680-04:00 gotConn=2024-10-25T00:10:17.680-04:00 responseStart=2024-10-25T00:10:17.711-04:00 tlsStart=0001-01-01T00:00:00.000Z tlsDone=0001-01-01T00:00:00.000Z end=0001-01-01T00:00:00.000Z
time=2024-10-25T00:10:17.826-04:00 level=INFO source=http.go:601 msg="Response timings for roundtrip" module=http_2xx target=prometheus.io roundtrip=1 start=2024-10-25T00:10:17.712-04:00 dnsDone=2024-10-25T00:10:17.713-04:00 connectDone=2024-10-25T00:10:17.722-04:00 gotConn=2024-10-25T00:10:17.760-04:00 responseStart=2024-10-25T00:10:17.823-04:00 tlsStart=2024-10-25T00:10:17.722-04:00 tlsDone=2024-10-25T00:10:17.760-04:00 end=2024-10-25T00:10:17.825-04:00
time=2024-10-25T00:10:17.826-04:00 level=INFO source=handler.go:133 msg="Probe succeeded" module=http_2xx target=prometheus.io duration_seconds=0.265705545



Metrics that would have been returned:
# HELP probe_dns_lookup_time_seconds Returns the time taken for probe dns lookup in seconds
# TYPE probe_dns_lookup_time_seconds gauge
probe_dns_lookup_time_seconds 0.001011623
# HELP probe_duration_seconds Returns how long the probe took to complete in seconds
# TYPE probe_duration_seconds gauge
probe_duration_seconds 0.265705545
# HELP probe_failed_due_to_regex Indicates if probe failed due to regex
# TYPE probe_failed_due_to_regex gauge
probe_failed_due_to_regex 0

Copy link
Member

@SuperQ SuperQ left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice. I'll lave this up to the other maintainers to give a second review.

Address PR feedback, not necessary. Can always be re-added.

Signed-off-by: TJ Hoplock <[email protected]>
@SuperQ SuperQ merged commit 73cc86f into prometheus:master Oct 26, 2024
5 checks passed
@SuperQ SuperQ mentioned this pull request Feb 26, 2025
@Jakob3xD Jakob3xD mentioned this pull request Mar 4, 2025
tjhop added a commit to tjhop/blackbox_exporter that referenced this pull request Mar 18, 2025
Immediate fix for prometheus#1377. This fix will need to be refactored a bit when
we upgrade prometheus/common to v0.63.0, since prometheus/common#754
contains a breaking change to promslog's level code.

Explanation:
When I ported the blackbox exporter to use slog in prometheus#1311, I removed the
`none` option from the log prober flag, as it was non-standard to other
prometheus projects, not a valid value in the original promlog package,
and slog has no inherent corresponding concept. The log prober flag
value is used to create the scrape logger (which is specifically used
for scrape-related logging, not overall application logging). I also
included some code that ensured a level config would be created for use
with the scrape logger. In combination, this causes the blackbox
exporter to always output scrape logs, which can be noisy. If we instead
default the flag to an empty string and do some intelligent handling of
an nil log prober value within the scrapeLogger's implementation of the
slog.Handler interface, then we can restore the previous behavior.

TL;DR:
Leave `--log.prober` flag unset and it won't do scrape-level logging

Signed-off-by: TJ Hoplock <[email protected]>
tjhop added a commit to tjhop/blackbox_exporter that referenced this pull request Mar 18, 2025
Immediate fix for prometheus#1377. This fix will need to be refactored a bit when
we upgrade prometheus/common to v0.63.0, since prometheus/common#754
contains a breaking change to promslog's level code.

Explanation:
When I ported the blackbox exporter to use slog in prometheus#1311, I removed the
`none` option from the log prober flag, as it was non-standard to other
prometheus projects, not a valid value in the original promlog package,
and slog has no inherent corresponding concept. The log prober flag
value is used to create the scrape logger (which is specifically used
for scrape-related logging, not overall application logging). I also
included some code that ensured a level config would be created for use
with the scrape logger. In combination, this causes the blackbox
exporter to always output scrape logs, which can be noisy. If we instead
default the flag to an empty string and do some intelligent handling of
an nil log prober value within the scrapeLogger's implementation of the
slog.Handler interface, then we can restore the previous behavior.

TL;DR:
Leave `--log.prober` flag unset and it won't do scrape-level logging

Signed-off-by: TJ Hoplock <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants