From 3c29a3656f4ecf5f1cfda9f74344fa4132ebca75 Mon Sep 17 00:00:00 2001 From: emily-shen <69125074+emily-shen@users.noreply.github.com> Date: Fri, 15 Nov 2024 16:25:12 +0000 Subject: [PATCH] respect metric enabled/disabled --- packages/wrangler/src/config/index.ts | 71 +++++++++++-------- packages/wrangler/src/index.ts | 14 ++-- .../wrangler/src/metrics/metrics-config.ts | 4 +- .../src/metrics/metrics-dispatcher.ts | 16 +++-- packages/wrangler/src/metrics/send-event.ts | 16 ----- 5 files changed, 65 insertions(+), 56 deletions(-) diff --git a/packages/wrangler/src/config/index.ts b/packages/wrangler/src/config/index.ts index b1973e748b631..296f5fd15cd8d 100644 --- a/packages/wrangler/src/config/index.ts +++ b/packages/wrangler/src/config/index.ts @@ -55,35 +55,13 @@ export function readConfig( requirePagesConfig?: boolean, hideWarnings: boolean = false ): Config { - const isJsonConfigEnabled = - getFlag("JSON_CONFIG_FILE") ?? args.experimentalJsonConfig; - let rawConfig: RawConfig = {}; - - if (!configPath) { - configPath = findWranglerToml(process.cwd(), isJsonConfigEnabled); - } + const { + rawConfig, + configPath: updateConfigPath, + isJsonConfigEnabled, + } = parseConfig(configPath, args, requirePagesConfig); - try { - // Load the configuration from disk if available - if (configPath?.endsWith("toml")) { - rawConfig = parseTOML(readFileSync(configPath), configPath); - } else if (configPath?.endsWith("json") || configPath?.endsWith("jsonc")) { - rawConfig = parseJSONC(readFileSync(configPath), configPath); - } - } catch (e) { - // Swallow parsing errors if we require a pages config file. - // At this point, we can't tell if the user intended to provide a Pages config file (and so should see the parsing error) or not (and so shouldn't). - // We err on the side of swallowing the error so as to not break existing projects - if (requirePagesConfig) { - logger.error(e); - throw new FatalError( - "Your wrangler.toml is not a valid Pages config file", - EXIT_CODE_INVALID_PAGES_CONFIG - ); - } else { - throw e; - } - } + configPath = updateConfigPath; /** * Check if configuration file belongs to a Pages project. @@ -154,6 +132,43 @@ export function readConfig( return config; } +export const parseConfig = ( + configPath: string | undefined, + // Include command specific args as well as the wrangler global flags + args: ReadConfigCommandArgs, + requirePagesConfig?: boolean +) => { + const isJsonConfigEnabled = + getFlag("JSON_CONFIG_FILE") ?? args.experimentalJsonConfig; + let rawConfig: RawConfig = {}; + + if (!configPath) { + configPath = findWranglerToml(process.cwd(), isJsonConfigEnabled); + } + + try { + // Load the configuration from disk if available + if (configPath?.endsWith("toml")) { + rawConfig = parseTOML(readFileSync(configPath), configPath); + } else if (configPath?.endsWith("json") || configPath?.endsWith("jsonc")) { + rawConfig = parseJSONC(readFileSync(configPath), configPath); + } + } catch (e) { + // Swallow parsing errors if we require a pages config file. + // At this point, we can't tell if the user intended to provide a Pages config file (and so should see the parsing error) or not (and so shouldn't). + // We err on the side of swallowing the error so as to not break existing projects + if (requirePagesConfig) { + logger.error(e); + throw new FatalError( + "Your wrangler.toml is not a valid Pages config file", + EXIT_CODE_INVALID_PAGES_CONFIG + ); + } else { + throw e; + } + } + return { rawConfig, configPath, isJsonConfigEnabled }; +}; /** * Modifies the provided config to support python workers, if the entrypoint is a .py file */ diff --git a/packages/wrangler/src/index.ts b/packages/wrangler/src/index.ts index 6f6568a87c091..507b7fb1a0d58 100644 --- a/packages/wrangler/src/index.ts +++ b/packages/wrangler/src/index.ts @@ -7,7 +7,7 @@ import makeCLI from "yargs"; import { version as wranglerVersion } from "../package.json"; import { ai } from "./ai"; import { cloudchamber } from "./cloudchamber"; -import { loadDotEnv } from "./config"; +import { loadDotEnv, parseConfig } from "./config"; import { createCommandRegister } from "./core/register-commands"; import { d1 } from "./d1"; import { deleteHandler, deleteOptions } from "./delete"; @@ -52,7 +52,7 @@ import "./user/commands"; import "./metrics/commands"; import { demandSingleValue } from "./core"; import { logBuildFailure, logger, LOGGER_LEVELS } from "./logger"; -import { sendNewMetricsEvent } from "./metrics/send-event"; +import { getMetricsDispatcher } from "./metrics"; import { mTlsCertificateCommands } from "./mtls-certificate/cli"; import { writeOutput } from "./output"; import { pages } from "./pages"; @@ -686,7 +686,7 @@ export async function main(argv: string[]): Promise { const wrangler = createCLIParser(argv); let command: string | undefined; let metricsArgs: Record | undefined; - + let dispatcher: ReturnType | undefined; // Register Yargs middleware to record command as Sentry breadcrumb let recordedCommand = false; const wranglerWithMiddleware = wrangler.middleware((args) => { @@ -698,10 +698,12 @@ export async function main(argv: string[]): Promise { // `args._` doesn't include any positional arguments (e.g. script name, // key to fetch) or flags + const { rawConfig } = parseConfig(args.config, args, false); + dispatcher = getMetricsDispatcher({ sendMetrics: rawConfig.send_metrics }); command = `wrangler ${args._.join(" ")}`; metricsArgs = args; addBreadcrumb(command); - void sendNewMetricsEvent("wrangler command started", { + void dispatcher.sendNewEvent("wrangler command started", { command, args, }); @@ -713,7 +715,7 @@ export async function main(argv: string[]): Promise { const durationMs = Date.now() - startTime; - void sendNewMetricsEvent("wrangler command completed", { + void dispatcher?.sendNewEvent("wrangler command completed", { command, args: metricsArgs, durationMs, @@ -829,7 +831,7 @@ export async function main(argv: string[]): Promise { const durationMs = Date.now() - startTime; - void sendNewMetricsEvent("wrangler command errored", { + void dispatcher?.sendNewEvent("wrangler command errored", { command, args: metricsArgs, durationMs, diff --git a/packages/wrangler/src/metrics/metrics-config.ts b/packages/wrangler/src/metrics/metrics-config.ts index b4b4f86bf6df3..7a4ed47dcc75b 100644 --- a/packages/wrangler/src/metrics/metrics-config.ts +++ b/packages/wrangler/src/metrics/metrics-config.ts @@ -53,9 +53,9 @@ export interface MetricsConfig { * If the current process is not interactive then we will cannot prompt the user and just assume * we cannot send metrics if there is no cached or project-level preference available. */ -export async function getMetricsConfig({ +export function getMetricsConfig({ sendMetrics, -}: MetricsConfigOptions): Promise { +}: MetricsConfigOptions): MetricsConfig { const config = readMetricsConfig(); const deviceId = getDeviceId(config); diff --git a/packages/wrangler/src/metrics/metrics-dispatcher.ts b/packages/wrangler/src/metrics/metrics-dispatcher.ts index 8c5b79c8a5e8d..cf5de6d4863c1 100644 --- a/packages/wrangler/src/metrics/metrics-dispatcher.ts +++ b/packages/wrangler/src/metrics/metrics-dispatcher.ts @@ -4,7 +4,7 @@ import { version as wranglerVersion } from "../../package.json"; import { logger } from "../logger"; import { getMetricsConfig, readMetricsConfig } from "./metrics-config"; import type { MetricsConfigOptions } from "./metrics-config"; -import type { CommonEventProperties } from "./send-event"; +import type { CommonEventProperties, Events } from "./send-event"; // The SPARROW_SOURCE_KEY is provided at esbuild time as a `define` for production and beta // releases. Otherwise it is left undefined, which automatically disables metrics requests. @@ -26,7 +26,7 @@ function getPlatform() { } } -export async function getMetricsDispatcher(options: MetricsConfigOptions) { +export function getMetricsDispatcher(options: MetricsConfigOptions) { const platform = getPlatform(); const packageManager = whichPmRuns()?.name; const isFirstUsage = readMetricsConfig().permission === undefined; @@ -54,6 +54,15 @@ export async function getMetricsDispatcher(options: MetricsConfigOptions) { async identify(properties: Properties): Promise { await dispatch({ type: "identify", name: "identify", properties }); }, + async sendNewEvent( + name: EventName, + properties: Omit< + Extract["properties"], + keyof CommonEventProperties + > + ): Promise { + await dispatch({ type: "event", name, properties }); + }, }; async function dispatch(event: { @@ -61,8 +70,7 @@ export async function getMetricsDispatcher(options: MetricsConfigOptions) { name: string; properties: Properties; }): Promise { - // TODO: make not async - const metricsConfig = await getMetricsConfig(options); + const metricsConfig = getMetricsConfig(options); const commonEventProperties: CommonEventProperties = { amplitude_session_id, diff --git a/packages/wrangler/src/metrics/send-event.ts b/packages/wrangler/src/metrics/send-event.ts index c7cb4916a33d2..2e224edd5f9e2 100644 --- a/packages/wrangler/src/metrics/send-event.ts +++ b/packages/wrangler/src/metrics/send-event.ts @@ -170,19 +170,3 @@ export type Events = errorType: string | undefined; }; }; - -export async function sendNewMetricsEvent( - event: EventName, - properties: Omit< - Extract["properties"], - keyof CommonEventProperties - >, - options?: MetricsConfigOptions -): Promise { - try { - const metricsDispatcher = await getMetricsDispatcher(options ?? {}); - await metricsDispatcher.sendEvent(event, properties); - } catch (err) { - logger.debug("Error sending metrics event", err); - } -}