feat(build): add defaults-without-telemetry feature alias by russellb · Pull Request #2843 · NVIDIA/OpenShell

russellb · 2026-08-20T17:32:17Z

Summary

Adds a defaults-without-telemetry Cargo feature alias to the three binary crates that carry telemetry, so compiling telemetry out no longer means hand-maintaining a keep-list of each crate's other default features. Telemetry-free builds become --no-default-features --features defaults-without-telemetry.

Related Issue

No accepted issue yet — raising this for maintainer disposition. It came out of a contributor question about whether --no-default-features is a safe stand-in for "disable telemetry." It is not, and that is already a live correctness bug (see Changes), so this is filed as a PR rather than a spike. Happy to open a feature request issue first if the convention should apply here.

Changes

Cargo cannot subtract a single default feature, so the documented way to compile telemetry out is --no-default-features plus every other default re-added by hand. That keep-list is already wrong for operators:

Crate	`default`	Bare `--no-default-features` also drops
`openshell-server`	`["telemetry"]`	—
`openshell-driver-vm`	`["telemetry"]`	—
`openshell-sandbox`	`["telemetry", "bundled-ca-roots"]`	`bundled-ca-roots`

Following the README today therefore silently swaps the supervisor from bundled Mozilla roots onto the platform trust store. The alias fixes that and stays correct as the default set grows.

Add defaults-without-telemetry to openshell-server, openshell-sandbox, and openshell-driver-vm, each enumerating its defaults minus telemetry.
Add a compile_error! in each crate root for telemetry + defaults-without-telemetry. The alias is a keep-list, not a switch — without the guard, --features defaults-without-telemetry on its own (no --no-default-features) would compile a telemetry-on binary that reads as telemetry-free.
Add rust:verify:defaults-without-telemetry (tasks/scripts/verify-defaults-without-telemetry.sh), which asserts via cargo metadata that each alias still equals its crate's defaults minus telemetry, and that the mutual-exclusion error is wired up. The second check matches on the compile_error! text rather than a nonzero exit code, so it cannot pass vacuously on hosts where openshell-driver-vm fails to build for unrelated reasons.
Point rust:verify:telemetry-off at the alias, so CI builds the artifacts the same way the docs tell operators to.
Run the new task in branch-checks.yml.
Update the telemetry section of README.md and the Build Features section of architecture/build.md.

openshell-core also defaults to telemetry, but is left alone: every consumer already depends on it with default-features = false, so an alias there would be a no-op.

Testing

Command	Result
`mise run pre-commit`	pass
`mise run test`	pass, 0 failures
`mise run rust:verify:telemetry-off`	pass — positive control present in the default gateway, markers absent from both telemetry-free binaries
`mise run rust:verify:defaults-without-telemetry`	pass, 6/6 checks
`mise run rust:verify:system-ca-roots`	pass

Negative-controlled the drift check by temporarily setting the sandbox alias to []; it failed with the expected diff and exit 1, then passed again on restore.

Not run: mise run e2e. This changes build-time feature wiring, not sandbox runtime behavior, and the telemetry-off guard already inspects the resulting binaries.

mise run pre-commit passes
Unit tests added/updated — covered by the two rust:verify:* guards rather than #[test]s; feature-set drift and compile_error! wiring are not observable from inside a compiled test binary
E2E tests added/updated (if applicable) — not applicable

Checklist

Follows Conventional Commits
Commits are signed off (DCO)
Architecture docs updated (if applicable)

copy-pr-bot · 2026-08-20T17:32:21Z

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

mrunalp · 2026-08-24T02:19:28Z

The changes look fine. @drew @johntmyers wdyt?

github-actions · 2026-08-24T22:03:31Z

Label test:e2e applied, but pull-request/2843 does not exist yet. A maintainer needs to comment /ok to test 9d9688a15692ec2c33768a0ef17391379f83af83 to mirror this PR. Once the mirror exists, re-apply the label or re-run Branch E2E Checks from the Actions tab.

johntmyers · 2026-08-24T22:03:40Z

/ok to test 9d9688a

github-actions · 2026-08-24T22:04:03Z

Label test:e2e applied for 9d9688a. Open the existing run and click Re-run all jobs to execute with the label set. The run will execute the standard E2E suite after building the required gateway and supervisor images once. The matching required CI gate status on this PR will flip green automatically once the run finishes.

johntmyers · 2026-08-24T22:05:12Z

gator-agent

PR Review Status

Thanks @mrunalp. You said the changes look fine and asked for another look; I checked the complete current-head patch, including the Cargo feature aliases, mutual-exclusion guards, verification script, CI wiring, and documentation. The independent review found no blocking or non-blocking findings.

Blocking findings:

No blocking findings remain

Carried findings:

None

The test:e2e label is applied, the contributor mirror matches this head, and Branch/Helm/E2E workflows have started. The E2E Label Help bot requires current-head Branch E2E run 32782778291 to be rerun after its first attempt becomes rerunnable; Gator will perform that already-authorized action in a later watch cycle.

Gator metadata

Validation: Concentrated build correctness fix with a clear operator impact; no duplicate candidate found, and trusted maintainer feedback supports project fit.
Docs: Source-build behavior is documented in README.md and architecture/build.md; no separate Fern page is needed because the published docs do not currently expose a source-build Cargo-feature workflow.
Checks: DCO and vouch are green; current-head Branch Checks and Helm Lint are queued or running.
E2E: test:e2e applied; /ok to test 9d9688a15692ec2c33768a0ef17391379f83af83 created the exact-head mirror; bot-required rerun remains pending until run 32782778291 becomes rerunnable.
Head SHA: 9d9688a15692ec2c33768a0ef17391379f83af83
Base SHA: 4d7f402ce2ac88958c3fb3ae0018d7e1a17a9783
Merge base SHA: 7909fb5d0f54a06e26eb79e47885d7dd105aef24
Patch ID: 42411c46c88c79dfc1f47f79816e001a6d25ca96
Gator payload: 7
Review mode: initial
Previous reviewed SHA: none
Review budget exhausted: no
Maintainer decision required: no
Next state: gator:in-review

russellb · 2026-08-25T20:14:43Z

It looks like it failed because of a real error introduced in a rebase.

I added a check in CI to catch with these build feature sets got out of sync and it caught that defaults-without-telemetry is missing something in one binary.

johntmyers · 2026-08-25T20:31:12Z

Thanks @russellb - feel free to ping me when we need to try again. Slack works too.

johntmyers · 2026-08-25T21:02:04Z

/ok to test 95e1805

johntmyers · 2026-08-25T21:02:32Z

gator-agent

Re-check After Author Update

Thanks @russellb. You identified that the rebase added in-tree-compute-drivers to the server's default feature set and pushed the matching alias fix; I checked that author-only delta against the current manifest and the existing drift verifier. The independent follow-up review found no blocking findings.

Blocking findings:

No blocking findings remain

Carried findings:

None

The retained test:e2e label still applies. The contributor mirror was stale at the previous head, so Gator posted the authorized /ok to test 95e18053350b101bc3f298ca1cf70e9e60e7be41; exact-head Branch Checks, Helm Lint, and E2E dispatch are not confirmed yet, so pipeline monitoring has not started.

Gator metadata

Validation: Concentrated build-correctness fix with clear operator impact; trusted maintainer feedback supports project fit.
Docs: Existing README and architecture documentation cover this source-build behavior; no additional Fern navigation change is needed.
Checks: DCO is green; required Branch Checks, Helm Lint, and E2E gates are pending without confirmed exact-head workflow runs.
E2E: test:e2e retained; exact-head mirror refresh and workflow dispatch are pending after /ok to test.
Head SHA: 95e18053350b101bc3f298ca1cf70e9e60e7be41
Base SHA: 4d16a2a6f448bbe9639cae13f102115deaf78b0d
Merge base SHA: 4d16a2a6f448bbe9639cae13f102115deaf78b0d
Patch ID: a145f3a16a23c333f74b55456c0d0ef875f165f0
Gator payload: 7
Review mode: follow_up
Previous reviewed SHA: 9d9688a15692ec2c33768a0ef17391379f83af83
Review budget exhausted: no
Maintainer decision required: no
Next state: gator:in-review

johntmyers · 2026-08-28T18:19:55Z

gator-agent

Blocker Follow-Up Nudge

This PR is still blocked by merge conflicts with main after more than 48 business hours.

Next action: @russellb, rebase or merge the latest main into the PR branch, resolve the conflicts, and push the updated head.

Cargo cannot subtract a single default feature, so compiling telemetry out meant `--no-default-features` plus a hand-maintained keep-list of the crate's other defaults. That keep-list was already wrong for operators: telemetry is the only default on openshell-server and openshell-driver-vm, but openshell-sandbox also defaults to `bundled-ca-roots`, so a bare `--no-default-features` silently swapped the supervisor onto the platform trust store. Add a `defaults-without-telemetry` alias to each of the three telemetry- carrying binary crates, enumerating every default except `telemetry`. Telemetry-free builds become `--no-default-features --features defaults-without-telemetry` and stay correct as the default set grows. The alias is a keep-list, not a switch. Enabling it on top of the defaults would otherwise produce a telemetry-on binary that reads as telemetry-free, so each crate root carries a `compile_error!` for the `telemetry` + `defaults-without-telemetry` combination. Add `rust:verify:defaults-without-telemetry` to guard both properties: each alias still equals its crate's defaults minus `telemetry`, and the mutual-exclusion error is wired up. The additive-misuse check matches on the `compile_error!` text rather than a nonzero exit code so it cannot pass vacuously on hosts where openshell-driver-vm fails to build for unrelated reasons. `rust:verify:telemetry-off` now builds through the alias. Signed-off-by: Russell Bryant <rbryant@redhat.com>

Signed-off-by: Russell Bryant <rbryant@redhat.com>

russellb · 2026-08-28T18:45:37Z

gator-agent

Blocker Follow-Up Nudge

This PR is still blocked by merge conflicts with main after more than 48 business hours.

Next action: @russellb, rebase or merge the latest main into the PR branch, resolve the conflicts, and push the updated head.

done

johntmyers · 2026-08-28T18:56:01Z

/ok to test cbd4a71

johntmyers · 2026-08-28T18:56:44Z

gator-agent

Re-check After Author Update

Thanks @russellb. You confirmed the merge conflicts were resolved and pushed the rebased head cbd4a7108144476803ba642d73bf6caf9dc23047; I compared the author series with the previously reviewed head and checked the current feature-alias, verifier, CI wiring, and documentation changes. The independent follow-up review found no blocking findings, and there are no carried obligations.

The retained test:e2e label still applies. Gator refreshed the contributor mirror with the authorized exact-head /ok to test; current-head Branch Checks, Helm Lint, and Branch E2E workflows are now queued or running, so pipeline monitoring can begin.

Blocking findings:

No blocking findings remain

Carried findings:

None

Gator metadata

Validation: Concentrated build-correctness fix with clear operator impact; trusted maintainer feedback supports project fit.
Docs: Existing README and architecture documentation cover this source-build behavior; no additional Fern navigation change is needed.
Checks: DCO is green; current-head Branch Checks and Helm Lint are queued.
E2E: test:e2e retained; exact-head mirror refreshed and Branch E2E run 33201573985 is in progress.
Head SHA: cbd4a7108144476803ba642d73bf6caf9dc23047
Base SHA: 74654ac30dfe7115b6b9dfdf9480342b34290607
Merge base SHA: 74654ac30dfe7115b6b9dfdf9480342b34290607
Patch ID: 3e9946f1fc5d6b1205865c1652a240a082a39448
Gator payload: 7
Review mode: follow_up
Previous reviewed SHA: 95e18053350b101bc3f298ca1cf70e9e60e7be41
Review budget exhausted: no
Maintainer decision required: no
Next state: gator:watch-pipeline

russellb requested review from a team, derekwaynecarr, mrunalp and sjenning as code owners August 20, 2026 17:32

johntmyers added gator:in-review Gator is reviewing or awaiting PR review feedback test:e2e Requires end-to-end coverage labels Aug 24, 2026

johntmyers added test:e2e Requires end-to-end coverage and removed test:e2e Requires end-to-end coverage labels Aug 24, 2026

johntmyers previously approved these changes Aug 24, 2026

View reviewed changes

johntmyers enabled auto-merge August 24, 2026 22:07

johntmyers added this pull request to the merge queue Aug 24, 2026

johntmyers added gator:watch-pipeline Gator is monitoring PR CI/CD status and removed gator:in-review Gator is reviewing or awaiting PR review feedback labels Aug 24, 2026

github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Aug 24, 2026

johntmyers added gator:blocked Gator is blocked by process or repository gates gator:merge-ready and removed gator:watch-pipeline Gator is monitoring PR CI/CD status gator:blocked Gator is blocked by process or repository gates gator:merge-ready labels Aug 24, 2026

johntmyers added gator:blocked Gator is blocked by process or repository gates gator:merge-ready and removed gator:blocked Gator is blocked by process or repository gates gator:merge-ready labels Aug 25, 2026

mrunalp added this pull request to the merge queue Aug 25, 2026

johntmyers added gator:merge-ready and removed gator:blocked Gator is blocked by process or repository gates labels Aug 25, 2026

github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Aug 25, 2026

johntmyers added this pull request to the merge queue Aug 25, 2026

github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Aug 25, 2026

johntmyers added gator:in-review Gator is reviewing or awaiting PR review feedback and removed gator:merge-ready labels Aug 25, 2026

russellb dismissed johntmyers’s stale review via 95e1805 August 25, 2026 20:45

russellb added 2 commits August 28, 2026 14:44

fix feature alias for openshell-server

cbd4a71

Signed-off-by: Russell Bryant <rbryant@redhat.com>

Uh oh!

Conversation

russellb commented Aug 20, 2026

Summary

Related Issue

Changes

Testing

Checklist

Uh oh!

copy-pr-bot Bot commented Aug 20, 2026

Uh oh!

mrunalp commented Aug 24, 2026

Uh oh!

github-actions Bot commented Aug 24, 2026

Uh oh!

johntmyers commented Aug 24, 2026

Uh oh!

github-actions Bot commented Aug 24, 2026

Uh oh!

johntmyers commented Aug 24, 2026

PR Review Status

Uh oh!

Uh oh!

Uh oh!

Uh oh!

russellb commented Aug 25, 2026

Uh oh!

johntmyers commented Aug 25, 2026

Uh oh!

johntmyers commented Aug 25, 2026

Uh oh!

johntmyers commented Aug 25, 2026

Re-check After Author Update

Uh oh!

johntmyers commented Aug 28, 2026

Blocker Follow-Up Nudge

Uh oh!

russellb commented Aug 28, 2026

Blocker Follow-Up Nudge

Uh oh!

johntmyers commented Aug 28, 2026

Uh oh!

johntmyers commented Aug 28, 2026

Re-check After Author Update

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants