Skip to content

fix: do not warn on valid known experiments #18514

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jun 24, 2025
Merged

fix: do not warn on valid known experiments #18514

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
38d9e77
coderd: rename handler to match route and semantics
johnstcn Jun 23, 2025
9a359e8
coderd: log on invalid experiments
johnstcn Jun 23, 2025
0ba4772
generate known experiments and warn only invalid experiments only in …
johnstcn Jun 23, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions coderd/coderd.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -972,7 +972,7 @@ func New(options *Options) *API {
})
r.Route("/experiments", func(r chi.Router) {
r.Use(apiKeyMiddleware)
r.Get("/available", handleExperimentsSafe)
r.Get("/available", handleExperimentsAvailable)
Copy link
Member

Choose a reason for hiding this comment

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

Kinda unfortunate we have /available that returns only the safe experiments. Ideally there would be a /available and /safe. Or just have /available return a map[string]bool where the bool is true for safe or something.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah I considered also returning all available experiments here but elected to keep the scope of this PR small.

r.Get("/", api.handleExperimentsGet)
})
r.Get("/updatecheck", api.updateCheck)
Expand Down Expand Up @@ -1895,7 +1895,9 @@ func ReadExperiments(log slog.Logger, raw []string) codersdk.Experiments {
exps = append(exps, codersdk.ExperimentsSafe...)
default:
ex := codersdk.Experiment(strings.ToLower(v))
if !slice.Contains(codersdk.ExperimentsSafe, ex) {
if !slice.Contains(codersdk.ExperimentsKnown, ex) {
log.Warn(context.Background(), "ignoring unknown experiment", slog.F("experiment", ex))
} else if !slice.Contains(codersdk.ExperimentsSafe, ex) {
Comment on lines +1898 to +1900
Copy link
Member

Choose a reason for hiding this comment

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

👍.

log.Warn(context.Background(), "🐉 HERE BE DRAGONS: opting into hidden experiment", slog.F("experiment", ex))
}
exps = append(exps, ex)
Expand Down
2 changes: 1 addition & 1 deletion coderd/experiments.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ func (api *API) handleExperimentsGet(rw http.ResponseWriter, r *http.Request) {
// @Tags General
// @Success 200 {array} codersdk.Experiment
// @Router /experiments/available [get]
func handleExperimentsSafe(rw http.ResponseWriter, r *http.Request) {
func handleExperimentsAvailable(rw http.ResponseWriter, r *http.Request) {
ctx := r.Context()
httpapi.Write(ctx, rw, http.StatusOK, codersdk.AvailableExperiments{
Safe: codersdk.ExperimentsSafe,
Expand Down
15 changes: 15 additions & 0 deletions codersdk/deployment.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3372,6 +3372,18 @@ const (
ExperimentAITasks Experiment = "ai-tasks" // Enables the new AI tasks feature.
)

// ExperimentsKnown should include all experiments defined above.
var ExperimentsKnown = Experiments{
ExperimentExample,
ExperimentAutoFillParameters,
ExperimentNotifications,
ExperimentWorkspaceUsage,
ExperimentWebPush,
ExperimentWorkspacePrebuilds,
ExperimentAgenticChat,
ExperimentAITasks,
}

// ExperimentsSafe should include all experiments that are safe for
// users to opt-in to via --experimental='*'.
// Experiments that are not ready for consumption by all users should
Expand All @@ -3384,6 +3396,9 @@ var ExperimentsSafe = Experiments{
// Multiple experiments may be enabled at the same time.
// Experiments are not safe for production use, and are not guaranteed to
// be backwards compatible. They may be removed or renamed at any time.
// The below typescript-ignore annotation allows our typescript generator
// to generate an enum list, which is used in the frontend.
// @typescript-ignore Experiments
type Experiments []Experiment

// Returns a list of experiments that are enabled for the deployment.
Expand Down
8 changes: 6 additions & 2 deletions site/src/api/queries/experiments.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
import { API } from "api/api";
import type { Experiments } from "api/typesGenerated";
import { type Experiment, Experiments } from "api/typesGenerated";
import type { MetadataState } from "hooks/useEmbeddedMetadata";
import { cachedQuery } from "./util";

const experimentsKey = ["experiments"] as const;

export const experiments = (metadata: MetadataState<Experiments>) => {
export const experiments = (metadata: MetadataState<Experiment[]>) => {
return cachedQuery({
metadata,
queryKey: experimentsKey,
Expand All @@ -19,3 +19,7 @@ export const availableExperiments = () => {
queryFn: async () => API.getAvailableExperiments(),
};
};

export const isKnownExperiment = (experiment: string): boolean => {
return Experiments.includes(experiment as Experiment);
};
Comment on lines +23 to +25
Copy link
Member Author

Choose a reason for hiding this comment

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

Alternatively, we could have the FE return the list of known experiments, but it seems silly to do this when we can just use the auto-generated type.

12 changes: 10 additions & 2 deletions site/src/api/typesGenerated.ts
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 3 additions & 3 deletions site/src/hooks/useEmbeddedMetadata.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import type {
AppearanceConfig,
BuildInfoResponse,
Entitlements,
Experiments,
Experiment,
Region,
User,
UserAppearanceSettings,
Expand All @@ -24,7 +24,7 @@ export const DEFAULT_METADATA_KEY = "property";
*/
type AvailableMetadata = Readonly<{
user: User;
experiments: Experiments;
experiments: Experiment[];
appearance: AppearanceConfig;
userAppearance: UserAppearanceSettings;
entitlements: Entitlements;
Expand Down Expand Up @@ -89,7 +89,7 @@ export class MetadataManager implements MetadataManagerApi {
userAppearance:
this.registerValue<UserAppearanceSettings>("userAppearance"),
entitlements: this.registerValue<Entitlements>("entitlements"),
experiments: this.registerValue<Experiments>("experiments"),
experiments: this.registerValue<Experiment[]>("experiments"),
"build-info": this.registerValue<BuildInfoResponse>("build-info"),
regions: this.registerRegionValue(),
tasksTabVisible: this.registerValue<boolean>("tasksTabVisible"),
Expand Down
4 changes: 2 additions & 2 deletions site/src/modules/dashboard/DashboardProvider.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { organizations } from "api/queries/organizations";
import type {
AppearanceConfig,
Entitlements,
Experiments,
Experiment,
Organization,
} from "api/typesGenerated";
import { ErrorAlert } from "components/Alert/ErrorAlert";
Expand All @@ -19,7 +19,7 @@ import { selectFeatureVisibility } from "./entitlements";

export interface DashboardValue {
entitlements: Entitlements;
experiments: Experiments;
experiments: Experiment[];
appearance: AppearanceConfig;
organizations: readonly Organization[];
showOrganizations: boolean;
Expand Down
8 changes: 6 additions & 2 deletions site/src/pages/DeploymentSettingsPage/OverviewPage/OverviewPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
import { deploymentDAUs } from "api/queries/deployment";
import { availableExperiments, experiments } from "api/queries/experiments";
import {
availableExperiments,
experiments,
isKnownExperiment,
} from "api/queries/experiments";
import { useEmbeddedMetadata } from "hooks/useEmbeddedMetadata";
import { useDeploymentConfig } from "modules/management/DeploymentConfigProvider";
import type { FC } from "react";
Expand All @@ -18,7 +22,7 @@ const OverviewPage: FC = () => {
const safeExperiments = safeExperimentsQuery.data?.safe ?? [];
const invalidExperiments =
enabledExperimentsQuery.data?.filter((exp) => {
return !safeExperiments.includes(exp);
return !isKnownExperiment(exp);
}) ?? [];

const { data: dailyActiveUsers } = useQuery(deploymentDAUs());
Expand Down
8 changes: 4 additions & 4 deletions site/src/pages/DeploymentSettingsPage/OverviewPage/OverviewPageView.stories.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ const meta: Meta<typeof OverviewPageView> = {
description:
"Enable one or more experiments. These are not ready for production. Separate multiple experiments with commas, or enter '*' to opt-in to all available experiments.",
flag: "experiments",
value: ["workspace_actions"],
value: ["example"],
flag_shorthand: "",
hidden: false,
},
Expand Down Expand Up @@ -82,8 +82,8 @@ export const allExperimentsEnabled: Story = {
hidden: false,
},
],
safeExperiments: ["shared-ports"],
invalidExperiments: ["invalid"],
safeExperiments: ["example"],
invalidExperiments: [],
},
};

Expand Down Expand Up @@ -118,7 +118,7 @@ export const invalidExperimentsEnabled: Story = {
hidden: false,
},
],
safeExperiments: ["shared-ports"],
safeExperiments: ["example"],
Copy link
Member Author

Choose a reason for hiding this comment

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

review: using the "example" experiment enum here instead

Copy link
Member

Choose a reason for hiding this comment

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

Makes sense to me. Using a consistent experiment that we don't need to keep changing.

I wonder if example is the best name, but I do not have a better suggestion.

ExperimentPlaceholder, ExperimentDummy, idk.

invalidExperiments: ["invalid"],
},
};
6 changes: 3 additions & 3 deletions site/src/pages/DeploymentSettingsPage/OverviewPage/OverviewPageView.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import AlertTitle from "@mui/material/AlertTitle";
import type {
DAUsResponse,
Experiments,
Experiment,
SerpentOption,
} from "api/typesGenerated";
import { Link } from "components/Link/Link";
Expand All @@ -22,8 +22,8 @@ import { UserEngagementChart } from "./UserEngagementChart";
type OverviewPageViewProps = {
deploymentOptions: SerpentOption[];
dailyActiveUsers: DAUsResponse | undefined;
readonly invalidExperiments: Experiments | string[];
readonly safeExperiments: Experiments | string[];
readonly invalidExperiments: readonly string[];
Comment on lines 23 to +25
Copy link
Member Author

Choose a reason for hiding this comment

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

review: this has to be a string because it might not be an Experiment

readonly safeExperiments: readonly Experiment[];
};

export const OverviewPageView: FC<OverviewPageViewProps> = ({
Expand Down
Loading