Skip to content

Log missing ACME challenges at DEBUG#8

Merged
danillouz merged 1 commit into
masterfrom
fix/acme-challenge-log-noise
Jul 13, 2026
Merged

Log missing ACME challenges at DEBUG#8
danillouz merged 1 commit into
masterfrom
fix/acme-challenge-log-noise

Conversation

@danillouz

Copy link
Copy Markdown
Member

The "looking up info for HTTP challenge" logs are incredibly noisy: https://onenr.io/0Zw0aEpp3Rv

They produced ~2M logs in the last 24 hours!

image

Example: https://onenr.io/0BR6EMlkqwO

As discussed here:
https://framer-team.slack.com/archives/C06505JMQK0/p1741360219851809?thread_ts=1741345255.812599&cid=C06505JMQK0

Most requests come from external ACME clients and pre-checks for challenges FDS did not initiate. So treat the "expected" case, where no issuer has stored a matching token, as DEBUG.

But note that actionable failures will remain visible:

  • Storage lookup failures: WARN
  • Empty or corrupt challenge data: WARN
  • Certificate issuance/renewal failures: visible via existing error logs

So if FDS initiated a challenge whose token is unexpectedly gone, this request log becomes DEBUG. If that causes validation to fail, the issuance or renewal failure remains visible separately.

Fixes https://github.com/framer/company/issues/32009

The "looking up info for HTTP challenge" logs are incredibly noisy:
https://onenr.io/0Zw0aEpp3Rv

They produced ~2M logs in the last 24 hours!

As discussed here:
https://framer-team.slack.com/archives/C06505JMQK0/p1741360219851809?thread_ts=1741345255.812599&cid=C06505JMQK0

Most requests come from external ACME clients and pre-checks for
challenges FDS did not initiate. So treat the "expected" case,
where no issuer has stored a matching token, as DEBUG.

But note that actionable failures will remain visible:

- Storage lookup failures: WARN
- Empty or corrupt challenge data: WARN
- Certificate issuance/renewal failures: visible via existing error logs

So if FDS initiated a challenge whose token is unexpectedly gone,
this request log becomes DEBUG. If that causes validation to fail,
the issuance or renewal failure remains visible separately.

Fixes framer/company#32009
Copilot AI review requested due to automatic review settings July 12, 2026 17:38

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Reduces production log noise from ACME HTTP-01 challenge lookups by downgrading the expected “no active challenge” case from WARN to DEBUG, while keeping actionable failure cases at WARN.

Changes:

  • Introduce a sentinel error (errNoACMEChallengeInfo) to distinguish “no active challenge” from other lookup failures.
  • Log “looking up info for HTTP challenge” at DEBUG when the sentinel error is returned; otherwise keep WARN.
  • Add tests to assert the log level for: no active challenge (DEBUG), empty/corrupt data (WARN), and storage failure (WARN).

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
httphandlers.go Switches the challenge-lookup log level to DEBUG only for the expected “no active challenge” error.
config.go Adds a sentinel error and refines distributed challenge lookup errors to enable log-level selection.
httphandlers_test.go Adds log-observer tests to validate the new log-level behavior across key scenarios.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@danillouz

Copy link
Copy Markdown
Member Author

As per: https://github.com/framer/FramerDomainsServer/#upgrading-certmagic

This needs to become a "fork revision" after merging: v0.25.3-framer.1

And a corresponding FDS PR to use that

@ankon ankon left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM. One request though: Please upstream this, so that we can keep certmagic as a relatively light fork.

@danillouz

Copy link
Copy Markdown
Member Author

LGTM. One request though: Please upstream this, so that we can keep certmagic as a relatively light fork.

Good one, I also flagged it here: https://framer-team.slack.com/archives/C06505JMQK0/p1783883343394879?thread_ts=1741345255.812599&cid=C06505JMQK0

Will deploy this first, then if Matt is okay with it, will upstream it

@danillouz danillouz merged commit ab7c252 into master Jul 13, 2026
7 checks passed
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.

3 participants