feat: add AWS implementation and IaC#36
Conversation
|
|
||
| namespace Stickerlandia.UserManagement.Lambda; | ||
|
|
||
| public class Sqs(ILogger<Sqs> logger, IServiceScopeFactory serviceScopeFactory, OutboxProcessor outboxProcessor) |
There was a problem hiding this comment.
🟠 Code Quality Violation
This class is considered dead code because it has a private constructor that isn't instantiated within the class and no public static declarations. (...read more)
Classes with a private constructor can't be instantiated outside of the class itself. Because they are unreachable, they should be removed or made public.
An exception is made for classes that access their own constructors (like a singleton), and classes that derive from System.Runtime.InteropServices.SafeHandle.
There was a problem hiding this comment.
Pull Request Overview
This PR removes legacy Azure Application Insights resources, centralizes connection strings, and introduces a parallel AWS CDK-based infrastructure for the user-management service.
- Removed Azure App Insights & consumption plan resources and switched function app settings to use
var.database_connection_string. - Added Azure Container Apps environment and data subscription blocks.
- Bootstrapped a full AWS CDK TypeScript project with networking, ECS Fargate services, Lambda constructs, Datadog instrumentation, and related configs.
Reviewed Changes
Copilot reviewed 60 out of 60 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| user-management/infra/azure/main.tf | Updated app_settings, removed App Insights & flex plan |
| user-management/infra/azure/data.tf | Added subscription data source |
| user-management/infra/azure/application.tf | Provisioned Container Apps environment & app deployments |
| user-management/infra/aws/tsconfig.json | New TS compiler options for AWS CDK |
| user-management/infra/aws/package.json | Added CDK & Datadog dependencies |
| user-management/infra/aws/lib/user-service-stack.ts | Defined VPC, ECS cluster, Datadog constructs |
| user-management/infra/aws/lib/network.ts | VPC construct |
| user-management/infra/aws/lib/constructs/shared-props.ts | SharedProps interface |
| user-management/infra/aws/lib/constructs/instrumented-function.ts | Instrumented Lambda construct |
| user-management/infra/aws/lib/background-workers.ts | Background worker lambdas & event bus setup |
| user-management/infra/aws/lib/api.ts | ECS Fargate service & load balancer config |
| user-management/infra/aws/jest.config.js | Jest config |
| user-management/infra/aws/cdk.json | CDK app & context flags |
| user-management/infra/aws/README.md | CDK project README |
| user-management/infra/aws/.npmignore | npmignore for publishing |
| user-management/infra/aws/.gitignore | gitignore for AWS folder |
| user-management/Stickerlandia.UserManagement.sln | Fixed project path for Lambda csproj |
Comments suppressed due to low confidence (2)
user-management/infra/azure/main.tf:67
- Terraform map assignments use
=rather than:. Replace"DRIVING" : "AZURE"with"DRIVING" = "AZURE"(and similarly forDRIVEN).
"DRIVING" : "AZURE",
user-management/infra/azure/main.tf:103
local.environment_variablesis not defined—this will cause a plan error. It should referencelocal.app_settings.
app_settings = local.environment_variables
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
… into feat/infra-as-code
feat(award-service): sticker-award to return structured errors according to spec
scottgerring
left a comment
There was a problem hiding this comment.
This is huge!
i've done what I can. The main thing that is unclear to me is how the the combination of bits ties back to https://github.com/DataDog/stickerlandia/blob/main/docs/deploy.md
I think it would be helpful to map each configuration through to the concrete components used; it's hard to see the woods-for-the-trees in terms of the two concrete deployment configurations we are pushing here (AWS/Azure Serverless)
| this.stickerClaimedDLQ = new Queue(this, "StickerClaimedDLQ", { | ||
| queueName: `${props.sharedProps.serviceName}-${props.sharedProps.environment}-sticker-claimed-dlq`, | ||
| }); | ||
| this.stickerClaimedQueue = new Queue(this, "StickerClaimedQueue", { |
There was a problem hiding this comment.
I was going to ask how we're doing queue/topic ownership given the stacks are going to be broken down by service!
So the queue exists here - and when we add the topic on the award-service side, we'll add a subscription here?
There was a problem hiding this comment.
I think the point of integration between services should be Amazon EventBridge. So we'd have a shared event bus deployed as part of some shared infra, with the name/ARN stored in SSM. In the respective CDK stacks we can then pull down a reference to that event bus to define rules. Left it out for the moment, can add when we come to add the proper AWS deployments
There was a problem hiding this comment.
Then we likely don't need SNS, eh?
| protocol: Protocol.TCP, | ||
| }); | ||
|
|
||
| applicationTaskDef.addContainer("datadog-agent", { |
There was a problem hiding this comment.
What's going to be interesting here is, once we deploy more than one service to ECS-fargate we're likely going to want a single Daemon style instance of the agent in the cluster, I suspect
There was a problem hiding this comment.
At the moment, the only deployment method for Fargate is per task. The team are working on that.
There was a problem hiding this comment.
I don't see why you couldn't just deploy the agent ordinarily and point at it, the same as in any other containerised environment? Does some part of the auto-instrumentation or magical environment discovery not work?
| export class UserServiceStack extends cdk.Stack { | ||
| constructor(scope: Construct, id: string, props?: cdk.StackProps) { | ||
| super(scope, id, props); | ||
|
|
||
| const serviceName = "user-service"; | ||
| const environment = process.env.ENV || "dev"; | ||
| const version = process.env.VERSION || "latest"; | ||
| const connectionString = process.env.CONNECTION_STRING; | ||
|
|
||
| if (!connectionString) { | ||
| throw new Error("CONNECTION_STRING environment variable is required"); | ||
| } | ||
|
|
||
| const network = new Network(this, "Network", { | ||
| networkName: `${serviceName}-${environment}-vpc`, | ||
| }); | ||
|
|
||
| const ddSite = process.env.DD_SITE || "datadoghq.com"; | ||
| const ddApiKey = process.env.DD_API_KEY || ""; | ||
|
|
||
| const cluster = new Cluster(this, "ApiCluster", { | ||
| vpc: network.vpc, | ||
| clusterName: `${serviceName}-${environment}`, | ||
| }); | ||
|
|
||
| const sharedProps: SharedProps = { | ||
| connectionString, | ||
| serviceName: "user-service", | ||
| environment: process.env.ENV || "dev", | ||
| version: process.env.VERSION || "latest", | ||
| team: "users", | ||
| domain: "users", | ||
| datadog: { | ||
| apiKey: ddApiKey, | ||
| site: ddSite, | ||
| lambda: new DatadogLambda(this, "DatadogLambda", { | ||
| apiKey: ddApiKey, | ||
| site: ddSite, | ||
| }), | ||
| ecsFargate: new DatadogECSFargate({ | ||
| // One of the following 3 apiKey params are required | ||
| apiKey: ddApiKey, | ||
| cpu: 256, | ||
| memoryLimitMiB: 512, | ||
| isDatadogEssential: true, | ||
| isDatadogDependencyEnabled: true, | ||
| site: ddSite, | ||
| clusterName: cluster.clusterName, | ||
| environmentVariables: {}, | ||
| dogstatsd: { | ||
| isEnabled: true, | ||
| }, | ||
| apm: { | ||
| isEnabled: true, | ||
| traceInferredProxyServices: true, | ||
| }, | ||
| logCollection: { | ||
| isEnabled: true, | ||
| fluentbitConfig: { | ||
| firelensOptions: { | ||
| enableECSLogMetadata: true, | ||
| }, | ||
| logDriverConfig: { | ||
| hostEndpoint: `http-intake.logs.${ddSite}`, | ||
| serviceName: serviceName, | ||
| }, | ||
| }, | ||
| }, | ||
| env: environment, | ||
| service: serviceName, | ||
| version: version, | ||
| }), | ||
| }, | ||
| }; | ||
|
|
||
| const api = new Api(this, "Api", { | ||
| sharedProps: sharedProps, | ||
| vpc: network.vpc, | ||
| cluster: cluster, | ||
| }); | ||
|
|
||
| const backgroundWorkers = new BackgroundWorkers(this, "BackgroundWorkers", { | ||
| sharedProps: sharedProps, | ||
| stickerClaimedQueue: api.stickerClaimedQueue, | ||
| stickerClaimedDLQ: api.stickerClaimedDLQ, | ||
| userRegisteredTopic: api.userRegisteredTopic, | ||
| }); | ||
| } | ||
| } |
There was a problem hiding this comment.
⚪ Code Quality Violation
This class is unnecessary (...read more)
This rule advises against the unnecessary use of classes that contain only static members, or nothing. In JavaScript, classes are primarily used for object-oriented programming, where each instance of a class has its own state and behavior. Static members, on the other hand, belong to the class itself and not to any instance of the class.
When a class contains only static members, it does not make use of JavaScript's object-oriented capabilities, and it can be more difficult to understand, test, and maintain than necessary. In order to avoid this issue, consider using regular functions and variables instead of static class members. This makes your code easier to understand and maintain, and it allows you to make better use of JavaScript's features.
* feat: implement fixes from static analysis * feat: add static analysis configuration * feat: add static analysis configuration * fix: remove duplicate .sln file * Update user-management/src/Stickerlandia.UserManagement.Api/Configurations/RemoveSwaggerDefinitionsFilter.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
feat: add Datadog tracer to Dockerfiles
* chore: update OpenAPI docs * chore: update README * chore: add license and notice files * chore: add dockerignore file * Update user-management/src/Stickerlandia.UserManagement.Api/Configurations/RemoveSwaggerDefinitionsFilter.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update user-management/src/Stickerlandia.UserManagement.Api/Configurations/AuthorizeOperationFilter.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update user-management/README.md Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * chore: expand local dev and code structure docs * chore: add cloud provider specific SLN files * fix: specify .sln in dotnet restore * chore: docs updates --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
… into feat/infra-as-code
No description provided.