Feat/fix cdk

[jeastham1993]

What does this PR do?

Motivation

Testing Guidelines

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

[Copilot]

Pull Request Overview

This PR removes log group removal policy configurations from Lambda functions in the pricing service CDK infrastructure. The changes eliminate explicit applyRemovalPolicy(RemovalPolicy.DESTROY) calls on Lambda function log groups.

• Removes explicit log group removal policies from pricing event handlers
• Simplifies CDK configuration by relying on default log group behavior

---

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

[claude]

Claude finished @jeastham1993's task —— View job

---

PR Review: Feat/fix cdk

I've completed my review of this merged PR. Here's my comprehensive analysis:

Todo List

• Read repository guidelines (CLAUDE.md)
• Examine the modified file and understand changes
• Analyze code quality and best practices
• Check for potential bugs or issues
• Evaluate performance considerations
• Review security concerns
• Assess test coverage implications
• Provide comprehensive feedback

Review Summary

Context: This PR appears to be part of an initial codebase commit rather than a specific fix removing log group removal policies. The file pricingEventHandlers.ts is newly created, not modified.

Code Quality & Best Practices ✅

Positive aspects:

• Clean, well-structured TypeScript CDK code
• Proper separation of concerns with separate methods for each handler
• Consistent naming conventions (camelCase for methods, PascalCase for classes)
• Good use of TypeScript interfaces for type safety
• Proper imports and dependency management

Areas for improvement:

• Missing JSDoc comments for public methods and interfaces
• Hardcoded string literals could be extracted to constants
• No explicit error handling in construct definitions

Potential Issues ⚠️

1. Log Group Management: The current implementation relies on CDK's default log group creation. While not necessarily problematic, this means:

   • Log groups will be created automatically by Lambda with default settings
   • No explicit retention period control
   • Default removal policy (logs persist after stack deletion)

2. Resource Naming: Function names use template literals with environment, which is good, but consider length limits for AWS resources.

3. Build Dependencies: The code references build files (buildProductCreatedPricingHandler.js) that may not exist at CDK synth time, potentially causing deployment issues.

Performance Considerations 🚀

• Memory allocation: 512MB is reasonable for event handlers
• Timeout: 29 seconds is conservative and appropriate
• Runtime: Node.js 22.x is current and well-optimized
• Bundling: ESBuild configuration is appropriate for Lambda

Security Review 🔒

Good practices:

• Environment variables properly scoped
• Datadog tracing configuration follows security guidelines
• IAM permissions granted through proper CDK constructs
• No hardcoded secrets or sensitive data

Considerations:

• Datadog configuration allows extensive tracing - ensure this aligns with data governance policies
• Event bus permissions should follow principle of least privilege

Test Coverage 📋

Existing test structure: The pricing service has comprehensive test coverage with:

• Unit tests for handlers and core logic
• Integration tests for end-to-end flows
• Good test organization under /tests/unit/ and /tests/integration/

Missing coverage for this file: No direct tests for the CDK construct itself, though this is common for infrastructure code.

Recommendations

1. Add Log Group Management (if explicit control needed):

   import { LogGroup } from "aws-cdk-lib/aws-logs";
   
   const logGroup = new LogGroup(this, 'ProductCreatedLogGroup', {
     logGroupName: `/aws/lambda/${functionName}`,
     retention: RetentionDays.ONE_WEEK, // or as per requirements
     removalPolicy: RemovalPolicy.DESTROY // or RETAIN for production
   });

2. Extract Constants:

   const BUILD_PATHS = {
     productCreated: "./src/pricing-api/adapters/buildProductCreatedPricingHandler.js",
     productUpdated: "./src/pricing-api/adapters/buildProductUpdatedPricingHandler.js"
   } as const;

3. Add JSDoc Documentation:

   /**
    * Constructs Lambda functions for handling pricing-related product events
    */
   export class PricingEventHandlers extends Construct {

Overall Assessment

This is well-written infrastructure code that follows AWS CDK best practices. The "removal of log group policies" mentioned in the PR description isn't evident in this review, as this appears to be new code rather than a modification. The current approach of relying on default Lambda log group creation is acceptable for most use cases.

Score: 8/10 - Solid implementation with room for minor improvements in documentation and explicit resource management.