Skip to content

chore(featureflags): remove sandbox-metrics read/write kill-switch flags - #3162

Merged
arkamar merged 1 commit into
mainfrom
remove-sandbox-metrics-read-write-flags
Jul 2, 2026
Merged

chore(featureflags): remove sandbox-metrics read/write kill-switch flags#3162
arkamar merged 1 commit into
mainfrom
remove-sandbox-metrics-read-write-flags

Conversation

@arkamar

@arkamar arkamar commented Jul 1, 2026

Copy link
Copy Markdown
Member

These flags were kill-switches for the ClickHouse metrics pipeline, both defaulting to true (set in 63a151a, "chore(metrics): turn feature flags on by default", #2202). This hardwires the always-on path: the orchestrator always collects sandbox metrics and the API always queries them, removing the flag declarations, the UseClickhouseMetrics field on Checks, the collection skip, the four handler short-circuits, and now-unused featureflag context and imports. No references remain.

Link: #2202

These flags were kill-switches for the ClickHouse metrics pipeline, both
defaulting to true (set in 63a151a, "chore(metrics): turn feature
flags on by default", #2202). This hardwires the always-on path: the
orchestrator always collects sandbox metrics and the API always queries
them, removing the flag declarations, the UseClickhouseMetrics field on
Checks, the collection skip, the four handler short-circuits, and
now-unused featureflag context and imports. No references remain.

Link: #2202
@cursor

cursor Bot commented Jul 1, 2026

Copy link
Copy Markdown

PR Summary

Low Risk
Behavior change is making an already-default-on pipeline unconditional; no new auth or data paths, only loss of an emergency off switch.

Overview
Removes the sandbox-metrics-read and sandbox-metrics-write LaunchDarkly flags and the code paths that used them as ClickHouse metrics kill-switches (they had defaulted to on).

On the API, metrics handlers no longer check the read flag or return empty payloads when disabled; they always query cluster resources or ClickHouse. Related LaunchDarkly context wiring and imports are dropped from those handlers.

On the orchestrator, sandbox Checks no longer carry UseClickhouseMetrics; NewChecks takes only the sandbox, and the metrics observer no longer skips sandboxes based on the write flag.

flags.go no longer declares the two bool flags.

Reviewed by Cursor Bugbot for commit e019bd4. Bugbot is set up for automated code reviews on this repo. Configure here.

@codecov

codecov Bot commented Jul 1, 2026

Copy link
Copy Markdown

❌ 4 Tests Failed:

Tests completed Failed Passed Skipped
3123 4 3119 8
View the top 3 failed test(s) by shortest run time
github.com/e2b-dev/infra/tests/integration/internal/tests/api/templates::TestTemplateBuildRUN
Stack Traces | 0s run time
=== RUN   TestTemplateBuildRUN
=== PAUSE TestTemplateBuildRUN
=== CONT  TestTemplateBuildRUN
--- FAIL: TestTemplateBuildRUN (0.00s)
github.com/e2b-dev/infra/tests/integration/internal/tests/api/templates::TestUpdateTemplateNotOwnedByTeam
Stack Traces | 154s run time
=== RUN   TestUpdateTemplateNotOwnedByTeam
=== PAUSE TestUpdateTemplateNotOwnedByTeam
=== CONT  TestUpdateTemplateNotOwnedByTeam
    template_update_test.go:205: Build failed: {<nil> build was cancelled <nil>}
--- FAIL: TestUpdateTemplateNotOwnedByTeam (154.22s)
github.com/e2b-dev/infra/tests/integration/internal/tests/api/templates::TestTemplateBuildRUN/Single_RUN_command
Stack Traces | 161s run time
=== RUN   TestTemplateBuildRUN/Single_RUN_command
=== PAUSE TestTemplateBuildRUN/Single_RUN_command
=== CONT  TestTemplateBuildRUN/Single_RUN_command
    build_template_test.go:133: test-ubuntu-run: [info] Building template nc8lozxs0e1lk1xgwcrk/1c226a72-0f23-4dce-9f4a-4d31f718840d
    build_template_test.go:133: test-ubuntu-run: [info] [base] FROM ubuntu:22.04 [f9f564014e009a9561a82bf8c84f9314242971e833fb019936654ecba452f184]
    build_template_test.go:133: test-ubuntu-run: [info] Base Docker image size: 30 MB
    build_template_test.go:133: test-ubuntu-run: [info] Creating file system and pulling Docker image
    build_template_test.go:133: test-ubuntu-run: [info] Uncompressing layer sha256:40d16f30db405106ef8074779bdf41f012465c2a785bbeaa2eab9f2081099b47 30 MB
    build_template_test.go:133: test-ubuntu-run: [info] Uncompressing layer sha256:c8eeccbfff3f5c9f59179309d06c5d00d551eb815dbdc776902b9cb7962e5922 13 MB
    build_template_test.go:133: test-ubuntu-run: [info] Uncompressing layer sha256:8c4b1b28875140ed3abacaf16ad0d696f6bef912f52d2148f261a23e3349465b 168 B
    build_template_test.go:133: test-ubuntu-run: [info] Layers extracted
    build_template_test.go:133: test-ubuntu-run: [info] Root filesystem structure: bin, boot, dev, etc, home, lib, lib32, lib64, libx32, media, mnt, opt, proc, root, run, sbin, srv, sys, tmp, usr, var
    build_template_test.go:133: test-ubuntu-run: [info] Provisioning sandbox template
    build_template_test.go:133: test-ubuntu-run: [info] Provisioning was successful, cleaning up
    build_template_test.go:133: test-ubuntu-run: [info] Sandbox template provisioned
    build_template_test.go:133: test-ubuntu-run: [info] [base] DEFAULT USER user [49e586c2171254c6bc4a09e84eedac32dbcf113a158c24248129af2f49cbed74]
    build_template_test.go:133: test-ubuntu-run: [info] [builder 1/1] RUN echo 'Hello, World!' [c72b4f813c2a16b0fc1a1c5da7b1365a304cbac516b22dc304a71f70aae48ac0]
    build_template_test.go:133: test-ubuntu-run: [info] [builder 1/1] [stdout]: Hello, World!
    build_template_test.go:133: test-ubuntu-run: [info] [finalize] Finalizing template build [92c524e30533398ebb41ce04c2596130f0cdecc9aa328e28fdb16a1b11f61d62]
    build_template_test.go:133: test-ubuntu-run: [error] Build failed: build was cancelled
    build_template_test.go:166: Build failed: {<nil> build was cancelled <nil>}
--- FAIL: TestTemplateBuildRUN/Single_RUN_command (160.54s)
View the full list of 1 ❄️ flaky test(s)
github.com/e2b-dev/infra/tests/integration/internal/tests/api/templates::TestUpdateTemplateWithAPIKey

Flake rate in main: 13.78% (Passed 344 times, Failed 55 times)

Stack Traces | 155s run time
=== RUN   TestUpdateTemplateWithAPIKey
=== PAUSE TestUpdateTemplateWithAPIKey
=== CONT  TestUpdateTemplateWithAPIKey
    template_update_test.go:148: Build failed: {<nil> build was cancelled <nil>}
--- FAIL: TestUpdateTemplateWithAPIKey (155.32s)

To view more test analytics, go to the Test Analytics Dashboard
📋 Got 3 mins? Take this short survey to help us improve Test Analytics.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

A nil pointer dereference can occur in the sandbox observer if it attempts to fetch metrics before the sandbox checks are fully initialized. Additionally, initializing sandbox checks after assigning the network to the sandbox exposes a race condition where concurrent observers or cleanup routines accessing the sandbox from the map can encounter a nil checks pointer.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread packages/orchestrator/pkg/metrics/sandboxes.go
Comment thread packages/orchestrator/pkg/sandbox/sandbox.go

@claude claude Bot 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.

LGTM — mechanical removal of two always-on kill-switch flags and their now-dead branches.

Extended reasoning...

Overview

This PR removes two ClickHouse sandbox metrics kill-switch feature flags (sandbox-metrics-write and sandbox-metrics-read) that have been defaulting to true since #2202. Touches 8 files: 4 API handlers lose short-circuit branches that returned empty results when the read flag was disabled; orchestrator/pkg/metrics/sandboxes.go loses the write-flag skip; Checks loses the UseClickhouseMetrics field; sandbox.go loses two MetricsWriteFlag lookups on Create/Resume; and flags.go loses the two flag declarations. Follows the exact same pattern as the recently-merged #3158 (removal of the dead EdgeProvidedSandboxMetricsFlag).

Security risks

None. This is a pure code-deletion refactor — no auth, permissions, input validation, or data-boundary changes. The removed branches only ever returned empty JSON collections when the flag was off; hardwiring them to the always-on path matches prod behavior since the flags were flipped in #2202.

Level of scrutiny

Low. Mechanical cleanup of dead kill-switch flags following a documented pattern. I grepped the tree for MetricsWriteFlag, MetricsReadFlag, UseClickhouseMetrics, and the LD flag names — zero remaining references, so the removal is complete and self-contained.

Other factors

The bug hunting system found no issues. The prior sibling PR (#3158) removing a related sandbox-metrics flag was recently merged, confirming this is an ongoing intentional cleanup. Import removals are consistent (featureflags, ldcontext, logger where only used for the removed debug log) and the NewChecks signature simplification is applied at both call sites.

@arkamar
arkamar merged commit ad26b0e into main Jul 2, 2026
55 checks passed
@arkamar
arkamar deleted the remove-sandbox-metrics-read-write-flags branch July 2, 2026 06:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants