From 408ff5654cc04aae35d05eb5bbc47a51f99ec5b2 Mon Sep 17 00:00:00 2001 From: Blayne Chard Date: Wed, 22 Sep 2021 16:45:02 +1200 Subject: [PATCH] feat(server): use the lambda handler directly (#1870) * feat(server): use the lambda handler directly This gives a much better logging output by default This also enforces pretty printing of logs when run from the console * refactor: apply lint fix Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> --- packages/cli/src/cli/base.cli.ts | 6 --- packages/cli/src/cli/basemaps/index.ts | 6 --- packages/lambda-tiler/src/cli/dump.ts | 9 ++-- packages/lambda-tiler/src/cli/serve.ts | 3 -- packages/server/bin/basemaps-server.js | 3 +- packages/server/src/cli.ts | 7 +-- packages/server/src/server.ts | 70 +++++++++++--------------- packages/shared/src/log.ts | 15 ++---- 8 files changed, 41 insertions(+), 78 deletions(-) diff --git a/packages/cli/src/cli/base.cli.ts b/packages/cli/src/cli/base.cli.ts index 00fe2ac56..883fe1821 100644 --- a/packages/cli/src/cli/base.cli.ts +++ b/packages/cli/src/cli/base.cli.ts @@ -4,7 +4,6 @@ import { GitTag } from '@basemaps/shared/build/cli/git.tag.js'; import { CommandLineParser } from '@rushstack/ts-command-line'; import { readFileSync } from 'fs'; import path from 'path'; -import { PrettyTransform } from 'pretty-json-log'; import 'source-map-support/register.js'; import * as ulid from 'ulid'; import url from 'url'; @@ -35,11 +34,6 @@ export abstract class BaseCommandLine extends CommandLineParser { }); protected onExecute(): Promise { - // If the console is a tty pretty print the output - if (process.stdout.isTTY) { - LogConfig.setOutputStream(PrettyTransform.stream()); - } - if (this.verbose.value) { LogConfig.get().level = 'debug'; } else if (this.extraVerbose.value) { diff --git a/packages/cli/src/cli/basemaps/index.ts b/packages/cli/src/cli/basemaps/index.ts index 59647a4d0..1b5eaf666 100644 --- a/packages/cli/src/cli/basemaps/index.ts +++ b/packages/cli/src/cli/basemaps/index.ts @@ -1,6 +1,4 @@ #!/usr/bin/env node -import { LogConfig } from '@basemaps/shared'; -import { PrettyTransform } from 'pretty-json-log'; import 'source-map-support/register.js'; import { BaseCommandLine } from '../base.cli.js'; import { TileSetInvalidateAction } from './action.invalidate.js'; @@ -14,10 +12,6 @@ export class BasemapsCommandLine extends BaseCommandLine { }); this.addAction(new TileSetInfoAction()); this.addAction(new TileSetInvalidateAction()); - - if (process.stdout.isTTY) { - LogConfig.setOutputStream(PrettyTransform.stream()); - } } } diff --git a/packages/lambda-tiler/src/cli/dump.ts b/packages/lambda-tiler/src/cli/dump.ts index fd5101d09..e888d4cb1 100644 --- a/packages/lambda-tiler/src/cli/dump.ts +++ b/packages/lambda-tiler/src/cli/dump.ts @@ -1,16 +1,13 @@ import { Nztm2000Tms } from '@basemaps/geo'; -import { LambdaAlbRequest } from '@linzjs/lambda'; import { LogConfig } from '@basemaps/shared'; import { ImageFormat } from '@basemaps/tiler'; +import { LambdaAlbRequest } from '@linzjs/lambda'; +import { Context } from 'aws-lambda'; import { promises as fs } from 'fs'; -import { PrettyTransform } from 'pretty-json-log'; import { TileRoute } from '../routes/tile.js'; -import { TileSet } from '../tile.set.js'; import { TileSets } from '../tile.set.cache.js'; +import { TileSet } from '../tile.set.js'; import { TileSetLocal } from './tile.set.local.js'; -import { Context } from 'aws-lambda'; - -if (process.stdout.isTTY) LogConfig.setOutputStream(PrettyTransform.stream()); const xyz = { x: 0, y: 0, z: 0 }; const tileMatrix = Nztm2000Tms; diff --git a/packages/lambda-tiler/src/cli/serve.ts b/packages/lambda-tiler/src/cli/serve.ts index 51b4b4695..d3fd44511 100644 --- a/packages/lambda-tiler/src/cli/serve.ts +++ b/packages/lambda-tiler/src/cli/serve.ts @@ -4,7 +4,6 @@ import { HttpHeader, LambdaAlbRequest, LambdaHttpRequest } from '@linzjs/lambda' import { Context } from 'aws-lambda'; import express from 'express'; import path from 'path'; -import { PrettyTransform } from 'pretty-json-log'; import 'source-map-support/register.js'; import * as ulid from 'ulid'; import url from 'url'; @@ -19,8 +18,6 @@ const __dirname = path.dirname(url.fileURLToPath(import.meta.url)); const app = express(); const port = Env.getNumber('PORT', 5050); -if (process.stdout.isTTY) LogConfig.setOutputStream(PrettyTransform.stream()); - async function handleRequest( ctx: LambdaHttpRequest, res: express.Response, diff --git a/packages/server/bin/basemaps-server.js b/packages/server/bin/basemaps-server.js index 606ebf8c1..7191f8267 100755 --- a/packages/server/bin/basemaps-server.js +++ b/packages/server/bin/basemaps-server.js @@ -2,7 +2,8 @@ import { BasemapsServerCommand } from '../build/cli.js'; import Errors from '@oclif/errors/handle.js'; -BasemapsServerCommand.run().catch((error) => { + +BasemapsServerCommand.run(void 0, import.meta.url).catch((error) => { if (error.oclif) return Errors(error); console.log(error); }); diff --git a/packages/server/src/cli.ts b/packages/server/src/cli.ts index b5f06a6c2..0cc537821 100644 --- a/packages/server/src/cli.ts +++ b/packages/server/src/cli.ts @@ -1,13 +1,10 @@ -import { ConfigProviderMemory, parseRgba } from '@basemaps/config'; +// Configure the logging before importing everything +import { ConfigPrefix, ConfigProviderMemory, parseRgba } from '@basemaps/config'; import { Config, Env, LogConfig } from '@basemaps/shared'; import { fsa } from '@linzjs/s3fs'; import { Command, flags } from '@oclif/command'; -import { PrettyTransform } from 'pretty-json-log'; -import { ConfigPrefix } from '@basemaps/config'; import { BasemapsServer } from './server.js'; -if (process.stdout.isTTY) LogConfig.setOutputStream(PrettyTransform.stream()); - const logger = LogConfig.get(); export class BasemapsServerCommand extends Command { diff --git a/packages/server/src/server.ts b/packages/server/src/server.ts index a0b630075..d14577024 100644 --- a/packages/server/src/server.ts +++ b/packages/server/src/server.ts @@ -1,50 +1,40 @@ -import { HttpHeader, HttpHeaderRequestId, LambdaAlbRequest } from '@linzjs/lambda'; -import { handleRequest } from '@basemaps/lambda-tiler'; -import * as ulid from 'ulid'; +import { handler } from '@basemaps/lambda-tiler'; +import { ALBEvent, ALBResult, APIGatewayProxyResultV2, CloudFrontRequestResult, Context } from 'aws-lambda'; +import ulid from 'ulid'; import express from 'express'; -import { LogConfig } from '@basemaps/shared'; -import { Context } from 'aws-lambda'; +import { lf } from '@linzjs/lambda'; export const BasemapsServer = express(); -BasemapsServer.get('/v1/*', async (req: express.Request, res: express.Response) => { - const startTime = Date.now(); - const requestId = ulid.ulid(); - const logger = LogConfig.get().child({ id: requestId }); - const ctx = new LambdaAlbRequest( - { - httpMethod: 'get', - path: req.path, - queryStringParameters: req.query, - } as any, - {} as Context, - logger, - ); +function isAlbResult(r: ALBResult | CloudFrontRequestResult | APIGatewayProxyResultV2): r is ALBResult { + if (typeof r !== 'object') return false; + if (r == null) return false; + return 'statusCode' in r; +} + +const instanceId = ulid.ulid(); - if (ctx.header(HttpHeaderRequestId.RequestId) == null) { - ctx.headers.set(HttpHeaderRequestId.RequestId, 'c' + requestId); - } - const ifNoneMatch = req.header(HttpHeader.IfNoneMatch); - if (ifNoneMatch != null && !Array.isArray(ifNoneMatch)) { - ctx.headers.set(HttpHeader.IfNoneMatch.toLowerCase(), ifNoneMatch); - } +BasemapsServer.get('/v1/*', async (req: express.Request, res: express.Response) => { + const event: ALBEvent = { + httpMethod: 'GET', + requestContext: { elb: { targetGroupArn: 'arn:fake' } }, + path: req.path, + headers: req.headers as Record, + queryStringParameters: req.query as Record, + body: null, + isBase64Encoded: false, + }; + if (req.query['api'] == null) req.query['api'] = 'c' + instanceId; - try { - const data = await handleRequest(ctx); - res.status(data.status); - if (data.headers) { - for (const [header, value] of data.headers) { - res.header(header, String(value)); - } + handler(event, {} as Context, (err, r) => { + if (err || !isAlbResult(r)) { + lf.Logger.fatal({ err }, 'RequestFailed'); + return res.end(err); } - if (data.status < 299 && data.status > 199) res.end(data.body); - else res.end(); - const duration = Date.now() - startTime; - logger.info({ ...ctx.logContext, metrics: ctx.timer.metrics, status: data.status, duration }, 'Request:Done'); - } catch (e) { - logger.fatal({ ...ctx.logContext, err: e }, 'Request:Failed'); - res.status(500); + res.status(r.statusCode); + for (const [key, value] of Object.entries(r.headers ?? {})) res.header(key, String(value)); + if (r.body) res.send(Buffer.from(r.body, r.isBase64Encoded ? 'base64' : 'utf8')); res.end(); - } + }); }); diff --git a/packages/shared/src/log.ts b/packages/shared/src/log.ts index 47e170a7e..94fee623f 100644 --- a/packages/shared/src/log.ts +++ b/packages/shared/src/log.ts @@ -1,5 +1,5 @@ import pino from 'pino'; -import { Writable } from 'stream'; +import { PrettyTransform } from 'pretty-json-log'; export interface LogFunc { (msg: string): void; @@ -21,6 +21,8 @@ export interface LogType { fatal: LogFunc; child: (obj: Record) => LogType; } + +const defaultOpts = { level: 'debug' }; /** * Encapsulate the logger so that it can be swapped out */ @@ -28,7 +30,7 @@ export const LogConfig = { /** Get the currently configured logger */ get(): LogType { if (currentLog == null) { - currentLog = pino({ level: 'debug' }); + currentLog = process.stdout.isTTY ? pino(defaultOpts, PrettyTransform.stream()) : pino(defaultOpts); } return currentLog; }, @@ -37,15 +39,6 @@ export const LogConfig = { currentLog = log; }, - /** Overwrite the logger with a new logger that outputs to the provided stream*/ - setOutputStream(stream: Writable): void { - let level = 'debug'; - if (currentLog) { - level = currentLog.level; - } - currentLog = pino({ level }, stream); - }, - /** Disable the logger */ disable(): void { LogConfig.get().level = 'silent';