Fix/build image

[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 implements Datadog Data Streams Monitoring configuration and updates across multiple services to fix build image issues and improve observability.

• Adds Datadog trace propagation configuration to disable automatic extraction
• Updates Datadog layer versions across services to maintain compatibility
• Fixes build image configuration for multi-platform support and improved dependency management

Reviewed Changes

Copilot reviewed 43 out of 43 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
src/product-management-service/template.yaml	Adds DD trace propagation environment variables to Lambda functions
src/product-management-service/src/product-api/handle-*/main.go	Implements data streams checkpoints for observability
src/pricing-service/src/observability/observability.ts	Refactors data streams monitoring and fixes trace context handling
src/order-service/template.yaml	Updates Datadog extension layer version
src/inventory-service/template.yaml	Updates Datadog Java and extension layer versions
build-image/*	Improves Docker build configuration and dependency management

---

_{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: Fix/build image

I've completed a comprehensive review of this pull request. Here's my analysis:

Review Checklist:

• Analyze build image and deployment changes
• Review Datadog configuration and observability updates
• Check service-specific implementations (Go, TypeScript, Java, C#)
• Evaluate security implications of configuration changes
• Assess performance impact of observability changes
• Review error handling and edge cases
• Check for test coverage and documentation

---

🔍 Code Quality & Best Practices

✅ Positive Changes:

• Improved build infrastructure: Multi-platform Docker support and better dependency management
• Standardized observability: Consistent Datadog Data Streams Monitoring across all services
• Good separation of concerns: Trace propagation configuration isolated to ACL functions
• Version updates: Consistent Datadog extension layer versions (v80-84 range)

⚠️ Issues Identified:

1. Redundant condition in observability.ts (pricing-service:61):

   if (evt.traceparent !== undefined && evt.traceparent !== undefined) {

   Should be:

   if (evt.traceparent !== undefined) {

2. Unused options object (loyalty-point-service:33):

   const options: SpanOptions = {
     references: [{ referencedContext: undefined }]
   }
   // options is defined but never used in tracer.startSpan()

3. Interface contract change (product-management-service Go):

   func processMessage(ctx context.Context, record events.SQSMessage, span ddtrace.Span) error

   Adding the span parameter breaks the expected SQS handler interface. Consider retrieving span from context instead.

---

🔒 Security Considerations

✅ Good Security Practices:

• API keys properly stored in AWS Secrets Manager
• IAM permissions follow least-privilege principle
• CloudWatch logs disabled where Datadog handles logging
• No hardcoded secrets in environment variables

⚠️ Security Concerns:

• Trace propagation disabled: DD_TRACE_PROPAGATION_STYLE_EXTRACT: "none" may break distributed tracing security boundaries
• Empty requirements files: dev_requirements.txt and lambda_requirements.txt in activity-service are completely empty, which could cause build issues

---

🚀 Performance Impact

✅ Performance Improvements:

• Data Streams Monitoring provides better observability with minimal overhead
• Proper span linking and checkpointing for message flow tracking
• Build image optimizations should reduce cold start times

⚠️ Performance Considerations:

• Additional trace context processing in TypeScript services adds minimal latency
• Message size calculations (textEncoder.encode()) called multiple times per message
• Manual context parsing in Go services may be expensive for high-volume processing

---

🐛 Potential Bugs

1. Traceparent parsing vulnerability (product-management-service Go:97-110):

   parts := strings.Split(evt.TraceParent, "-")
   if len(parts) == 4 {
       traceId, err := strconv.ParseUint(parts[1], 16, 64)

   Missing validation for malformed trace IDs could cause panics.

2. Empty requirements files: Activity service Python dependencies are completely removed, which will break the build process.

3. Error handling gaps: JSON unmarshaling errors in Go services don't provide sufficient context for debugging.

---

🧪 Test Coverage

⚠️ Testing Gaps:

• No visible tests for the new Data Streams functionality
• Trace propagation configuration changes lack integration tests
• Build script changes (local-build.sh) need validation testing
• Manual span context handling needs unit tests

Recommendations:

• Add tests for traceparent parsing edge cases
• Test Data Streams checkpoint functionality
• Validate build process works with empty Python requirements

---

🛠️ Recommendations

High Priority:

1. Fix the redundant condition in pricing-service/src/observability/observability.ts:61
2. Restore Python dependencies or document why they were removed
3. Add input validation for trace ID parsing in Go services

Medium Priority:

1. Remove unused options object in loyalty-point-service
2. Consider retrieving span from context instead of passing as parameter
3. Add comprehensive error messages for debugging

Low Priority:

1. Optimize message size calculations to avoid duplicate encoding
2. Add performance benchmarks for Data Streams overhead
3. Document trace propagation configuration decisions

---

Overall Assessment: This is a solid infrastructure improvement that standardizes observability across services. The main concerns are a few code quality issues and potential build problems with empty Python requirements files. The changes should be safe to deploy after addressing the critical issues.

[claude]

Claude finished @jeastham1993's task —— View job

---

PR Review - Fix/build image

I've conducted a comprehensive review of this 39-file PR focused on Datadog observability improvements and build system fixes. Here's my detailed analysis:

Review Checklist:

• Analyze overall PR scope and architecture changes
• Review Datadog observability improvements across services
• Examine build system and Docker configuration changes
• Check for security vulnerabilities and best practices
• Validate performance considerations
• Review code quality and maintainability
• Assess test coverage implications
• Validate existing copilot feedback and provide additional insights

---

🔍 Code Quality & Issues

Critical Issues

1. Redundant condition check in src/loyalty-point-service/src/observability/observability.ts:62

   if (evt.traceparent !== undefined && evt.traceparent !== undefined) {

   The condition is duplicated. Should likely check evt.tracestate in the second condition or remove the duplicate.

2. Unused variable in src/loyalty-point-service/src/observability/observability.ts:27-29

   const options = {
     childOf: conventions.parentSpan ?? undefined,
   };

   The options object is defined but never used in tracer.startSpan call.

3. Interface contract breakage in Go code
   In src/product-management-service/src/product-acl/inventory-stock-updated-event-handler/main.go:74, the function signature was changed to add a span parameter:

   func processMessage(ctx context.Context, record events.SQSMessage, span ddtrace.Span) error

   This breaks the expected interface contract and may cause runtime issues.

---

🏗️ Architecture & Design

Positive Changes

• Consistent Datadog Data Streams Monitoring: Properly implemented across all services with standardized checkpoint configuration
• Layer version standardization: Updated to consistent versions (Java 24, Extension 84, .NET 20)
• Build system improvements: Multi-language build infrastructure enhanced with proper dependency management
• Trace propagation control: Added DD_TRACE_PROPAGATION_STYLE_EXTRACT: "none" for ACL handlers to prevent automatic extraction conflicts

Concerns

• Disabled CI workflows: Many deployment workflows are commented out (e.g., src/.github/workflows/deploy-activity-service.yml), which reduces deployment automation
• Dependency cleanup: Python requirements files were emptied (src/activity-service/dev_requirements.txt, src/activity-service/lambda_requirements.txt) which may cause build issues

---

🔒 Security Analysis

Good Practices

• CloudWatch logs properly disabled to prevent double billing while using Datadog extension
• API keys managed through AWS Secrets Manager
• Proper IAM role restrictions and least privilege principles

Minor Concerns

• Sample secret in template: "This is a sample secret key that should not be used in production" at src/product-management-service/template.yaml:102

---

⚡ Performance Considerations

Positive Impact

• Data Streams Monitoring: Efficient checkpoint implementation for message flow tracking
• Layer optimization: Updated to latest Datadog layer versions likely containing performance improvements
• Build optimization: Enhanced Docker build process should reduce build times

Potential Issues

• Memory allocation: Some functions use 2048MB memory (e.g., Java functions) - consider if this is necessary
• Span overhead: Extensive tagging in observability code may have minor performance impact

---

🧪 Test Coverage Implications

• Missing integration tests: No new tests added for Data Streams Monitoring functionality
• Build verification: CI workflow changes disable some test automation
• Manual testing required: Changes to core observability require thorough manual testing

---

🔧 Recommendations

Immediate Fixes Required

1. Fix the redundant condition in src/loyalty-point-service/src/observability/observability.ts:62
2. Remove unused options variable in the same file
3. Review the Go function signature change to ensure it doesn't break callers

Code Quality Improvements

1. Add error handling for Datadog checkpoint operations
2. Consider extracting observability configuration into shared modules to reduce duplication
3. Add validation for trace context parsing in manual context implementations

Build & Deployment

1. Re-enable commented workflow sections if they're still needed
2. Verify Python dependency requirements are intentionally emptied
3. Add health checks to verify Datadog integration after deployment

Documentation

1. Document the trace propagation configuration changes
2. Add examples of Data Streams Monitoring usage
3. Update deployment guides if build process changed

---

✅ Overall Assessment

This PR makes significant improvements to Datadog observability across the entire serverless application. The Data Streams Monitoring implementation is well-structured and follows best practices. The build system enhancements will improve developer experience.

However, there are several code quality issues that should be addressed before merging, particularly the JavaScript/TypeScript bugs and the Go interface changes that could cause runtime failures.

Recommendation: Request fixes for the critical issues before merging, but approve the overall direction and architecture of these changes.

[claude]

Claude finished @jeastham1993's task —— View job

---

PR Review: Fix/build image

I'm conducting a comprehensive review of this pull request which introduces Datadog Data Streams Monitoring configuration and build image improvements across multiple services.

Review Checklist:

• Analyze build image and deployment changes
• Review Datadog configuration and observability updates
• Check service-specific implementations (Go, TypeScript, Java, C#)
• Evaluate security implications of configuration changes
• Assess performance impact of observability changes
• Review error handling and edge cases
• Check for test coverage and documentation

Review Summary

This PR implements comprehensive changes to enable Datadog Data Streams Monitoring across the serverless sample application, fixes build image issues, and updates observability configurations. Here's my detailed analysis:

🎯 Major Changes Overview

1. Build Image Improvements

• ✅ Good: Enhanced build-image/local-build.sh with better dependency management
• ✅ Good: Improved multi-platform support and pre-compilation of services
• ✅ Good: Updated success-build.sh with better version management (Node 22, Go 1.24.1, Java 22)

2. Datadog Configuration Updates

• ✅ Good: Consistent Data Streams Monitoring enablement across all services
• ✅ Good: Updated Datadog layer versions for compatibility
• ⚠️ Note: Trace propagation disabled in several services - ensure this is intentional

3. Activity Service Dependencies

• ❌ Issue: Both dev_requirements.txt and lambda_requirements.txt were completely emptied - this will break the Python service

🔍 Code Quality Analysis

TypeScript/Node.js Services (Pricing, Loyalty-Point)

Issues Found:

1. Redundant Condition in loyalty-point-service/observability.ts:62

   if (evt.traceparent !== undefined && evt.traceparent !== undefined) {

   Fix: Remove duplicate condition, likely should check evt.tracestate instead.

2. Unused Variable in loyalty-point-service/observability.ts:27

   const options = {}; // Defined but never used

   Fix: Either use the options object or remove it.

Go Services (Product Management)

Issues Found:

1. Interface Contract Change in product-acl/inventory-stock-updated-event-handler/main.go:74

   func processMessage(ctx context.Context, record events.SQSMessage, span ddtrace.Span) error

   Issue: Adding the span parameter breaks the expected interface contract. Consider retrieving span from context instead.

Template & Infrastructure Changes

✅ Good Practices:

• Consistent environment variable configuration (DD_DATA_STREAMS_ENABLED: "true")
• Proper layer version updates across services
• Appropriate trace propagation settings for ACL handlers

⚠️ Concerns:

• Pricing service template has duplicate Type: String declaration (line 36-37)
• Some services disable automatic trace extraction - confirm this is intended behavior

Build & Deployment

✅ Improvements:

• Enhanced build script with better error handling (set -ex)
• Comprehensive dependency pre-compilation
• Updated runtime versions for better performance

❌ Critical Issue:

• Activity service Python dependencies completely removed, which will cause deployment failures

🔒 Security Analysis

✅ Good Security Practices:

• API keys properly managed through AWS Secrets Manager
• No hardcoded secrets or credentials
• CloudWatch logs disabled for Datadog extension to prevent double billing
• Appropriate IAM permissions for Datadog access

⚠️ Minor Concerns:

• Sample secret values still contain the typo: "This is a sample secret key that should not be used in production"` (missing closing quote)

🚀 Performance Considerations

✅ Performance Improvements:

• Data Streams Monitoring will provide better observability into message processing
• Updated Datadog layers should include performance enhancements
• ARM64 architecture usage where appropriate

⚠️ Potential Impact:

• Additional observability overhead from data streams monitoring
• Memory usage increase from enhanced tracing

📋 Required Actions

Critical Issues (Must fix before merge):

1. Activity Service Dependencies: Restore Python dependencies in:

   • src/activity-service/dev_requirements.txt
   • src/activity-service/lambda_requirements.txt

2. TypeScript Code Issues:

   • Fix duplicate condition in loyalty-point observability
   • Remove unused options variable

3. Pricing Service Template: Remove duplicate Type: String declaration

Recommended Improvements:

1. Go Interface: Consider retrieving span from context instead of passing as parameter
2. Documentation: Update any relevant documentation about the new Data Streams Monitoring features
3. Testing: Ensure observability changes are covered by integration tests

💡 Recommendations

1. Gradual Rollout: Consider enabling Data Streams Monitoring gradually across environments
2. Monitoring: Set up alerts for the new observability metrics
3. Cost Tracking: Monitor Datadog costs after enabling Data Streams features
4. Performance Baseline: Establish performance baselines before and after these changes

✅ Approval Readiness

This PR introduces valuable observability improvements but has several critical issues that must be addressed. Once the Python dependencies are restored and the TypeScript issues are fixed, this will be a solid enhancement to the application's observability capabilities.

Overall Assessment: Strong improvements with critical fixes needed before merge.

[claude]

Claude finished @jeastham1993's task —— View job

---

PR Review: Fix/build image

Comprehensive code review completed for PR #427 implementing Datadog Data Streams Monitoring configuration and build image improvements across multiple microservices.

Review Summary

Overall Assessment: This is a substantial infrastructure improvement PR that standardizes Datadog observability across multiple services and fixes build system issues. The changes are well-structured but contain several code quality issues that need attention.

✅ Strengths

1. Consistent Observability Implementation

   • Standardized Data Streams Monitoring across all services
   • Proper trace propagation and span linking implementation
   • Consistent environment variable configuration

2. Build System Improvements

   • Enhanced multi-platform Docker support
   • Streamlined dependency management
   • Updated toolchain versions (Node 22, .NET 9, Java 22, Go 1.24.1)

3. Layer Version Updates

   • Updated Datadog extension layers consistently (version 84)
   • Service-specific layer updates (Java 24, .NET 20)

❌ Critical Issues Found

1. Redundant Condition Check src/loyalty-point-service/src/observability/observability.ts:62

if (evt.traceparent !== undefined && evt.traceparent !== undefined) {

The condition is duplicated. Should check for both traceparent and tracestate or remove redundancy:

if (evt.traceparent !== undefined) {

2. Function Signature Breaking Change src/product-management-service/src/product-acl/inventory-stock-updated-event-handler/main.go:57,74
The processMessage function has inconsistent signatures:

• Line 57: processMessage(ctx, record, span) (3 params)
• Line 74: processMessage(ctx context.Context, record events.SQSMessage) (2 params)

This breaks the interface contract and will cause compilation errors.

3. Empty Dependency Files

• src/activity-service/dev_requirements.txt - Completely empty (was 585 lines)
• src/activity-service/lambda_requirements.txt - Completely empty (was 330 lines)

This suggests dependency management issues that could break the activity service.

⚠️ Security Concerns

1. CloudWatch Logs Disabled by Default

   • Both Java and C# services disable CloudWatch logs unless ENABLE_CLOUDWATCH_LOGS=Y
   • Could impact debugging and compliance requirements
   • Location: src/inventory-service/cdk/src/main/java/com/cdk/constructs/InstrumentedFunction.java:94-102

2. Environment Variable Exposure

   • Multiple services expose build IDs and deployment timestamps
   • Consider if this metadata should be available in production

🔧 Code Quality Issues

1. Trace Context Validation Missing

   • No validation for W3C trace context format before parsing
   • Could cause runtime errors with malformed trace headers
   • Location: src/product-management-service/src/product-acl/inventory-stock-updated-event-handler/main.go:95-111

2. Error Handling Inconsistency

   • Some services use try-catch blocks, others don't
   • Inconsistent error logging patterns across services

3. Resource Naming Consistency

   • Function names use different patterns across services
   • Some use dots, some use hyphens in identifiers

⚡ Performance Considerations

1. Memory Allocation

   • Java service uses 2048MB memory (high for serverless)
   • C# service uses 1024MB memory
   • Consider if these allocations are necessary

2. Cold Start Optimizations

   • Java service enables SnapStart for prod/test environments ✅
   • Other services could benefit from similar optimizations

📋 Recommendations

1. Immediate Fixes Required:

   • Fix the duplicated condition in loyalty-point-service
   • Resolve the function signature mismatch in product-management-service
   • Restore missing dependencies in activity-service

2. Follow-up Improvements:

   • Add trace context validation before parsing
   • Standardize error handling patterns
   • Review memory allocations for cost optimization
   • Add integration tests for Data Streams Monitoring

3. Consider:

   • Adding circuit breakers for external service calls
   • Implementing retry policies for data streams operations
   • Adding metrics for observability coverage

📊 Test Coverage

The PR lacks visible test coverage for the new observability features. Consider adding:

• Unit tests for trace propagation logic
• Integration tests for data streams checkpoints
• End-to-end tests for cross-service tracing

✅ Review Checklist Completed:

• Analyze build system and Docker configuration changes
• Review Datadog observability improvements across services
• Examine service-specific implementations (Go, TypeScript, Java, C#)
• Validate existing copilot feedback and provide additional insights
• Check for security vulnerabilities and best practices
• Assess performance considerations
• Review code quality and maintainability
• Provide comprehensive feedback and recommendations

Verdict: This PR improves the overall observability infrastructure significantly but requires fixes for the critical issues before merging to ensure system stability.