Set TLS Handshake logs to debug#50
Merged
mcpherrinm merged 3 commits intomainfrom Mar 24, 2026
Merged
Conversation
This adds a logDebug, which if set true, prints debug-level logs. The logs from the http.Server are quite noisy from random internet scans, so set them to debug. Since we still want to see errors from our own getCertificate, add a logging wrapper for it at the warn level.
Drop it from the error message, and instead include it in the log line This theoretically makes the error worse if you were trying to use the get() function somewhere else, but we want this error for logging, and we add the SNI, so we've got what we need.
There was a problem hiding this comment.
Pull request overview
This PR introduces a configurable debug log level and reclassifies noisy TLS handshake-related server logs to debug, while adding structured warning logs around certificate lookup failures.
Changes:
- Add
LogDebugto configuration and wire it to a runtime-adjustablesloglevel inmain.go. - Route
http.Servererror logging throughslogat debug level to reduce noise at default info level. - Add a
GetCertificatewrapper that logs certificate lookup failures at warn level with structured SNI context.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
server/server.go |
Routes http.Server.ErrorLog through slog at debug level. |
main.go |
Adds a slog.LevelVar and sets debug level based on cfg.LogDebug. |
integration/test-certs-site-config.json |
Enables logDebug for integration testing. |
config/config.go |
Adds LogDebug to the Config struct. |
certs/certs.go |
Wraps GetCertificate to emit warn logs with sni and simplifies returned error strings. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
pgporada
reviewed
Mar 23, 2026
Bradatletsencrypt
approved these changes
Mar 24, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This adds a logDebug, which if set true, prints debug-level logs. Set it true in integration test config for testing, but I expect any internet-facing instance would set this false (the default).
The logs from the http.Server are quite noisy from random internet scans, so set them to debug.
Since we still want to see errors from our own getCertificate, add a logging wrapper for it at the warn level. There still might be a bit of noise for SNI with unknown hostnames, but empirically those are much less frequent, and are mostly empty if they happen. Include the SNI in the slog line, rather than the error, for more structured output.
The funcorder linter wants me to put get() after the exported functions, but I don't want to move it as it follows directly from GetCertificate. So throw an ignore in.
fixes #37