feat: Rename: Datadog -> DatadogLambda #285
Conversation
![datadog-datadog-prod-us1[bot]](https://avatars.githubusercontent.com/u/365230?s=60&v=4){kind=small}
| props: DatadogProps; | ||
| transport: Transport; | ||
| constructor(scope: Construct, id: string, props: DatadogProps) { | ||
| if (process.env.DD_CONSTRUCT_DEBUG_LOGS?.toLowerCase() == "true") { |
There was a problem hiding this comment.
⚪ Code Quality Violation
| if (process.env.DD_CONSTRUCT_DEBUG_LOGS?.toLowerCase() == "true") { | |
| if (process.env.DD_CONSTRUCT_DEBUG_LOGS?.toLowerCase() === "true") { |
Expected '===' and instead saw '=='. (...read more)
In JavaScript, == and != comparisons do type coercion, which can be confusing and may introduce potential errors. Use the type-safe equality operators === and !== instead.
| public addGitCommitMetadata( | ||
| lambdaFunctions: LambdaFunction[], | ||
| // eslint-disable-next-line @typescript-eslint/ban-ts-comment | ||
| // @ts-ignore |
There was a problem hiding this comment.
⚪ Code Quality Violation
Disallow `// tslint:` comments (...read more)
Correct your types instead of disabling TypeScript.
| public addGitCommitMetadata( | ||
| lambdaFunctions: LambdaFunction[], | ||
| // eslint-disable-next-line @typescript-eslint/ban-ts-comment | ||
| // @ts-ignore | ||
| gitCommitSha?: string, | ||
| // eslint-disable-next-line @typescript-eslint/ban-ts-comment | ||
| // @ts-ignore | ||
| gitRepoUrl?: string, | ||
| ): void { |
There was a problem hiding this comment.
⚪ Code Quality Violation
Too many parameters (7). Maximum allowed is 4. (...read more)
Having too many parameters can make your code hard to read. The parameters must be used in appropriate order. Forgetting the order of parameters can cause mistakes.
Too many parameters is a code smell. You should refactor your code in smaller reusable bits. While it may be valid to require more than four parameters, you should use object destructuring.
| } from "./index"; | ||
| import { LambdaFunction } from "./interfaces"; | ||
|
|
||
| const versionJson = require("../version.json"); |
There was a problem hiding this comment.
🟠 Code Quality Violation
Require statement not part of import statement. (...read more)
Use ESM instead of CommonJS imports.
| console.log( | ||
| "Instrumenting Lambda Functions in TypeScript stack with Datadog" | ||
| ); | ||
| console.log("Instrumenting Lambda Functions in TypeScript stack with Datadog"); |
There was a problem hiding this comment.
🟠 Code Quality Violation
Unexpected console statement. (...read more)
Debugging with console is not considered a bad practice, but it's easy to forget about console statements and leave them in production code. There is no need to pollute production builds with debugging statements.
| props: DatadogProps; | ||
| transport: Transport; | ||
| constructor(scope: Construct, id: string, props: DatadogProps) { | ||
| if (process.env.DD_CONSTRUCT_DEBUG_LOGS?.toLowerCase() == "true") { |
There was a problem hiding this comment.
⚪ Code Quality Violation
| if (process.env.DD_CONSTRUCT_DEBUG_LOGS?.toLowerCase() == "true") { | |
| if (process.env.DD_CONSTRUCT_DEBUG_LOGS?.toLowerCase() === "true") { |
Expected '===' and instead saw '=='. (...read more)
In JavaScript, == and != comparisons do type coercion, which can be confusing and may introduce potential errors. Use the type-safe equality operators === and !== instead.
| public addGitCommitMetadata( | ||
| lambdaFunctions: LambdaFunction[], | ||
| // eslint-disable-next-line @typescript-eslint/ban-ts-comment | ||
| // @ts-ignore |
There was a problem hiding this comment.
⚪ Code Quality Violation
Disallow `// tslint:` comments (...read more)
Correct your types instead of disabling TypeScript.
| public addGitCommitMetadata( | ||
| lambdaFunctions: LambdaFunction[], | ||
| // eslint-disable-next-line @typescript-eslint/ban-ts-comment | ||
| // @ts-ignore | ||
| gitCommitSha?: string, | ||
| // eslint-disable-next-line @typescript-eslint/ban-ts-comment | ||
| // @ts-ignore | ||
| gitRepoUrl?: string, | ||
| ): void { |
There was a problem hiding this comment.
⚪ Code Quality Violation
Too many parameters (7). Maximum allowed is 4. (...read more)
Having too many parameters can make your code hard to read. The parameters must be used in appropriate order. Forgetting the order of parameters can cause mistakes.
Too many parameters is a code smell. You should refactor your code in smaller reusable bits. While it may be valid to require more than four parameters, you should use object destructuring.
| } from "./index"; | ||
| import { LambdaFunction } from "./interfaces"; | ||
|
|
||
| const versionJson = require("../version.json"); |
There was a problem hiding this comment.
🟠 Code Quality Violation
Require statement not part of import statement. (...read more)
Use ESM instead of CommonJS imports.
| console.log( | ||
| "Instrumenting Lambda Functions in TypeScript stack with Datadog" | ||
| ); | ||
| console.log("Instrumenting Lambda Functions in TypeScript stack with Datadog"); |
There was a problem hiding this comment.
🟠 Code Quality Violation
Unexpected console statement. (...read more)
Debugging with console is not considered a bad practice, but it's easy to forget about console statements and leave them in production code. There is no need to pollute production builds with debugging statements.
lym953
force-pushed
the
yiming.luo/step-function-refactor
branch
2 times, most recently
from
aa63b23
to
de6d7e4
Compare
lym953
force-pushed
the
yiming.luo/auto-format
branch
3 times, most recently
from
983592b
to
46a3573
Compare
lym953
force-pushed
the
yiming.luo/step-function-refactor
branch
from
de6d7e4
to
2208e0e
Compare
| // @ts-ignore | ||
| gitCommitSha?: string, | ||
| // eslint-disable-next-line @typescript-eslint/ban-ts-comment | ||
| // @ts-ignore |
There was a problem hiding this comment.
⚪ Code Quality Violation
Disallow `// tslint:` comments (...read more)
Correct your types instead of disabling TypeScript.
lym953
force-pushed
the
yiming.luo/step-function-refactor
branch
from
2208e0e
to
635722f
Compare
| new LambdaRestApi(this, "rest-test", { | ||
| handler: lambdaFunction, | ||
| deployOptions: { | ||
| accessLogDestination: new LogGroupLogDestination(restLogGroup), | ||
| }, | ||
| }); |
There was a problem hiding this comment.
⚪ Code Quality Violation
Do not use 'new' for side effects. (...read more)
A lonely instance is almost always useless. Do not create objects without assigning them to a variable that you will use later.
| import * as lambda from "aws-cdk-lib/aws-lambda"; | ||
| import { LogGroup } from "aws-cdk-lib/aws-logs"; | ||
| import { addCdkConstructVersionTag, checkForMultipleApiKeys, DatadogLambda, DD_HANDLER_ENV_VAR } from "../src/index"; | ||
| const { ISecret } = require("aws-cdk-lib/aws-secretsmanager"); |
There was a problem hiding this comment.
🟠 Code Quality Violation
Require statement not part of import statement. (...read more)
Use ESM instead of CommonJS imports.
| const app = new App(); | ||
| const env = { account: "601427279990", region: "sa-east-1" }; | ||
| const stack = new ExampleStack(app, "lambda-function-stack", { env: env }); | ||
| console.log("Stack name: " + stack.stackName); |
There was a problem hiding this comment.
🟠 Code Quality Violation
Unexpected console statement. (...read more)
Debugging with console is not considered a bad practice, but it's easy to forget about console statements and leave them in production code. There is no need to pollute production builds with debugging statements.
| import { LogGroup } from "aws-cdk-lib/aws-logs"; | ||
| import { addCdkConstructVersionTag, checkForMultipleApiKeys, DatadogLambda, DD_HANDLER_ENV_VAR } from "../src/index"; | ||
| const { ISecret } = require("aws-cdk-lib/aws-secretsmanager"); | ||
| const versionJson = require("../version.json"); |
There was a problem hiding this comment.
🟠 Code Quality Violation
Require statement not part of import statement. (...read more)
Use ESM instead of CommonJS imports.
There was a problem hiding this comment.
These are just comments on existing code. Let me address them in a separate PR.
lym953
force-pushed
the
yiming.luo/step-function-refactor
branch
from
635722f
to
3646d05
Compare
lym953
force-pushed
the
yiming.luo/step-function-refactor
branch
from
3646d05
to
a61fe64
Compare
| @@ -0,0 +1,52 @@ | |||
| /* | |||
There was a problem hiding this comment.
Copied and modified from integration_tests/stacks/lambda-function-stack.ts
| @@ -0,0 +1,365 @@ | |||
| /* | |||
There was a problem hiding this comment.
This file is renamed from datadog.ts
| @@ -1,537 +1,11 @@ | |||
| import { App, Stack, Token } from "aws-cdk-lib"; | |||
| import { Template } from "aws-cdk-lib/assertions"; | |||
| import { App, Stack } from "aws-cdk-lib"; | |||
There was a problem hiding this comment.
This file was first renamed as datadog-lambda.spec.ts. Then datadog.spec.ts was created again with one test, to ensure the Datadog API still works.
|
/merge |
|
🚂 MergeQueue: pull request added to the queue The median merge time in Use |
dd-devflow
bot
added
mergequeue-status: queued
mergequeue-status: in_progress
and removed
mergequeue-status: queued
labels
dd-devflow
bot
added
mergequeue-status: done
and removed
mergequeue-status: in_progress
labels
|
Just an FYI, this is a breaking change. Packages that depend on datadog-cdk-constructs will now fail, with errors similar to (in Python, for example) |
What does this PR do?
Datadogclass asDatadogLambda, including:datadog.tsasdatadog-lambda.tsdatadog.spec.tsasdatadog-lambda.spec.tsDatadoguseDatadogLambdaDatadogclass which just wraps (extends) the newDatadogLambdaclass, so its behavior will be the same as the oldDatadogclass. Let me know if you think I should wrap it in a different way.lambda-function-stack-legacy-datadog-api.ts, which usesDatadoginstead ofDatadogLambda, showing how to use the legacyDatadogAPIdatadog.spec.tsto ensure theDatadogAPI still worksMotivation
To make it cleaner to add
DatadogStepFunctionclass later. Details in [RFC] Changing API for Datadog CDK ConstructTesting Guidelines
Automated testing
datadog.spec.tsManual testing
1. using
DatadogLambdaAPISteps:
examples/typescript-stack/lib/cdk-typescript-stack.ts, replace theCdkTypeScriptStackclass with theExampleStackclass inintegration_tests/stacks/lambda-function-stack.ts. I can't use the existingCdkTypeScriptStackclass due to some docker permission issue.ExampleStackclass usesDatadogLambdaAPIResult:
2. using
DatadogAPIStep:
ExampleStackclass useDatadogclass instead ofDatadogLambda. Deploy again.Result:
Additional Notes
Next steps:
README.md, recommending users to useDatadogLambdainstead ofDatadogTypes of Changes
Check all that apply