fix(lambda): use enum values for applicationLogLevel and systemLogLevel

[dali546]

Issue # (if applicable)

Reason for this change

Enumerate ApplicationLogLevel and SystemLogLevel to help with typing

Description of changes

Both fields should use the enum type for available options

Description of how you validated changes

Checklist

• My code adheres to the CONTRIBUTING GUIDE and DESIGN GUIDELINES

---

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

[aws-cdk-automation]

The pull request linter has failed. See the aws-cdk-automation comment below for failure reasons. If you believe this pull request should receive an exemption, please comment and provide a justification.

A comment requesting an exemption should contain the text Exemption Request. Additionally, if clarification is needed add Clarification Request to a comment.

[dali546]

Exemption Request: minor typescript change

[aaythapa]

Hi @dali546, thanks for making this change! Not sure why it wasn't an enum before. I'll have to double check to see if this is backwards compatible. In the meantime looks like the build step is failing because the some integration test snapshots need updates. You can use yarn integ --update-on-failed to update the integ snapshots.

[dali546]

Hi @aaythapa, thank you for taking a look at this PR... I've cloned down this repo, but unsure where to run yarn integ --update-on-failed? And it also suggests I should be authenticated with AWS for this too? Is this something I should be doing as a external contributor?

[aaythapa]

Hi @dali546, looking through the build log and I see API elements with incompatible change, this basically means that this PR in its current state is backwards incompatible :(.

You can enforce the enum type by making a new property (maybe called systemLogLevelV2) and have that property use the enum. You'd then deprecate the old systemLogLevel property. This would also fix the build failures you're seeing in the PR but you'd still have to add some integ tests.

This PR pretty much fixes the same issue you're trying to fix so feel free to use it as reference (note this PR doesn't deprecate the old property for some reason, this pr is a good example of adding a deprecated label). You can use our integ test guide to add new integ tests.

[dali546]

Hi @aaythapa. I need a little help. Locally I am trying to run yarn run integ-runner --update-on-failed --directory packages/@aws-cdk-testing/test/aws-lambda but my results show as UNCHANGED for all of them. it's not generating a new snapshot

Any advice?

[aaythapa]

Hi @dali546, sorry about the delay. Not sure what the exact issue could be. You can try deleting the snapshot folder for the test and re-making it. I see that integ.logging-config.js test is failing, so you can delete that tests snapshot folder and re-run so it creates a new folder, hopefully the different snapshot will show up.

✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.

[aaythapa]

@dali546 looks good, just some small final comments.

[aaythapa]

Can you add one more test for the case where systemLogLevelV2 or applicationLogLevelV2 is set but loggingFormat isn't set to LoggingFormat.JSON

[aaythapa]

nit: can you use loggingFormat here too

[aaythapa]

@mergify update

[mergify]

update

✅ Branch has been successfully updated

[aws-cdk-automation]

AWS CodeBuild CI Report

• CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
• Commit ID: 3e8b49d
• Result: SUCCEEDED
• Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository