feat: add initial AWS implementation#24
Conversation
![datadog-datadog-prod-us1[bot]](https://avatars.githubusercontent.com/u/365230?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull Request Overview
Add initial AWS support for the User Management service, including message processing and infrastructure deployment.
- Introduce SQS worker to handle
StickerClaimedEventV1messages. - Implement SNS publisher with CloudEvents for user registration announcements.
- Define CDK stack for deploying queues, topics, Lambdas, and API Gateway.
Reviewed Changes
Copilot reviewed 40 out of 40 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Stickerlandia.UserManagement.AWS/Stickerlandia.UserManagement.AWS.csproj | Bump and add AWS SDK and CloudEvents package references. |
| src/Stickerlandia.UserManagement.AWS/SqsStickerClaimedWorker.cs | New worker to poll and process SQS messages. |
| src/Stickerlandia.UserManagement.AWS/SnsEventPublisher.cs | Publisher converting events to CloudEvents and sending to SNS. |
| src/Stickerlandia.UserManagement.AWS/ServiceExtensions.cs | Register AWS clients, Lambda hosting, and messaging adapters. |
| infra/aws/src/UserManagementServiceStack.cs | CDK stack for DynamoDB, SQS, SNS, Lambdas, and API Gateway. |
Comments suppressed due to low confidence (2)
user-management/src/Stickerlandia.UserManagement.AWS/SqsStickerClaimedWorker.cs:19
- [nitpick] Public classes like this should have XML doc comments summarizing their role and public surface to improve maintainability and discoverability.
public class SqsStickerClaimedWorker : IMessagingWorker
user-management/src/Stickerlandia.UserManagement.AWS/SqsStickerClaimedWorker.cs:38
- The message processing logic, including JSON deserialization and exception handling, should have unit tests to validate both happy and failure paths.
private async Task ProcessMessageAsync(Message message)
Co-authored-by: datadog-datadog-prod-us1[bot] <88084959+datadog-datadog-prod-us1[bot]@users.noreply.github.com>
…ublisher.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…rClaimedWorker.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
scottgerring
left a comment
scottgerring
left a comment
There was a problem hiding this comment.
I had a skim and added a couple of small comments, but it struck me that we probably want to formalise our deployment targets before we go much further down the road with IaC. What do you think?
I jotted down what I recall we've discussed so far over on #26
| //TODO: Implement AWS Lambda support | ||
| var apiLambdaFunction = builder.AddAWSLambdaFunction<Projects.Stickerlandia_UserManagement_Api>("UsersApi", | ||
| "Stickerlandia.UserManagement.Api") | ||
| .WithEnvironment("ConnectionStrings__messaging", resources.MessagingResource) |
There was a problem hiding this comment.
This env var style grosses me out a bit, but it's the established pattern in dotnet, right?
There was a problem hiding this comment.
Yeah, but not a strict rule. What would you prefer? All uppercase?
There was a problem hiding this comment.
I'm going to leave this for the moment, can fix in a later update if needed.
| 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 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: | ||
| "Server=ep-divine-snow-abgipduo-pooler.eu-west-2.aws.neon.tech;Port=5432;Database=stickerlandia-users;User Id=stickerlandia-users_owner;Password=npg_buwe2PoK1NgV;sslmode=require;", | ||
| 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.
| export class BackgroundWorkers extends Construct { | ||
| constructor(scope: Construct, id: string, props: BackgroundWorkersProps) { | ||
| super(scope, id); | ||
|
|
||
| const eventBus = new EventBus(this, "UserManagementEventBus", { | ||
| eventBusName: `${props.sharedProps.serviceName}-${props.sharedProps.environment}-bus`, | ||
| }); | ||
|
|
||
| const environmentVariables = { | ||
| POWERTOOLS_SERVICE_NAME: props.sharedProps.serviceName, | ||
| POWERTOOLS_LOG_LEVEL: | ||
| props.sharedProps.environment === "prod" ? "WARN" : "INFO", | ||
| ENV: props.sharedProps.environment, | ||
| ConnectionStrings__messaging: "", | ||
| ConnectionStrings__database: props.sharedProps.connectionString, | ||
| Aws__UserRegisteredTopicArn: props.userRegisteredTopic.topicArn, | ||
| Aws__StickerClaimedQueueUrl: props.stickerClaimedQueue.queueUrl, | ||
| Aws__StickerClaimedDLQUrl: props.stickerClaimedDLQ.queueUrl, | ||
| DRIVING: "ASPNET", | ||
| DRIVEN: "AWS", | ||
| DISABLE_SSL: "true", | ||
| }; | ||
|
|
||
| const stickerClaimedWorker = new InstrumentedLambdaFunction( | ||
| this, | ||
| "StickerClaimedWorkerFunction", | ||
| { | ||
| sharedProps: props.sharedProps, | ||
| handler: | ||
| "Stickerlandia.UserManagement.Lambda::Stickerlandia.UserManagement.Lambda.Sqs_StickerClaimed_Generated::StickerClaimed", | ||
| buildDef: | ||
| "../../src/Stickerlandia.UserManagement.Lambda/", | ||
| functionName: "sticker-claimed-worker", | ||
| environment: environmentVariables, | ||
| memorySize: 1024, | ||
| timeout: Duration.seconds(25), | ||
| logLevel: props.sharedProps.environment === "prod" ? "WARN" : "INFO", | ||
| onFailure: new SqsDestination(props.stickerClaimedDLQ), | ||
| } | ||
| ); | ||
|
|
||
| stickerClaimedWorker.function.addEventSource( | ||
| new SqsEventSource(props.stickerClaimedQueue, { | ||
| batchSize: 10, | ||
| reportBatchItemFailures: true, | ||
| }) | ||
| ); | ||
|
|
||
| const rule = new Rule(this, "StickerClaimedEventRule", { | ||
| eventBus: eventBus, | ||
| ruleName: `${props.sharedProps.serviceName}-${props.sharedProps.environment}-sticker-claimed-rule`, | ||
| eventPattern: { | ||
| source: [`${props.sharedProps.environment}.stickers`], | ||
| detailType: ["users.stickerClaimed.v1"], | ||
| }, | ||
| }); | ||
| rule.addTarget(new SqsQueue(props.stickerClaimedQueue)); | ||
|
|
||
| const outboxWorker = new InstrumentedLambdaFunction( | ||
| this, | ||
| "OutboxWorkerFunction", | ||
| { | ||
| sharedProps: props.sharedProps, | ||
| handler: | ||
| "Stickerlandia.UserManagement.Lambda::Stickerlandia.UserManagement.Lambda.OutboxFunctions_Worker_Generated::Worker", | ||
| buildDef: | ||
| "../../src/Stickerlandia.UserManagement.Lambda/", | ||
| functionName: "outbox-worker", | ||
| environment: environmentVariables, | ||
| memorySize: 1024, | ||
| timeout: Duration.seconds(50), | ||
| logLevel: props.sharedProps.environment === "prod" ? "WARN" : "INFO", | ||
| onFailure: new SqsDestination(props.stickerClaimedDLQ), | ||
| } | ||
| ); | ||
| props.userRegisteredTopic.grantPublish(outboxWorker.function); | ||
|
|
||
| const outboxWorkerSchedule = new Rule(this, "OutboxWorkerSchedule", { | ||
| description: "Trigger outbox worker every 1 minute", | ||
| schedule: Schedule.rate(Duration.minutes(1)), | ||
| }); | ||
|
|
||
| // Add the Lambda function as a target | ||
| outboxWorkerSchedule.addTarget( | ||
| new LambdaFunction(outboxWorker.function, { | ||
| retryAttempts: 2, | ||
| event: RuleTargetInput.fromObject({ | ||
| run: true, | ||
| }), | ||
| }) | ||
| ); | ||
| } | ||
| } |
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.
|
|
||
| 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.
No description provided.