diff --git a/src/commands/stepfunctions/__tests__/helpers.test.ts b/src/commands/stepfunctions/__tests__/helpers.test.ts index 90d19bea7..1d0b05024 100644 --- a/src/commands/stepfunctions/__tests__/helpers.test.ts +++ b/src/commands/stepfunctions/__tests__/helpers.test.ts @@ -1,4 +1,5 @@ import {DescribeStateMachineCommandOutput, LogLevel} from '@aws-sdk/client-sfn' +import {BaseContext} from 'clipanion' import { buildArn, @@ -16,7 +17,18 @@ import { import {describeStateMachineFixture} from './fixtures/aws-resources' +const createMockContext = (): BaseContext => { + return { + stdout: { + write: (input: string) => { + return true + }, + }, + } as BaseContext +} + describe('stepfunctions command helpers tests', () => { + const context = createMockContext() describe('shouldUpdateStepForTracesMerging test', () => { test('already has JsonMerge added to payload field', () => { const step: StepType = { @@ -28,7 +40,7 @@ describe('stepfunctions command helpers tests', () => { }, End: true, } - expect(shouldUpdateStepForTracesMerging(step)).toBeFalsy() + expect(shouldUpdateStepForTracesMerging(step, context, 'Lambda Invoke')).toBeFalsy() }) test('no payload field', () => { @@ -40,7 +52,7 @@ describe('stepfunctions command helpers tests', () => { }, End: true, } - expect(shouldUpdateStepForTracesMerging(step)).toBeTruthy() + expect(shouldUpdateStepForTracesMerging(step, context, 'Lambda Invoke')).toBeTruthy() }) test('default payload field of $', () => { @@ -53,7 +65,7 @@ describe('stepfunctions command helpers tests', () => { }, End: true, } - expect(shouldUpdateStepForTracesMerging(step)).toBeTruthy() + expect(shouldUpdateStepForTracesMerging(step, context, 'Lambda Invoke')).toBeTruthy() }) test('none-lambda step should not be updated', () => { @@ -65,7 +77,7 @@ describe('stepfunctions command helpers tests', () => { }, End: true, } - expect(shouldUpdateStepForTracesMerging(step)).toBeFalsy() + expect(shouldUpdateStepForTracesMerging(step, context, 'DynamoDB Update')).toBeFalsy() }) test('legacy lambda api should not be updated', () => { @@ -74,7 +86,7 @@ describe('stepfunctions command helpers tests', () => { Resource: 'arn:aws:lambda:sa-east-1:601427271234:function:hello-function', End: true, } - expect(shouldUpdateStepForTracesMerging(step)).toBeFalsy() + expect(shouldUpdateStepForTracesMerging(step, context, 'Legacy Lambda')).toBeFalsy() }) }) @@ -185,7 +197,9 @@ describe('stepfunctions command helpers tests', () => { }, End: true, } - expect(shouldUpdateStepForStepFunctionContextInjection(step)).toBeTruthy() + expect( + shouldUpdateStepForStepFunctionContextInjection(step, context, 'Step Functions StartExecution') + ).toBeTruthy() }) test('is true for undefined', () => { @@ -197,7 +211,9 @@ describe('stepfunctions command helpers tests', () => { }, End: true, } - expect(shouldUpdateStepForStepFunctionContextInjection(step)).toBeTruthy() + expect( + shouldUpdateStepForStepFunctionContextInjection(step, context, 'Step Functions StartExecution') + ).toBeTruthy() }) test('is false when Input is an object that contains a CONTEXT key', () => { @@ -210,7 +226,9 @@ describe('stepfunctions command helpers tests', () => { }, End: true, } - expect(shouldUpdateStepForStepFunctionContextInjection(step)).toBeFalsy() + expect( + shouldUpdateStepForStepFunctionContextInjection(step, context, 'Step Functions StartExecution') + ).toBeFalsy() }) }) @@ -226,7 +244,7 @@ describe('stepfunctions command helpers tests', () => { End: true, } - const changed = injectContextForStepFunctions(step) + const changed = injectContextForStepFunctions(step, context, 'Step Functions StartExecution') expect(changed).toBeTruthy() expect(step.Parameters?.Input).toEqual({'CONTEXT.$': 'States.JsonMerge($$, $, false)'}) }) diff --git a/src/commands/stepfunctions/helpers.ts b/src/commands/stepfunctions/helpers.ts index 9e4158ded..5da8aba5c 100644 --- a/src/commands/stepfunctions/helpers.ts +++ b/src/commands/stepfunctions/helpers.ts @@ -95,7 +95,7 @@ export const injectContextIntoTasks = async ( if (definitionObj.States.hasOwnProperty(stepName)) { const step = definitionObj.States[stepName] const lambdaUpdated = injectContextForLambdaFunctions(step, context, stepName) - const stepUpdated = injectContextForStepFunctions(step) + const stepUpdated = injectContextForStepFunctions(step, context, stepName) definitionHasBeenUpdated = lambdaUpdated || stepUpdated } } @@ -125,20 +125,35 @@ export const addTraceContextToStepFunctionParameters = ({Parameters}: StepType): } } -export const shouldUpdateStepForTracesMerging = (step: StepType): boolean => { +export const shouldUpdateStepForTracesMerging = (step: StepType, context: BaseContext, stepName: string): boolean => { // is default lambda api if (step.Resource === 'arn:aws:states:::lambda:invoke') { if (!step.Parameters) { + context.stdout + .write(`[Warn] Step ${stepName} does not have a Parameters field. Step Functions Context Object injection \ +skipped. Your Step Functions trace will not be merged with downstream Lambda traces. To manually merge these traces, \ +check out https://docs.datadoghq.com/serverless/step_functions/troubleshooting/\n`) + return false } + // payload field not set if (!step.Parameters.hasOwnProperty('Payload.$')) { return true } + // default payload if (step.Parameters['Payload.$'] === '$') { return true } + + // custom payload + context.stdout + .write(`[Warn] Step ${stepName} has a custom Payload field. Step Functions Context Object injection skipped. \ +Your Step Functions trace will not be merged with downstream Lambda traces. To manually merge these traces, \ +check out https://docs.datadoghq.com/serverless/step_functions/troubleshooting/\n`) + + return false } return false @@ -152,21 +167,45 @@ export const shouldUpdateStepForTracesMerging = (step: StepType): boolean => { // not object | false // object without CONTEXT.$ | true // object with CONTEXT.$ | false -export const shouldUpdateStepForStepFunctionContextInjection = (step: StepType): boolean => { +export const shouldUpdateStepForStepFunctionContextInjection = ( + step: StepType, + context: BaseContext, + stepName: string +): boolean => { // is default lambda api if (step.Resource?.startsWith('arn:aws:states:::states:startExecution')) { if (!step.Parameters) { + context.stdout + .write(`[Warn] Step ${stepName} does not have a Parameters field. Step Functions Context Object injection \ +skipped. Your Step Functions trace will not be merged with downstream Step Function traces. To manually merge these \ +traces, check out https://docs.datadoghq.com/serverless/step_functions/troubleshooting/\n`) + return false } + if (!step.Parameters.Input) { return true } + if (typeof step.Parameters.Input !== 'object') { + context.stdout + .write(`[Warn] Step ${stepName}'s Parameters.Input field is not a JSON object. Step Functions Context Object \ +injection skipped. Your Step Functions trace will not be merged with downstream Step Function traces. To manually \ +merge these traces, check out https://docs.datadoghq.com/serverless/step_functions/troubleshooting/\n`) + return false } + if (!step.Parameters.Input['CONTEXT.$']) { return true } + + context.stdout + .write(`[Warn] Step ${stepName}'s Parameters.Input field has a custom CONTEXT field. Step Functions Context \ +Object injection skipped. Your Step Functions trace will not be merged with downstream Step Function traces. To \ +manually merge these traces, check out https://docs.datadoghq.com/serverless/step_functions/troubleshooting/\n`) + + return false } return false @@ -198,7 +237,7 @@ export type ParametersType = { } const injectContextForLambdaFunctions = (step: StepType, context: BaseContext, stepName: string): boolean => { - if (shouldUpdateStepForTracesMerging(step)) { + if (shouldUpdateStepForTracesMerging(step, context, stepName)) { addTraceContextToLambdaParameters(step) return true @@ -213,8 +252,8 @@ const injectContextForLambdaFunctions = (step: StepType, context: BaseContext, s return false } -export const injectContextForStepFunctions = (step: StepType): boolean => { - if (shouldUpdateStepForStepFunctionContextInjection(step)) { +export const injectContextForStepFunctions = (step: StepType, context: BaseContext, stepName: string): boolean => { + if (shouldUpdateStepForStepFunctionContextInjection(step, context, stepName)) { addTraceContextToStepFunctionParameters(step) return true