fix: config that check-config accepts but serve rejects, plus two stuck states - #121
Merged
Merged
Conversation
…ck states tracing_exporter advertised five values in the field's own comment — otlp-grpc, otlp-http, stdout, jaeger, zipkin — and SetDefaults handed four of them a plausible default endpoint. The provider implements two. So check-config returned 0 on a config that made serve exit 1 at startup with "unsupported trace exporter". Validate now rejects the exporter by name, and only exporters that exist get a default endpoint. A config the validator calls valid has to boot. tracing_sample_rate: 0.0 — the documented way to sample nothing, and what docs/observability/tracing.md's "ensure not 0.0" advice presupposes — produced 100% sampling, because SetDefaults treated 0 as "unset" and replaced it with 1.0. It is now a pointer, so an explicit zero is distinguishable from an absent key, read through TracingSampleRateValue(). A panicking job executor left the scheduled job in JobStateExecuting for good: the state was reset only on the normal return path, and the overlap check then refused every subsequent run with "already executing". The cron chain recovers the panic, so the process survived and the job was silently retired with nothing in the logs to explain it. The state is now restored from a defer, the failed attempt is recorded in the history, and the panic is re-raised for the cron chain's recoverer to log. The readiness manager's IsReady and Snapshot already refuse to report ready once stopped, but setReady still wrote the FILE — so an evaluation racing shutdown recreated the readiness file after Stop had removed it, and a file-based probe went on reporting the container ready while it tore itself down. Traffic kept arriving at a container that was leaving. Also: docs/configuration/global-settings.md said api_host defaults to "all interfaces". It defaults to 127.0.0.1, and has since the exposure guard landed — the doc was advertising the insecure default the code deliberately abandoned.
sylvesterdamgaard
force-pushed
the
fix/tracing-readiness-scheduler-p2
branch
from
August 27, 2026 12:18
e049f1b to
ae9ce39
Compare
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.
tracing_exporter advertised five values in the field's own comment — otlp-grpc,
otlp-http, stdout, jaeger, zipkin — and SetDefaults handed four of them a
plausible default endpoint. The provider implements two. So check-config
returned 0 on a config that made serve exit 1 at startup with "unsupported trace
exporter". Validate now rejects the exporter by name, and only exporters that
exist get a default endpoint. A config the validator calls valid has to boot.
tracing_sample_rate: 0.0 — the documented way to sample nothing, and what
docs/observability/tracing.md's "ensure not 0.0" advice presupposes — produced
100% sampling, because SetDefaults treated 0 as "unset" and replaced it with
1.0. It is now a pointer, so an explicit zero is distinguishable from an absent
key, read through TracingSampleRateValue().
A panicking job executor left the scheduled job in JobStateExecuting for good:
the state was reset only on the normal return path, and the overlap check then
refused every subsequent run with "already executing". The cron chain recovers
the panic, so the process survived and the job was silently retired with nothing
in the logs to explain it. The state is now restored from a defer, the failed
attempt is recorded in the history, and the panic is re-raised for the cron
chain's recoverer to log.
The readiness manager's IsReady and Snapshot already refuse to report ready once
stopped, but setReady still wrote the FILE — so an evaluation racing shutdown
recreated the readiness file after Stop had removed it, and a file-based probe
went on reporting the container ready while it tore itself down. Traffic kept
arriving at a container that was leaving.
Also: docs/configuration/global-settings.md said api_host defaults to "all
interfaces". It defaults to 127.0.0.1, and has since the exposure guard landed —
the doc was advertising the insecure default the code deliberately abandoned.