fix(doctor): remove config check, report every telemetry-suppressing gate by simonkrol · Pull Request #37 · aws-solutions/konductor

simonkrol · 2026-10-05T20:30:28Z

Description

Removes doctor's config check. It validated the full .konductor/config.yml schema, which belongs to the hidden config command. The starter config now holds only version and telemetry.enabled, so the check had nothing left to catch.

Extends telemetry_state to report every gate that can suppress telemetry. It used to check only the per-target install record and machine consent, so it could say reporting was on while KONDUCTOR_TELEMETRY=off or telemetry.enabled: false turned it off. Doctor and the send path now share one predicate in report.rs, so their answers can't diverge. These opt-outs are reported at Info severity. doctor --all evaluates config.yml per target. Telemetry sending behaviour is unchanged.

Type of change

Bug fix (non-breaking change which fixes an issue)
New feature
Breaking change
Documentation update

Testing

cargo build --release, and cargo test --release with 1723 unit and 44 integration tests passing.
cargo clippy --release --all-targets: no new warnings.
check-guide-facts.py: one failure, also present on main (WITHDRAWN_COMMANDS).
Ran the binary with each gate off in turn and with all gates on, and ran --all across two targets.
New tests cover each gate and the all-gates-on case, using temp HOME and target dirs and a shared env lock.

Checklist

My code follows the style guidelines of this project
I have performed a self-review of my own code
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation
My changes generate no new warnings
I have added tests that prove my fix is effective
New and existing unit tests pass locally with my changes

Notes

Destination is a v1.1.0 release branch, another PR will address regenerating the HTML user-guide rather than each PR managing that themselves.

…gate doctor's config check validated the full config.yml schema, which belongs to the hidden config command. The starter config now holds only version and telemetry.enabled, so the check caught nothing real. Removed it. telemetry_state checked only the per-target install-info opt-out and machine consent, so it could say reporting was on while KONDUCTOR_TELEMETRY=off or a target's own telemetry.enabled: false actually suppressed it. telemetry_state now reports all four gates, sharing one predicate with report.rs's own send-time gate so the two can never drift. doctor --all evaluates the config.yml gate per target, since it reads each target's own config.yml. Updated every doc listing or counting doctor's checks, and the Data Collection section describing what telemetry_state reports.

knihit · 2026-10-13T16:20:55Z

Adversarial review notes (security, data integrity, schema/contract). No security or data-integrity issues found. Four schema/contract items:

Important

doctor --json drops the config check with no deprecation path (cli/konductor-rs/src/cli/doctor.rs, run_checks). Scripts that run jq '.checks[] | select(.name=="config")' or count checks will silently change behavior. Suggest keeping an info stub for one release, or listing the removal as a breaking change in CHANGELOG.
A malformed .konductor/config.yml fails open on gate 4 (cli/konductor-rs/src/cli/telemetry/report.rs, read_telemetry_config / gate4_suppression_from). Read and parse errors both collapse to None, which is treated the same as "no telemetry section." A typo such as enabled: flase silently drops the opt-out, and doctor reports telemetry as on. Gates 2 and 3 already split not-found from broken; gate 4 could do the same and report a malformed section as Warn with the config path.

Suggestions

telemetry_state can now be info instead of ok on an otherwise healthy install. That's the intended fix, but a one-line note in the doctor section of cli/README.md would help --json consumers that check for ok.
CHANGELOG says the starter config now holds only version and telemetry.enabled. cli/gate-config/config.yml still has six fields and no telemetry section, and telemetry.enabled is read separately in report.rs. Worth rewording so it doesn't imply the preset shrank.

knihit · 2026-10-13T15:09:57Z


 - `konductor metrics`. It was a stub that printed "not yet implemented"; the CLI now exits `64`
  for it like any other unrecognized subcommand.
+- `doctor`'s `config` check. It validated the full config schema, which belongs to the hidden


to confirm, we are not versioning this change. I also dont see a VERSION file.

knihit · 2026-10-13T15:11:07Z

+    /// Gate 4, env var: `KONDUCTOR_TELEMETRY=off` suppresses reporting
+    /// even though gates 2 and 3 both allow, and the check names the
+    /// specific reason at `Info` severity.
+    #[test]


not in scope of this CR, but we should consider refactoring this file. It has grown over a period of time.

knihit · 2026-10-13T16:27:50Z

        check_source(&resolved_source),
        check_runtime(destination),
        check_manifest(destination),
-        check_config_with_home(&resolved_source, home_dir.as_deref()),


would this lead to any backward compatibility issues on installs where config exists?

knihit · 2026-10-13T16:29:34Z

+/// `gate4_suppression` so a caller that has already read
+/// `.konductor/config.yml` for another reason doesn't have to read it
+/// again just for this check.
+fn gate4_suppression_from(raw: &Option<RawTelemetrySection>) -> Option<Gate4Reason> {


during this check in general, have we validated config.yml? what if it is malformed.

simonkrol changed the base branch from main to release/v1.1.0 October 5, 2026 20:39

simonkrol force-pushed the dev/simonkrol/doctor_telemetry_check branch from 8fd40d7 to 4008a48 Compare October 5, 2026 22:03

simonkrol force-pushed the dev/simonkrol/doctor_telemetry_check branch from 4008a48 to e5c47a2 Compare October 9, 2026 15:34

knihit reviewed Oct 13, 2026

View reviewed changes

Navigation Menu

Repository navigation

fix(doctor): remove config check, report every telemetry-suppressing gate - #37

fix(doctor): remove config check, report every telemetry-suppressing gate#37
simonkrol wants to merge 1 commit into
release/v1.1.0from
dev/simonkrol/doctor_telemetry_check

simonkrol commented Oct 5, 2026 •

edited

Loading

Uh oh!

knihit commented Oct 13, 2026

Uh oh!

knihit Oct 13, 2026

Uh oh!

knihit Oct 13, 2026

Uh oh!

knihit Oct 13, 2026

Uh oh!

knihit Oct 13, 2026

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

simonkrol commented Oct 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

knihit commented Oct 13, 2026

Uh oh!

knihit Oct 13, 2026

Choose a reason for hiding this comment

Uh oh!

knihit Oct 13, 2026

Choose a reason for hiding this comment

Uh oh!

knihit Oct 13, 2026

Choose a reason for hiding this comment

Uh oh!

knihit Oct 13, 2026

Choose a reason for hiding this comment

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

simonkrol commented Oct 5, 2026 •

edited

Loading