From 7befd294c14b77a378396ef1c6c84bdf60949118 Mon Sep 17 00:00:00 2001 From: Stephen Firrincieli Date: Fri, 11 Oct 2019 12:34:44 -0400 Subject: [PATCH 1/5] Add enhanced lambda metrics --- src/metrics/enhanced-lambda-metrics.ts | 16 +++++ src/utils/arn.spec.ts | 35 +++++++++++ src/utils/arn.ts | 20 +++++++ src/utils/cold-start.spec.ts | 22 +++++++ src/utils/cold-start.ts | 27 +++++++++ src/utils/handler.spec.ts | 82 ++++++++++++++++++++++++-- src/utils/handler.ts | 5 ++ 7 files changed, 201 insertions(+), 6 deletions(-) create mode 100644 src/metrics/enhanced-lambda-metrics.ts create mode 100644 src/utils/arn.spec.ts create mode 100644 src/utils/arn.ts create mode 100644 src/utils/cold-start.spec.ts create mode 100644 src/utils/cold-start.ts diff --git a/src/metrics/enhanced-lambda-metrics.ts b/src/metrics/enhanced-lambda-metrics.ts new file mode 100644 index 00000000..318401e9 --- /dev/null +++ b/src/metrics/enhanced-lambda-metrics.ts @@ -0,0 +1,16 @@ +import { sendDistributionMetric } from "../index"; + +import { parseTagsFromARN } from "../utils/arn"; +import { getColdStartTag } from "../utils/cold-start"; + +const ENHANCED_LAMBDA_METRICS_NAMESPACE = "aws.lambda.enhanced"; + +export function incrementInvocationsMetric(functionARN: string): void { + const tags = [...parseTagsFromARN(functionARN), getColdStartTag()]; + sendDistributionMetric(`${ENHANCED_LAMBDA_METRICS_NAMESPACE}.invocations`, 1, ...tags); +} + +export function incrementErrorsMetric(functionARN: string): void { + const tags = [...parseTagsFromARN(functionARN), getColdStartTag()]; + sendDistributionMetric(`${ENHANCED_LAMBDA_METRICS_NAMESPACE}.errors`, 1, ...tags); +} diff --git a/src/utils/arn.spec.ts b/src/utils/arn.spec.ts new file mode 100644 index 00000000..5cfd3847 --- /dev/null +++ b/src/utils/arn.spec.ts @@ -0,0 +1,35 @@ +import { parseLambdaARN, parseTagsFromARN } from "./arn"; + +describe("arn utils", () => { + it("parses arn properties", () => { + expect(parseLambdaARN("arn:aws:lambda:us-east-1:123497598159:function:my-test-lambda")).toEqual({ + account_id: "123497598159", + functionname: "my-test-lambda", + region: "us-east-1", + }); + }); + + it("parses arn properties with version alias", () => { + expect(parseLambdaARN("arn:aws:lambda:us-east-1:123497598159:function:my-test-lambda:my-version-alias")).toEqual({ + account_id: "123497598159", + functionname: "my-test-lambda", + region: "us-east-1", + }); + }); + + it("parses arn tags", () => { + const parsedTags = parseTagsFromARN("arn:aws:lambda:us-east-1:123497598159:function:my-test-lambda"); + for (const tag of ["account_id:123497598159", "functionname:my-test-lambda", "region:us-east-1"]) { + expect(parsedTags).toContain(tag); + } + }); + + it("parses arn tags with version", () => { + const parsedTags = parseTagsFromARN( + "arn:aws:lambda:us-east-1:123497598159:function:my-test-lambda:my-version-alias", + ); + for (const tag of ["account_id:123497598159", "functionname:my-test-lambda", "region:us-east-1"]) { + expect(parsedTags).toContain(tag); + } + }); +}); diff --git a/src/utils/arn.ts b/src/utils/arn.ts new file mode 100644 index 00000000..de82cd69 --- /dev/null +++ b/src/utils/arn.ts @@ -0,0 +1,20 @@ +/** Parse properties of the ARN into an object */ +export function parseLambdaARN(functionARN: string) { + // Disabling variable name because account_id is the key we need to use for the tag + // tslint:disable-next-line: variable-name + const [, , , region, account_id, , functionname] = functionARN.split(":", 7); + return { region, account_id, functionname }; +} + +/** + * Parse keyValueObject to get the array of key:value strings to use in Datadog metric submission + * @param obj The object whose properties and values we want to get key:value strings from + */ +function makeTagStringsFromObject(keyValueObject: { [key: string]: string }) { + return Object.entries(keyValueObject).map(([tagKey, tagValue]) => `${tagKey}:${tagValue}`); +} + +/** Get the array of "key:value" string tags from the Lambda ARN */ +export function parseTagsFromARN(functionARN: string) { + return makeTagStringsFromObject(parseLambdaARN(functionARN)); +} diff --git a/src/utils/cold-start.spec.ts b/src/utils/cold-start.spec.ts new file mode 100644 index 00000000..28d86496 --- /dev/null +++ b/src/utils/cold-start.spec.ts @@ -0,0 +1,22 @@ +import { _resetColdStart, didFunctionColdStart, setColdStart } from "./cold-start"; + +beforeEach(_resetColdStart); +afterAll(_resetColdStart); + +describe("cold-start", () => { + it("identifies cold starts on the first execution", () => { + setColdStart(); + expect(didFunctionColdStart()).toEqual(true); + }); + + it("identifies non-cold starts on subsequent executions", () => { + setColdStart(); + expect(didFunctionColdStart()).toEqual(true); + + setColdStart(); + expect(didFunctionColdStart()).toEqual(false); + + setColdStart(); + expect(didFunctionColdStart()).toEqual(false); + }); +}); diff --git a/src/utils/cold-start.ts b/src/utils/cold-start.ts new file mode 100644 index 00000000..ae929952 --- /dev/null +++ b/src/utils/cold-start.ts @@ -0,0 +1,27 @@ +let _didFunctionColdStart = true; + +let _isColdStartSet = false; + +/** + * Use global variables to determine whether the container cold started + * On the first container run, _isColdStartSet and _didFunctionColdStart are true + * For subsequent executions _isColdStartSet will be true and _didFunctionColdStart will be false + */ +export function setColdStart() { + _didFunctionColdStart = !_isColdStartSet; + _isColdStartSet = true; +} + +export function didFunctionColdStart() { + return _didFunctionColdStart; +} + +export function getColdStartTag() { + return `cold_start:${didFunctionColdStart()}`; +} + +// For testing, reset the globals to their original values +export function _resetColdStart() { + _didFunctionColdStart = true; + _isColdStartSet = false; +} diff --git a/src/utils/handler.spec.ts b/src/utils/handler.spec.ts index 8dd2321a..74c0f560 100644 --- a/src/utils/handler.spec.ts +++ b/src/utils/handler.spec.ts @@ -1,8 +1,23 @@ import { Context, Handler } from "aws-lambda"; +import { didFunctionColdStart } from "./cold-start"; import { wrap } from "./handler"; import { LogLevel, setLogLevel } from "./log"; +import { sendDistributionMetric } from "../index"; + +jest.mock("../index"); + +const mockedSendDistributionMetric = sendDistributionMetric as jest.Mock; + +afterEach(() => { + mockedSendDistributionMetric.mockClear(); +}); + +const mockContext = ({ + invokedFunctionArn: "arn:aws:lambda:us-east-1:123497598159:function:my-test-lambda", +} as any) as Context; + beforeEach(() => { setLogLevel(LogLevel.NONE); }); @@ -27,7 +42,7 @@ describe("wrap", () => { }, ); - const result = await wrappedHandler({}, {} as Context, () => { + const result = await wrappedHandler({}, mockContext, () => { calledOriginalHandler = true; }); expect(result).toEqual({ statusCode: 200, body: "The body of the response" }); @@ -56,7 +71,7 @@ describe("wrap", () => { }, ); - const result = await wrappedHandler({}, {} as Context, () => { + const result = await wrappedHandler({}, mockContext, () => { calledOriginalHandler = true; }); expect(result).toEqual({ statusCode: 200, body: "The body of the response" }); @@ -85,7 +100,7 @@ describe("wrap", () => { }, ); - const result = await wrappedHandler({}, {} as Context, () => { + const result = await wrappedHandler({}, mockContext, () => { calledOriginalHandler = true; }); expect(result).toEqual({ statusCode: 200, body: "The body of the response" }); @@ -113,7 +128,7 @@ describe("wrap", () => { }, ); - const result = await wrappedHandler({}, {} as Context, () => { + const result = await wrappedHandler({}, mockContext, () => { calledOriginalHandler = true; }); @@ -143,7 +158,7 @@ describe("wrap", () => { }, ); - const result = wrappedHandler({}, {} as Context, () => { + const result = wrappedHandler({}, mockContext, () => { calledOriginalHandler = true; }); @@ -173,7 +188,7 @@ describe("wrap", () => { }, ); - const result = wrappedHandler({}, {} as Context, () => { + const result = wrappedHandler({}, mockContext, () => { calledOriginalHandler = true; }); await expect(result).rejects.toEqual(Error("Some error")); @@ -182,4 +197,59 @@ describe("wrap", () => { expect(calledComplete).toBeTruthy(); expect(calledOriginalHandler).toBeFalsy(); }); + + it("increments invocations for each function call", async () => { + const handler: Handler = (event, context, callback) => { + callback(null, { statusCode: 200, body: "The body of the response" }); + }; + + const wrappedHandler = wrap(handler, () => {}, async () => {}); + + await wrappedHandler({}, mockContext, () => {}); + + expect(mockedSendDistributionMetric).toBeCalledTimes(1); + expect(mockedSendDistributionMetric).toBeCalledWith( + "aws.lambda.enhanced.invocations", + 1, + "region:us-east-1", + "account_id:123497598159", + "functionname:my-test-lambda", + "cold_start:true", + ); + + await wrappedHandler({}, mockContext, () => {}); + await wrappedHandler({}, mockContext, () => {}); + await wrappedHandler({}, mockContext, () => {}); + + expect(mockedSendDistributionMetric).toBeCalledTimes(4); + }); + + it("increments errors correctly", async () => { + const handler: Handler = (event, context, callback) => { + throw Error("Some error"); + }; + + const wrappedHandler = wrap(handler, () => {}, async () => {}); + + const result = wrappedHandler({}, mockContext, () => {}); + await expect(result).rejects.toEqual(Error("Some error")); + + expect(mockedSendDistributionMetric).toBeCalledTimes(2); + expect(mockedSendDistributionMetric).toBeCalledWith( + "aws.lambda.enhanced.invocations", + 1, + "region:us-east-1", + "account_id:123497598159", + "functionname:my-test-lambda", + "cold_start:true", + ); + expect(mockedSendDistributionMetric).toBeCalledWith( + "aws.lambda.enhanced.errors", + 1, + "region:us-east-1", + "account_id:123497598159", + "functionname:my-test-lambda", + "cold_start:true", + ); + }); }); diff --git a/src/utils/handler.ts b/src/utils/handler.ts index a9ba2cc8..1060e4a8 100644 --- a/src/utils/handler.ts +++ b/src/utils/handler.ts @@ -1,5 +1,6 @@ import { Callback, Context, Handler } from "aws-lambda"; +import { incrementErrorsMetric, incrementInvocationsMetric } from "../metrics/enhanced-lambda-metrics"; import { logError } from "./log"; export type OnWrapFunc any> = (fn: T) => T; @@ -18,6 +19,7 @@ export function wrap( return async (event: TEvent, context: Context) => { try { await onStart(event, context); + incrementInvocationsMetric(context.invokedFunctionArn); } catch (error) { // Swallow the error and continue processing. logError("Pre-lambda hook threw error", { innerError: error }); @@ -26,6 +28,9 @@ export function wrap( try { const wrappedHandler = onWrap !== undefined ? onWrap(promHandler) : promHandler; result = await wrappedHandler(event, context); + } catch (error) { + incrementErrorsMetric(context.invokedFunctionArn); + throw error; } finally { try { await onComplete(); From b704255a6e2147043a12f16ed8f36a7efc17816c Mon Sep 17 00:00:00 2001 From: Stephen Firrincieli Date: Tue, 15 Oct 2019 16:46:52 -0400 Subject: [PATCH 2/5] Implement PR feedback --- src/index.spec.ts | 53 ++++++++++++++- src/index.ts | 16 ++++- ...-lambda-metrics.ts => enhanced-metrics.ts} | 12 +++- src/metrics/index.ts | 1 + src/trace/listener.ts | 6 +- src/utils/cold-start.ts | 18 ++--- src/utils/handler.spec.ts | 65 ------------------- src/utils/handler.ts | 17 +++-- 8 files changed, 101 insertions(+), 87 deletions(-) rename src/metrics/{enhanced-lambda-metrics.ts => enhanced-metrics.ts} (68%) diff --git a/src/index.spec.ts b/src/index.spec.ts index e884ea50..f6b5c79c 100644 --- a/src/index.spec.ts +++ b/src/index.spec.ts @@ -1,9 +1,25 @@ import http from "http"; import nock from "nock"; +import { Context, Handler } from "aws-lambda"; import { datadog, getTraceHeaders, sendDistributionMetric, TraceHeaders } from "./index"; +import { incrementErrorsMetric, incrementInvocationsMetric } from "./metrics/enhanced-metrics"; +import { MetricsListener } from "./metrics/listener"; import { LogLevel, setLogLevel } from "./utils"; -import tracer, { Span } from "dd-trace"; + +jest.mock("./metrics/enhanced-metrics"); + +const mockedIncrementErrors = incrementErrorsMetric as jest.Mock; +const mockedIncrementInvocations = incrementInvocationsMetric as jest.Mock; + +const mockARN = "arn:aws:lambda:us-east-1:123497598159:function:my-test-lambda"; +const mockContext = ({ + invokedFunctionArn: mockARN, +} as any) as Context; + +// const MockedListener = OriginalListenerModule.MetricsListener as jest.Mocked< +// typeof OriginalListenerModule.MetricsListener +// >; describe("datadog", () => { let traceId: string | undefined; @@ -27,6 +43,9 @@ describe("datadog", () => { oldEnv = process.env; process.env = { ...oldEnv }; nock.cleanAll(); + + mockedIncrementErrors.mockClear(); + mockedIncrementInvocations.mockClear(); }); afterEach(() => { process.env = oldEnv; @@ -162,4 +181,36 @@ describe("datadog", () => { "x-datadog-trace-id": "123456", }); }); + + it("increments invocations for each function call", async () => { + const wrapped = datadog(handler); + + await wrapped({}, mockContext, () => {}); + + expect(mockedIncrementInvocations).toBeCalledTimes(1); + expect(mockedIncrementInvocations).toBeCalledWith(mockARN); + + await wrapped({}, mockContext, () => {}); + await wrapped({}, mockContext, () => {}); + await wrapped({}, mockContext, () => {}); + + expect(mockedIncrementInvocations).toBeCalledTimes(4); + }); + + it("increments errors correctly", async () => { + const handlerError: Handler = (event, context, callback) => { + throw Error("Some error"); + }; + + const wrappedHandler = datadog(handlerError); + + const result = wrappedHandler({}, mockContext, () => {}); + await expect(result).rejects.toEqual(Error("Some error")); + + expect(mockedIncrementInvocations).toBeCalledTimes(1); + expect(mockedIncrementErrors).toBeCalledTimes(1); + + expect(mockedIncrementInvocations).toBeCalledWith(mockARN); + expect(mockedIncrementErrors).toBeCalledWith(mockARN); + }); }); diff --git a/src/index.ts b/src/index.ts index 1c5fdd71..3a7750be 100644 --- a/src/index.ts +++ b/src/index.ts @@ -1,6 +1,12 @@ import { Handler } from "aws-lambda"; -import { KMSService, MetricsConfig, MetricsListener } from "./metrics"; +import { + incrementErrorsMetric, + incrementInvocationsMetric, + KMSService, + MetricsConfig, + MetricsListener, +} from "./metrics"; import { TraceConfig, TraceHeaders, TraceListener } from "./trace"; import { logError, LogLevel, setLogLevel, wrap } from "./utils"; @@ -72,8 +78,12 @@ export function datadog( for (const listener of listeners) { listener.onStartInvocation(event, context); } + incrementInvocationsMetric(context.invokedFunctionArn); }, - async () => { + async (event, context, error?) => { + if (error) { + incrementErrorsMetric(context.invokedFunctionArn); + } // Completion hook, (called once per handler invocation) for (const listener of listeners) { await listener.onCompleteInvocation(); @@ -146,7 +156,7 @@ function getConfig(userConfig?: Partial): Config { return config; } -function getEnvValue(key: string, defaultValue: string): string { +export function getEnvValue(key: string, defaultValue: string): string { const val = process.env[key]; return val !== undefined ? val : defaultValue; } diff --git a/src/metrics/enhanced-lambda-metrics.ts b/src/metrics/enhanced-metrics.ts similarity index 68% rename from src/metrics/enhanced-lambda-metrics.ts rename to src/metrics/enhanced-metrics.ts index 318401e9..9cf43403 100644 --- a/src/metrics/enhanced-lambda-metrics.ts +++ b/src/metrics/enhanced-metrics.ts @@ -1,16 +1,26 @@ -import { sendDistributionMetric } from "../index"; +import { getEnvValue, sendDistributionMetric } from "../index"; import { parseTagsFromARN } from "../utils/arn"; import { getColdStartTag } from "../utils/cold-start"; const ENHANCED_LAMBDA_METRICS_NAMESPACE = "aws.lambda.enhanced"; +function areEnhancedMetricsEnabled() { + return getEnvValue("DD_ENHANCED_METRICS", "false").toLowerCase() === "true"; +} + export function incrementInvocationsMetric(functionARN: string): void { + if (!areEnhancedMetricsEnabled()) { + return; + } const tags = [...parseTagsFromARN(functionARN), getColdStartTag()]; sendDistributionMetric(`${ENHANCED_LAMBDA_METRICS_NAMESPACE}.invocations`, 1, ...tags); } export function incrementErrorsMetric(functionARN: string): void { + if (!areEnhancedMetricsEnabled()) { + return; + } const tags = [...parseTagsFromARN(functionARN), getColdStartTag()]; sendDistributionMetric(`${ENHANCED_LAMBDA_METRICS_NAMESPACE}.errors`, 1, ...tags); } diff --git a/src/metrics/index.ts b/src/metrics/index.ts index 869d1264..14d3419d 100644 --- a/src/metrics/index.ts +++ b/src/metrics/index.ts @@ -1,2 +1,3 @@ export { MetricsConfig, MetricsListener } from "./listener"; export { KMSService } from "./kms-service"; +export { incrementErrorsMetric, incrementInvocationsMetric } from "./enhanced-metrics"; diff --git a/src/trace/listener.ts b/src/trace/listener.ts index c4ad1276..e79281f0 100644 --- a/src/trace/listener.ts +++ b/src/trace/listener.ts @@ -5,6 +5,8 @@ import { extractTraceContext } from "./context"; import { patchHttp, unpatchHttp } from "./patch-http"; import { TraceContextService } from "./trace-context-service"; +import { didFunctionColdStart } from "../utils/cold-start"; + export interface TraceConfig { /** * Whether to automatically patch all outgoing http requests with Datadog's hybrid tracing headers. @@ -16,7 +18,6 @@ export interface TraceConfig { export class TraceListener { private contextService = new TraceContextService(); private context?: Context; - private coldstart = true; public get currentTraceHeaders() { return this.contextService.currentTraceHeaders; @@ -37,7 +38,6 @@ export class TraceListener { if (this.config.autoPatchHTTP) { unpatchHttp(); } - this.coldstart = false; } public onWrap any>(func: T): T { @@ -46,7 +46,7 @@ export class TraceListener { const options: SpanOptions & TraceOptions = {}; if (this.context) { options.tags = { - cold_start: this.coldstart, + cold_start: didFunctionColdStart(), function_arn: this.context.invokedFunctionArn, request_id: this.context.awsRequestId, resource_names: this.context.functionName, diff --git a/src/utils/cold-start.ts b/src/utils/cold-start.ts index ae929952..d0896cc6 100644 --- a/src/utils/cold-start.ts +++ b/src/utils/cold-start.ts @@ -1,19 +1,19 @@ -let _didFunctionColdStart = true; +let functionDidColdStart = true; -let _isColdStartSet = false; +let isColdStartSet = false; /** * Use global variables to determine whether the container cold started - * On the first container run, _isColdStartSet and _didFunctionColdStart are true - * For subsequent executions _isColdStartSet will be true and _didFunctionColdStart will be false + * On the first container run, isColdStartSet and functionDidColdStart are true + * For subsequent executions isColdStartSet will be true and functionDidColdStart will be false */ export function setColdStart() { - _didFunctionColdStart = !_isColdStartSet; - _isColdStartSet = true; + functionDidColdStart = !isColdStartSet; + isColdStartSet = true; } export function didFunctionColdStart() { - return _didFunctionColdStart; + return functionDidColdStart; } export function getColdStartTag() { @@ -22,6 +22,6 @@ export function getColdStartTag() { // For testing, reset the globals to their original values export function _resetColdStart() { - _didFunctionColdStart = true; - _isColdStartSet = false; + functionDidColdStart = true; + isColdStartSet = false; } diff --git a/src/utils/handler.spec.ts b/src/utils/handler.spec.ts index 74c0f560..33af8ed5 100644 --- a/src/utils/handler.spec.ts +++ b/src/utils/handler.spec.ts @@ -4,16 +4,6 @@ import { didFunctionColdStart } from "./cold-start"; import { wrap } from "./handler"; import { LogLevel, setLogLevel } from "./log"; -import { sendDistributionMetric } from "../index"; - -jest.mock("../index"); - -const mockedSendDistributionMetric = sendDistributionMetric as jest.Mock; - -afterEach(() => { - mockedSendDistributionMetric.mockClear(); -}); - const mockContext = ({ invokedFunctionArn: "arn:aws:lambda:us-east-1:123497598159:function:my-test-lambda", } as any) as Context; @@ -197,59 +187,4 @@ describe("wrap", () => { expect(calledComplete).toBeTruthy(); expect(calledOriginalHandler).toBeFalsy(); }); - - it("increments invocations for each function call", async () => { - const handler: Handler = (event, context, callback) => { - callback(null, { statusCode: 200, body: "The body of the response" }); - }; - - const wrappedHandler = wrap(handler, () => {}, async () => {}); - - await wrappedHandler({}, mockContext, () => {}); - - expect(mockedSendDistributionMetric).toBeCalledTimes(1); - expect(mockedSendDistributionMetric).toBeCalledWith( - "aws.lambda.enhanced.invocations", - 1, - "region:us-east-1", - "account_id:123497598159", - "functionname:my-test-lambda", - "cold_start:true", - ); - - await wrappedHandler({}, mockContext, () => {}); - await wrappedHandler({}, mockContext, () => {}); - await wrappedHandler({}, mockContext, () => {}); - - expect(mockedSendDistributionMetric).toBeCalledTimes(4); - }); - - it("increments errors correctly", async () => { - const handler: Handler = (event, context, callback) => { - throw Error("Some error"); - }; - - const wrappedHandler = wrap(handler, () => {}, async () => {}); - - const result = wrappedHandler({}, mockContext, () => {}); - await expect(result).rejects.toEqual(Error("Some error")); - - expect(mockedSendDistributionMetric).toBeCalledTimes(2); - expect(mockedSendDistributionMetric).toBeCalledWith( - "aws.lambda.enhanced.invocations", - 1, - "region:us-east-1", - "account_id:123497598159", - "functionname:my-test-lambda", - "cold_start:true", - ); - expect(mockedSendDistributionMetric).toBeCalledWith( - "aws.lambda.enhanced.errors", - 1, - "region:us-east-1", - "account_id:123497598159", - "functionname:my-test-lambda", - "cold_start:true", - ); - }); }); diff --git a/src/utils/handler.ts b/src/utils/handler.ts index 1060e4a8..e8d2604c 100644 --- a/src/utils/handler.ts +++ b/src/utils/handler.ts @@ -1,6 +1,5 @@ import { Callback, Context, Handler } from "aws-lambda"; -import { incrementErrorsMetric, incrementInvocationsMetric } from "../metrics/enhanced-lambda-metrics"; import { logError } from "./log"; export type OnWrapFunc any> = (fn: T) => T; @@ -11,7 +10,7 @@ export type OnWrapFunc any> = (fn: T) => T; export function wrap( handler: Handler, onStart: (event: TEvent, context: Context) => void, - onComplete: () => Promise, + onComplete: (event: TEvent, context: Context, error?: Error) => Promise, onWrap?: OnWrapFunc, ): Handler { const promHandler = promisifiedHandler(handler); @@ -19,21 +18,29 @@ export function wrap( return async (event: TEvent, context: Context) => { try { await onStart(event, context); - incrementInvocationsMetric(context.invokedFunctionArn); } catch (error) { // Swallow the error and continue processing. logError("Pre-lambda hook threw error", { innerError: error }); } let result: TResult; + // Need to disable linter rule to explicitly assign the variable, otherwise TS + // won't reccognize that the var may be assigned in the catch block + // tslint:disable-next-line: no-unnecessary-initializer + let handlerError: Error | undefined = undefined; + try { const wrappedHandler = onWrap !== undefined ? onWrap(promHandler) : promHandler; result = await wrappedHandler(event, context); } catch (error) { - incrementErrorsMetric(context.invokedFunctionArn); + handlerError = error; throw error; } finally { try { - await onComplete(); + if (handlerError) { + await onComplete(event, context, handlerError); + } else { + await onComplete(event, context); + } } catch (error) { // Swallow the error and continue processing. logError("Post-lambda hook threw error", { innerError: error }); From a171852cd5aceb606ec4739aaae25a4a4aaccbac Mon Sep 17 00:00:00 2001 From: Stephen Firrincieli Date: Thu, 17 Oct 2019 15:39:20 -0400 Subject: [PATCH 3/5] Allow enabling enhanced metrics with a config property --- src/index.spec.ts | 38 +++++++++++++++++++++++++++++++-- src/index.ts | 12 +++++++++-- src/metrics/enhanced-metrics.ts | 10 --------- src/metrics/listener.ts | 6 ++++++ 4 files changed, 52 insertions(+), 14 deletions(-) diff --git a/src/index.spec.ts b/src/index.spec.ts index f6b5c79c..8e6f87c4 100644 --- a/src/index.spec.ts +++ b/src/index.spec.ts @@ -182,7 +182,8 @@ describe("datadog", () => { }); }); - it("increments invocations for each function call", async () => { + it("increments invocations for each function call with env var", async () => { + process.env.DD_ENHANCED_METRICS = "true"; const wrapped = datadog(handler); await wrapped({}, mockContext, () => {}); @@ -197,7 +198,9 @@ describe("datadog", () => { expect(mockedIncrementInvocations).toBeCalledTimes(4); }); - it("increments errors correctly", async () => { + it("increments errors correctly with env var", async () => { + process.env.DD_ENHANCED_METRICS = "true"; + const handlerError: Handler = (event, context, callback) => { throw Error("Some error"); }; @@ -213,4 +216,35 @@ describe("datadog", () => { expect(mockedIncrementInvocations).toBeCalledWith(mockARN); expect(mockedIncrementErrors).toBeCalledWith(mockARN); }); + + it("increments errors and invocations with config setting", async () => { + const handlerError: Handler = (event, context, callback) => { + throw Error("Some error"); + }; + + const wrappedHandler = datadog(handlerError, { enhancedMetrics: true }); + + const result = wrappedHandler({}, mockContext, () => {}); + await expect(result).rejects.toEqual(Error("Some error")); + + expect(mockedIncrementInvocations).toBeCalledTimes(1); + expect(mockedIncrementErrors).toBeCalledTimes(1); + + expect(mockedIncrementInvocations).toBeCalledWith(mockARN); + expect(mockedIncrementErrors).toBeCalledWith(mockARN); + }); + + it("doesn't increment enhanced metrics without env var or config", async () => { + const handlerError: Handler = (event, context, callback) => { + throw Error("Some error"); + }; + + const wrappedHandler = datadog(handlerError); + + const result = wrappedHandler({}, mockContext, () => {}); + await expect(result).rejects.toEqual(Error("Some error")); + + expect(mockedIncrementInvocations).toBeCalledTimes(0); + expect(mockedIncrementErrors).toBeCalledTimes(0); + }); }); diff --git a/src/index.ts b/src/index.ts index 3a7750be..d68bef24 100644 --- a/src/index.ts +++ b/src/index.ts @@ -17,6 +17,7 @@ const apiKeyKMSEnvVar = "DD_KMS_API_KEY"; const siteURLEnvVar = "DD_SITE"; const logLevelEnvVar = "DD_LOG_LEVEL"; const logForwardingEnvVar = "DD_FLUSH_TO_LOG"; +const enhancedMetricsEnvVar = "DD_ENHANCED_METRICS"; const defaultSiteURL = "datadoghq.com"; @@ -38,6 +39,7 @@ export const defaultConfig: Config = { apiKeyKMS: "", autoPatchHTTP: true, debugLogging: false, + enhancedMetrics: false, logForwarding: false, shouldRetryMetrics: false, siteURL: "", @@ -78,10 +80,12 @@ export function datadog( for (const listener of listeners) { listener.onStartInvocation(event, context); } - incrementInvocationsMetric(context.invokedFunctionArn); + if (finalConfig.enhancedMetrics) { + incrementInvocationsMetric(context.invokedFunctionArn); + } }, async (event, context, error?) => { - if (error) { + if (finalConfig.enhancedMetrics && error) { incrementErrorsMetric(context.invokedFunctionArn); } // Completion hook, (called once per handler invocation) @@ -152,6 +156,10 @@ function getConfig(userConfig?: Partial): Config { const result = getEnvValue(logForwardingEnvVar, "false").toLowerCase(); config.logForwarding = result === "true"; } + if (userConfig === undefined || userConfig.enhancedMetrics === undefined) { + const result = getEnvValue(enhancedMetricsEnvVar, "false").toLowerCase(); + config.enhancedMetrics = result === "true"; + } return config; } diff --git a/src/metrics/enhanced-metrics.ts b/src/metrics/enhanced-metrics.ts index 9cf43403..c1dcdbfa 100644 --- a/src/metrics/enhanced-metrics.ts +++ b/src/metrics/enhanced-metrics.ts @@ -5,22 +5,12 @@ import { getColdStartTag } from "../utils/cold-start"; const ENHANCED_LAMBDA_METRICS_NAMESPACE = "aws.lambda.enhanced"; -function areEnhancedMetricsEnabled() { - return getEnvValue("DD_ENHANCED_METRICS", "false").toLowerCase() === "true"; -} - export function incrementInvocationsMetric(functionARN: string): void { - if (!areEnhancedMetricsEnabled()) { - return; - } const tags = [...parseTagsFromARN(functionARN), getColdStartTag()]; sendDistributionMetric(`${ENHANCED_LAMBDA_METRICS_NAMESPACE}.invocations`, 1, ...tags); } export function incrementErrorsMetric(functionARN: string): void { - if (!areEnhancedMetricsEnabled()) { - return; - } const tags = [...parseTagsFromARN(functionARN), getColdStartTag()]; sendDistributionMetric(`${ENHANCED_LAMBDA_METRICS_NAMESPACE}.errors`, 1, ...tags); } diff --git a/src/metrics/listener.ts b/src/metrics/listener.ts index 6fe28a62..858ba6cb 100644 --- a/src/metrics/listener.ts +++ b/src/metrics/listener.ts @@ -38,6 +38,12 @@ export interface MetricsConfig { * @default false */ logForwarding: boolean; + + /** + * Whether to increment invocations and errors Lambda integration metrics from this layer. + * @default false + */ + enhancedMetrics: boolean; } export class MetricsListener { From 87f3eaee3cc2c67776bab1285026828e3c99cae0 Mon Sep 17 00:00:00 2001 From: Stephen Firrincieli Date: Thu, 24 Oct 2019 14:44:37 -0400 Subject: [PATCH 4/5] Add enhancedMetrics to config in tests --- src/metrics/listener.spec.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/metrics/listener.spec.ts b/src/metrics/listener.spec.ts index 25df6fd3..9dc1abb0 100644 --- a/src/metrics/listener.spec.ts +++ b/src/metrics/listener.spec.ts @@ -26,6 +26,7 @@ describe("MetricsListener", () => { const listener = new MetricsListener(kms as any, { apiKey: "api-key", apiKeyKMS: "kms-api-key-encrypted", + enhancedMetrics: false, logForwarding: false, shouldRetryMetrics: false, siteURL, @@ -46,6 +47,7 @@ describe("MetricsListener", () => { const listener = new MetricsListener(kms as any, { apiKey: "", apiKeyKMS: "kms-api-key-encrypted", + enhancedMetrics: false, logForwarding: false, shouldRetryMetrics: false, siteURL, @@ -62,6 +64,7 @@ describe("MetricsListener", () => { const listener = new MetricsListener(kms as any, { apiKey: "", apiKeyKMS: "kms-api-key-encrypted", + enhancedMetrics: false, logForwarding: false, shouldRetryMetrics: false, siteURL, From edd74660916f7951e8f5d46891e7bc0ca2947965 Mon Sep 17 00:00:00 2001 From: Stephen Firrincieli Date: Thu, 24 Oct 2019 14:52:31 -0400 Subject: [PATCH 5/5] Update readme, bump version to 0.6.0 --- README.md | 4 ++++ package.json | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 8cf34486..b628f265 100644 --- a/README.md +++ b/README.md @@ -79,6 +79,10 @@ How much logging datadog-lambda-layer-js should do. Set this to "debug" for exte If you have the Datadog Lambda Log forwarder enabled and are sending custom metrics, set this to true so your metrics will be sent via logs, (instead of being sent at the end of your lambda invocation). +### DD_ENHANCED_METRICS + +If you set the value of this variable to "true" then the Lambda layer will increment a Lambda integration metric called `aws.lambda.enhanced.invocations` with each invocation and `aws.lambda.enhanced.errors` if the invocation results in an error. These metrics are tagged with the function name, region, and account, as well as `cold_start:true|false`. + ## Usage Datadog needs to be able to read headers from the incoming Lambda event. diff --git a/package.json b/package.json index a90e18d1..d7d6239a 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "datadog-lambda-js", - "version": "0.5.0", + "version": "0.6.0", "description": "Lambda client library that supports hybrid tracing in node js", "main": "dist/index.js", "types": "dist/index.d.ts",