Conversation
… at startup
`lore investigate` tells the user which evidence backends are off — "note:
running without metrics (query_metrics), logs (query_logs), ..." — but `lore
serve` never did. serve announces at Info everything it turned ON (the ledger,
the rate limit, the coalescer, the debounce, the watcher, the poller), so an
in-cluster install missing its evidence backends looked identical in the logs
to one that has them all. The `minimal` Helm profile wires neither metrics.url
nor logs.url, and was indistinguishable from a misconfigured full one.
The notice reuses disabledTools, the CLI's pure function over config, so the
two commands cannot disagree about what counts as off. Structured rather than
the CLI's prose, to match the surrounding startup lines:
INFO running without tools=["metrics (query_metrics)","logs (query_logs)"]
The issue's open questions, decided here:
- Info, not Warn. It fires on every boot of a deliberately-minimal install,
and a warning that fires on purpose is tuned out within a day.
- It fires even when the under-configuration is intentional. The log is
where someone looks when investigations seem thin, and an Info line costs
nothing.
- Scope stays what disabledTools covers — metrics, logs, the catalog — so the
CLI note and this one name the same set. Widening it to flows, cloud and
the forge is a change to disabledTools, and then both commands get it.
Pinned the way the recall-decay warning is: RunServe must call disabledTools
directly in its own body (not in a closure the incident path runs), exactly
once, bind the result, and pass it to .Info in the same statement. The shared
assertion matched callees by their *Warning suffix, which this notice
deliberately lacks, so it now matches through a name-agnostic callName and
warningCallName is that plus the suffix filter. Existing pins are unchanged.
Fixes #467
This branch has not been deployed
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.
Fixes #467.
Why
lore investigatetells the user which evidence backends are off.lore servenever did: it announces at Info everything it turned on (the ledger, the rate limit, the coalescer, the debounce, the watcher, the poller), so an in-cluster install missing its evidence backends looked identical in the logs to one that has them all. TheminimalHelm profile wires neithermetrics.urlnorlogs.url, and was indistinguishable from a misconfigured full one.What
One line, next to the other pure-config startup notices:
It reuses
disabledTools, the CLI's pure function over config, so the two commands cannot disagree about what counts as off.The issue's open questions, decided
disabledToolscovers: metrics, logs, the catalog. Widening to flows, cloud and the forge is a change todisabledTools, and then both commands get it. Left as a follow-up rather than smuggled in.Pinned
The same way the recall-decay warning is:
RunServemust calldisabledToolsdirectly in its own body, not in a closure the incident path runs, exactly once, bind the result, and pass it to.Infoin the same statement. The shared assertion matched callees by their*Warningsuffix, which this notice deliberately lacks, so it now matches through a name-agnosticcallName, andwarningCallNameis that plus the suffix filter. The existing pins are unchanged.Verified
TDD: the pin failed first against the unmodified
serve.go(verified after fixing the suffix mismatch, with the edited file set aside), then passed. Full gate:go build,go vet,gofmt -lclean,go test ./...,hack/lint.shat 0 issues. One unrelated flake surfaced under load,TestRunRetriesReindexOnAPinnedRevisionininternal/catalog; it passes three times in a row alone.