From 88031094968764be853f5d13617d9a5cd3966faf Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Wed, 7 Aug 2024 17:50:39 +0100 Subject: [PATCH] Add DataDog tracing to feature flags. --- packages/backend-core/src/features/index.ts | 116 ++++++++++-------- .../src/features/tests/features.spec.ts | 38 +++++- 2 files changed, 105 insertions(+), 49 deletions(-) diff --git a/packages/backend-core/src/features/index.ts b/packages/backend-core/src/features/index.ts index 4a29dc6daf..2ffa35974f 100644 --- a/packages/backend-core/src/features/index.ts +++ b/packages/backend-core/src/features/index.ts @@ -3,6 +3,7 @@ import * as context from "../context" import { cloneDeep } from "lodash" import { PostHog, PostHogOptions } from "posthog-node" import { IdentityType } from "@budibase/types" +import tracer from "dd-trace" let posthog: PostHog | undefined export function init(opts?: PostHogOptions) { @@ -119,65 +120,84 @@ function isFlagName(name: string): name is keyof Flags { * they will be accessed through this function as well. */ export async function fetch(): Promise { - const flags = defaultFlags() + return await tracer.trace("features.fetch", async span => { + const tags: Record = {} - const currentTenantId = context.getTenantId() + const flags = defaultFlags() - const split = (env.TENANT_FEATURE_FLAGS || "") - .split(",") - .map(x => x.split(":")) - for (const [tenantId, ...features] of split) { - if (!tenantId || (tenantId !== "*" && tenantId !== currentTenantId)) { - continue - } + const currentTenantId = context.getTenantId() + const specificallySetFalse = new Set() - for (let feature of features) { - let value = true - if (feature.startsWith("!")) { - feature = feature.slice(1) - value = false - } - - if (!isFlagName(feature)) { - throw new Error(`Feature: ${feature} is not an allowed option`) - } - - if (typeof flags[feature] !== "boolean") { - throw new Error(`Feature: ${feature} is not a boolean`) - } - - // @ts-ignore - flags[feature] = value - } - } - - const identity = context.getIdentity() - if (posthog && identity?.type === IdentityType.USER) { - const posthogFlags = await posthog.getAllFlagsAndPayloads(identity._id) - for (const [name, value] of Object.entries(posthogFlags.featureFlags)) { - const key = name as keyof typeof FLAGS - const flag = FLAGS[key] - if (!flag) { - // We don't want an unexpected PostHog flag to break the app, so we - // just log it and continue. - console.warn(`Unexpected posthog flag "${name}": ${value}`) + const split = (env.TENANT_FEATURE_FLAGS || "") + .split(",") + .map(x => x.split(":")) + for (const [tenantId, ...features] of split) { + if (!tenantId || (tenantId !== "*" && tenantId !== currentTenantId)) { continue } - const payload = posthogFlags.featureFlagPayloads?.[name] + for (let feature of features) { + let value = true + if (feature.startsWith("!")) { + feature = feature.slice(1) + value = false + specificallySetFalse.add(feature) + } + + if (!isFlagName(feature)) { + throw new Error(`Feature: ${feature} is not an allowed option`) + } + + if (typeof flags[feature] !== "boolean") { + throw new Error(`Feature: ${feature} is not a boolean`) + } - try { // @ts-ignore - flags[key] = flag.parse(payload || value) - } catch (err) { - // We don't want an invalid PostHog flag to break the app, so we just - // log it and continue. - console.warn(`Error parsing posthog flag "${name}": ${value}`, err) + flags[feature] = value + tags[`flags.${feature}.source`] = "environment" } } - } - return flags + const identity = context.getIdentity() + if (posthog && identity?.type === IdentityType.USER) { + const posthogFlags = await posthog.getAllFlagsAndPayloads(identity._id) + for (const [name, value] of Object.entries(posthogFlags.featureFlags)) { + const key = name as keyof typeof FLAGS + const flag = FLAGS[key] + if (!flag) { + // We don't want an unexpected PostHog flag to break the app, so we + // just log it and continue. + console.warn(`Unexpected posthog flag "${name}": ${value}`) + continue + } + + if (flags[key] === true || specificallySetFalse.has(key)) { + // If the flag is already set to through environment variables, we + // don't want to override it back to false here. + continue + } + + const payload = posthogFlags.featureFlagPayloads?.[name] + + try { + // @ts-ignore + flags[key] = flag.parse(payload || value) + tags[`flags.${key}.source`] = "posthog" + } catch (err) { + // We don't want an invalid PostHog flag to break the app, so we just + // log it and continue. + console.warn(`Error parsing posthog flag "${name}": ${value}`, err) + } + } + } + + for (const [key, value] of Object.entries(flags)) { + tags[`flags.${key}.value`] = value + } + span?.addTags(tags) + + return flags + }) } // Gets a single feature flag value. This is a convenience function for diff --git a/packages/backend-core/src/features/tests/features.spec.ts b/packages/backend-core/src/features/tests/features.spec.ts index 25e9765287..1b85a74024 100644 --- a/packages/backend-core/src/features/tests/features.spec.ts +++ b/packages/backend-core/src/features/tests/features.spec.ts @@ -79,7 +79,7 @@ describe("feature flags", () => { describe("posthog", () => { const identity: IdentityContext = { _id: "us_1234", - tenantId: "budibase", + tenantId: "tenant1", type: IdentityType.USER, email: "test@example.com", firstName: "Test", @@ -171,5 +171,41 @@ describe("feature flags", () => { await expect(fetch()).resolves.not.toThrow() }) }) + + it("should not override flags set in the environment", async () => { + mockFlags({ + featureFlags: { + _TEST_BOOLEAN: false, + }, + }) + + await withEnv( + { TENANT_FEATURE_FLAGS: `${identity.tenantId}:_TEST_BOOLEAN` }, + async () => { + await context.doInIdentityContext(identity, async () => { + const flags = await fetch() + expect(flags._TEST_BOOLEAN).toBe(true) + }) + } + ) + }) + + it("should not override flags set in the environment with a ! prefix", async () => { + mockFlags({ + featureFlags: { + _TEST_BOOLEAN: true, + }, + }) + + await withEnv( + { TENANT_FEATURE_FLAGS: `${identity.tenantId}:!_TEST_BOOLEAN` }, + async () => { + await context.doInIdentityContext(identity, async () => { + const flags = await fetch() + expect(flags._TEST_BOOLEAN).toBe(false) + }) + } + ) + }) }) })