Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: Refactor example typescript stack by extracting a base class #300

Merged
merged 1 commit into from
Sep 27, 2024

Conversation

lym953
Copy link
Contributor

@lym953 lym953 commented Sep 24, 2024

What does this PR do?

Right now

class CdkTypeScriptStack extends Stack

This PR adds a class CdkTypeScriptStackBase, and makes:

class CdkTypeScriptStackBase extends Stack

and

class CdkTypeScriptStack extends CdkTypeScriptStackBase

CdkTypeScriptStackBase contains the setup of AWS resources, not related to Datadog.
CdkTypeScriptStack contains Datadog instrumentation code.

Motivation

I'm going to rename the interfaces like I did in #288. To make sure both the new API and old API work, I will need to to write two stacks for testing the two APIs. The two stacks will share the non-Datadog setup code, so I'm putting this part in the base class.

Testing Guidelines

cdk deploy successfully deploys the TypeScript stack.

Additional Notes

Types of Changes

  • Bug fix
  • New feature
  • Breaking change
  • Misc (docs, refactoring, dependency upgrade, etc.)

Check all that apply

  • This PR's description is comprehensive
  • This PR contains breaking changes that are documented in the description
  • This PR introduces new APIs or parameters that are documented and unlikely to change in the foreseeable future
  • This PR impacts documentation, and it has been updated (or a ticket has been logged)
  • This PR's changes are covered by the automated tests
  • This PR collects user input/sensitive content into Datadog

@lym953 lym953 requested a review from a team as a code owner September 24, 2024 17:32
Comment on lines +30 to +41
const helloPython = new Function(this, "hello-python", {
runtime: lambda.Runtime.PYTHON_3_12,
timeout: Duration.seconds(10),
memorySize: 256,
code: lambda.Code.fromAsset("../lambda/python", {
bundling: {
image: lambda.Runtime.PYTHON_3_12.bundlingImage,
command: ["bash", "-c", "pip install -r requirements.txt -t /asset-output && cp -aT . /asset-output"],
},
}),
handler: "hello.lambda_handler",
});

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 Code Quality Violation

The Function constructor is eval. (...read more)

The Function constructor can lead to code similar to eval executions. Use function declarations instead of the Function constructor.

View in Datadog  Leave us feedback  Documentation

Comment on lines +57 to +75
const helloDotnet = new Function(this, "hello-dotnet", {
runtime: lambda.Runtime.DOTNET_8,
handler: "HelloWorld::HelloWorld.Handler::SayHi",
memorySize: 256,
code: lambda.Code.fromAsset("../lambda/dotnet/", {
bundling: {
image: lambda.Runtime.DOTNET_8.bundlingImage,
command: [
"/bin/sh",
"-c",
" dotnet tool install -g Amazon.Lambda.Tools" +
" && dotnet build" +
" && dotnet lambda package --output-package /asset-output/function.zip",
],
user: "root",
outputType: BundlingOutput.ARCHIVED,
},
}),
});

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 Code Quality Violation

The Function constructor is eval. (...read more)

The Function constructor can lead to code similar to eval executions. Use function declarations instead of the Function constructor.

View in Datadog  Leave us feedback  Documentation

constructor(scope: Construct, id: string, props?: StackProps) {
super(scope, id, props);

console.log("Creating Hello World TypeScript stack");

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 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.

View in Datadog  Leave us feedback  Documentation

Comment on lines +16 to +28
const helloNode = new Function(this, "hello-node", {
runtime: lambda.Runtime.NODEJS_20_X,
memorySize: 256,
timeout: Duration.seconds(10),
code: lambda.Code.fromAsset("../lambda/node", {
bundling: {
image: lambda.Runtime.NODEJS_20_X.bundlingImage,
command: ["bash", "-c", "cp -aT . /asset-output && npm install --prefix /asset-output"],
user: "root",
},
}),
handler: "hello.lambda_handler",
});

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 Code Quality Violation

The Function constructor is eval. (...read more)

The Function constructor can lead to code similar to eval executions. Use function declarations instead of the Function constructor.

View in Datadog  Leave us feedback  Documentation

Comment on lines +30 to +41
const helloPython = new Function(this, "hello-python", {
runtime: lambda.Runtime.PYTHON_3_12,
timeout: Duration.seconds(10),
memorySize: 256,
code: lambda.Code.fromAsset("../lambda/python", {
bundling: {
image: lambda.Runtime.PYTHON_3_12.bundlingImage,
command: ["bash", "-c", "pip install -r requirements.txt -t /asset-output && cp -aT . /asset-output"],
},
}),
handler: "hello.lambda_handler",
});

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 Code Quality Violation

The Function constructor is eval. (...read more)

The Function constructor can lead to code similar to eval executions. Use function declarations instead of the Function constructor.

View in Datadog  Leave us feedback  Documentation

Comment on lines +57 to +75
const helloDotnet = new Function(this, "hello-dotnet", {
runtime: lambda.Runtime.DOTNET_8,
handler: "HelloWorld::HelloWorld.Handler::SayHi",
memorySize: 256,
code: lambda.Code.fromAsset("../lambda/dotnet/", {
bundling: {
image: lambda.Runtime.DOTNET_8.bundlingImage,
command: [
"/bin/sh",
"-c",
" dotnet tool install -g Amazon.Lambda.Tools" +
" && dotnet build" +
" && dotnet lambda package --output-package /asset-output/function.zip",
],
user: "root",
outputType: BundlingOutput.ARCHIVED,
},
}),
});

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 Code Quality Violation

The Function constructor is eval. (...read more)

The Function constructor can lead to code similar to eval executions. Use function declarations instead of the Function constructor.

View in Datadog  Leave us feedback  Documentation

constructor(scope: Construct, id: string, props?: StackProps) {
super(scope, id, props);

console.log("Creating Hello World TypeScript stack");

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 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.

View in Datadog  Leave us feedback  Documentation

Comment on lines +16 to +28
const helloNode = new Function(this, "hello-node", {
runtime: lambda.Runtime.NODEJS_20_X,
memorySize: 256,
timeout: Duration.seconds(10),
code: lambda.Code.fromAsset("../lambda/node", {
bundling: {
image: lambda.Runtime.NODEJS_20_X.bundlingImage,
command: ["bash", "-c", "cp -aT . /asset-output && npm install --prefix /asset-output"],
user: "root",
},
}),
handler: "hello.lambda_handler",
});

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 Code Quality Violation

The Function constructor is eval. (...read more)

The Function constructor can lead to code similar to eval executions. Use function declarations instead of the Function constructor.

View in Datadog  Leave us feedback  Documentation

console.log("Instrumenting Lambda Functions in TypeScript stack with Datadog");

const datadogLambda = new DatadogLambda(this, "Datadog", {
const datadogProps: DatadogProps = {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make this a separate variable to ensure DatadogProps can be imported properly.

});
};

const datadogLambda = new Datadog(this, "Datadog", datadogProps);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just double checking, the change from DatadogLambda to Datadog was intended in this PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch. Yes it's intended. A PR for renaming the interfaces was reverted, so I want to change the example stack back to use all the old interfaces. Let me also rename the variable datadogLambda to datadog.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool, that sounds good

@lym953 lym953 force-pushed the yiming.luo/rename-interfaces-1 branch from cc428d7 to d4e5aa1 Compare September 27, 2024 19:26
@lym953
Copy link
Contributor Author

lym953 commented Sep 27, 2024

/merge

@dd-devflow
Copy link

dd-devflow bot commented Sep 27, 2024

🚂 MergeQueue: pull request added to the queue

The median merge time in main is 4m.

Use /merge -c to cancel this operation!

@dd-mergequeue dd-mergequeue bot merged commit a73aea6 into main Sep 27, 2024
13 checks passed
@dd-mergequeue dd-mergequeue bot deleted the yiming.luo/rename-interfaces-1 branch September 27, 2024 19:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants