From b63401c01a395cf6f032c6b4f2b9985e53eed43a Mon Sep 17 00:00:00 2001 From: Ryan Ling Date: Mon, 20 May 2024 09:52:09 +1000 Subject: [PATCH 1/2] template: Make all configuration values explicit Follows on from seek-oss/datadog-custom-metrics#286. --- .changeset/tasty-schools-sort.md | 9 +++++++++ package.json | 2 +- template/express-rest-api/package.json | 2 ++ template/express-rest-api/src/config.ts | 20 +++++++++++++------ .../express-rest-api/src/framework/metrics.ts | 11 ++++++++++ template/koa-rest-api/package.json | 2 +- template/koa-rest-api/src/config.ts | 18 +++++++++++++---- template/lambda-sqs-worker-cdk/src/config.ts | 16 ++++++--------- template/lambda-sqs-worker/src/config.ts | 13 ++++++++---- 9 files changed, 67 insertions(+), 26 deletions(-) create mode 100644 .changeset/tasty-schools-sort.md create mode 100644 template/express-rest-api/src/framework/metrics.ts diff --git a/.changeset/tasty-schools-sort.md b/.changeset/tasty-schools-sort.md new file mode 100644 index 000000000..285acc62b --- /dev/null +++ b/.changeset/tasty-schools-sort.md @@ -0,0 +1,9 @@ +--- +'skuba': patch +--- + +template: Make all configuration values explicit + +Previously, `src/config.ts` included optional configuration values and inheritance between environments in the spirit of [DRY](https://en.wikipedia.org/wiki/Don%27t_repeat_yourself). While the templated file was wired up in a "safe" way—the production environment never inherited from other environments and explicitly specified all its configuration values—its pattern was misappropriated elsewhere and led to local configuration values affecting production environments. + +Instead, we now list all configuration values explicitly against each environment. diff --git a/package.json b/package.json index 7d3f1a2e5..cf9560d3a 100644 --- a/package.json +++ b/package.json @@ -156,7 +156,7 @@ "optional": true } }, - "packageManager": "pnpm@9.0.5", + "packageManager": "pnpm@9.1.0", "engines": { "node": ">=18.18.0" }, diff --git a/template/express-rest-api/package.json b/template/express-rest-api/package.json index 489bf0837..de44ebfaa 100644 --- a/template/express-rest-api/package.json +++ b/template/express-rest-api/package.json @@ -14,6 +14,8 @@ "dependencies": { "@seek/logger": "^6.0.0", "express": "^4.17.1", + "hot-shots": "^10.0.0", + "seek-datadog-custom-metrics": "^4.6.3", "skuba-dive": "^2.0.0" }, "devDependencies": { diff --git a/template/express-rest-api/src/config.ts b/template/express-rest-api/src/config.ts index 3f785b971..33be89a76 100644 --- a/template/express-rest-api/src/config.ts +++ b/template/express-rest-api/src/config.ts @@ -7,8 +7,8 @@ interface Config { name: string; version: string; - metricsServer?: string; - port?: number; + metricsServer: string | null; + port: number | null; } type Environment = (typeof environments)[number]; @@ -26,19 +26,27 @@ const configs: Record Omit> = { logLevel: 'debug', name: '<%- serviceName %>', version: 'local', + + metricsServer: null, + port: null, }), test: () => ({ - ...configs.local(), - logLevel: Env.string('LOG_LEVEL', { default: 'silent' }), + name: '<%- serviceName %>', version: 'test', + + metricsServer: null, + port: null, }), [dev]: () => ({ - ...configs[prod](), - logLevel: 'debug', + name: Env.string('SERVICE'), + version: Env.string('VERSION'), + + metricsServer: 'localhost', + port: Env.nonNegativeInteger('PORT'), }), [prod]: () => ({ diff --git a/template/express-rest-api/src/framework/metrics.ts b/template/express-rest-api/src/framework/metrics.ts new file mode 100644 index 000000000..6c6fdd632 --- /dev/null +++ b/template/express-rest-api/src/framework/metrics.ts @@ -0,0 +1,11 @@ +import { StatsD } from 'hot-shots'; +import { createStatsDClient } from 'seek-datadog-custom-metrics'; + +import { config } from 'src/config'; + +import { logger } from './logging'; + +/* istanbul ignore next: StatsD client is not our responsibility */ +export const metricsClient = createStatsDClient(StatsD, config, (err) => + logger.error({ err }, 'StatsD error'), +); diff --git a/template/koa-rest-api/package.json b/template/koa-rest-api/package.json index 1b4e47da5..23908ad4d 100644 --- a/template/koa-rest-api/package.json +++ b/template/koa-rest-api/package.json @@ -26,7 +26,7 @@ "koa": "^2.13.4", "koa-bodyparser": "^4.3.0", "koa-compose": "^4.2.0", - "seek-datadog-custom-metrics": "^4.2.1", + "seek-datadog-custom-metrics": "^4.6.3", "seek-koala": "^7.0.0", "skuba-dive": "^2.0.0", "zod": "^3.19.1" diff --git a/template/koa-rest-api/src/config.ts b/template/koa-rest-api/src/config.ts index 3f785b971..7f0deccb7 100644 --- a/template/koa-rest-api/src/config.ts +++ b/template/koa-rest-api/src/config.ts @@ -7,8 +7,8 @@ interface Config { name: string; version: string; - metricsServer?: string; - port?: number; + metricsServer: string | null; + port: number | null; } type Environment = (typeof environments)[number]; @@ -26,19 +26,29 @@ const configs: Record Omit> = { logLevel: 'debug', name: '<%- serviceName %>', version: 'local', + + metricsServer: null, + port: null, }), test: () => ({ - ...configs.local(), - logLevel: Env.string('LOG_LEVEL', { default: 'silent' }), + name: '<%- serviceName %>', version: 'test', + + metricsServer: null, + port: null, }), [dev]: () => ({ ...configs[prod](), logLevel: 'debug', + name: Env.string('SERVICE'), + version: Env.string('VERSION'), + + metricsServer: 'localhost', + port: Env.nonNegativeInteger('PORT'), }), [prod]: () => ({ diff --git a/template/lambda-sqs-worker-cdk/src/config.ts b/template/lambda-sqs-worker-cdk/src/config.ts index 022638a9d..946645108 100644 --- a/template/lambda-sqs-worker-cdk/src/config.ts +++ b/template/lambda-sqs-worker-cdk/src/config.ts @@ -10,10 +10,7 @@ interface Config { type Environment = (typeof environments)[number]; -const dev = 'dev'; -const prod = 'prod'; - -const environments = ['local', 'test', dev, prod] as const; +const environments = ['local', 'test', 'dev', 'prod'] as const; const environment = Env.oneOf(environments)('ENVIRONMENT'); @@ -26,19 +23,18 @@ const configs: Record Omit> = { }), test: () => ({ - ...configs.local(), - logLevel: Env.string('LOG_LEVEL', { default: 'silent' }), + name: '<%- serviceName %>', version: 'test', }), - [dev]: () => ({ - ...configs[prod](), - + dev: () => ({ logLevel: 'debug', + name: Env.string('SERVICE'), + version: Env.string('VERSION'), }), - [prod]: () => ({ + prod: () => ({ logLevel: 'info', name: Env.string('SERVICE'), version: Env.string('VERSION'), diff --git a/template/lambda-sqs-worker/src/config.ts b/template/lambda-sqs-worker/src/config.ts index e341dafb4..34cc9ffc2 100644 --- a/template/lambda-sqs-worker/src/config.ts +++ b/template/lambda-sqs-worker/src/config.ts @@ -29,16 +29,21 @@ const configs: Record Omit> = { }), test: () => ({ - ...configs.local(), - logLevel: Env.string('LOG_LEVEL', { default: 'silent' }), + metrics: false, + name: '<%- serviceName %>', version: 'test', + + destinationSnsTopicArn: 'arn:aws:sns:us-east-2:123456789012:destination', }), dev: () => ({ - ...configs.prod(), - logLevel: 'debug', + metrics: true, + name: Env.string('SERVICE'), + version: Env.string('VERSION'), + + destinationSnsTopicArn: Env.string('DESTINATION_SNS_TOPIC_ARN'), }), prod: () => ({ From 759bb7d15f7d1c4d0d13b836524952e775fc9282 Mon Sep 17 00:00:00 2001 From: Ryan Ling Date: Mon, 20 May 2024 09:56:55 +1000 Subject: [PATCH 2/2] Rename a variable --- docs/cli/run.md | 4 ++-- template/express-rest-api/src/framework/logging.ts | 2 +- template/express-rest-api/src/listen.ts | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/docs/cli/run.md b/docs/cli/run.md index 74381aac4..2c46a52a1 100644 --- a/docs/cli/run.md +++ b/docs/cli/run.md @@ -44,7 +44,7 @@ For example, your `src/app.ts` may look like: import 'skuba-dive/register'; // You can use the `src` module alias after registration. -import { rootLogger } 'src/framework/logging'; +import { logger } 'src/framework/logging'; ``` --- @@ -73,7 +73,7 @@ For example, your `src/app.ts` may look like: import 'skuba-dive/register'; // You can use the `src` module alias after registration. -import { rootLogger } 'src/framework/logging'; +import { logger } 'src/framework/logging'; ``` ### Start an executable script diff --git a/template/express-rest-api/src/framework/logging.ts b/template/express-rest-api/src/framework/logging.ts index 7c16360da..3b7a70251 100644 --- a/template/express-rest-api/src/framework/logging.ts +++ b/template/express-rest-api/src/framework/logging.ts @@ -2,7 +2,7 @@ import createLogger from '@seek/logger'; import { config } from 'src/config'; -export const rootLogger = createLogger({ +export const logger = createLogger({ base: { environment: config.environment, version: config.version, diff --git a/template/express-rest-api/src/listen.ts b/template/express-rest-api/src/listen.ts index a0bcbcf0e..9739641de 100644 --- a/template/express-rest-api/src/listen.ts +++ b/template/express-rest-api/src/listen.ts @@ -2,7 +2,7 @@ import './register'; import app from './app'; import { config } from './config'; -import { rootLogger } from './framework/logging'; +import { logger } from './framework/logging'; // If your application is deployed with more than 1 vCPU you can delete this // file and use a clustering utility to run `lib/app`. @@ -11,7 +11,7 @@ const listener = app.listen(config.port, () => { const address = listener.address(); if (typeof address === 'object' && address) { - rootLogger.debug(`listening on port ${address.port}`); + logger.debug(`listening on port ${address.port}`); } });