Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

(stepfunctions): cannot disable StateMachine logging with removing LogGroup #30814

Closed
Tietew opened this issue Jul 10, 2024 · 5 comments · Fixed by #30816
Closed

(stepfunctions): cannot disable StateMachine logging with removing LogGroup #30814

Tietew opened this issue Jul 10, 2024 · 5 comments · Fixed by #30816
Labels
@aws-cdk/aws-stepfunctions Related to AWS StepFunctions bug This issue is a bug. effort/small Small work item – less than a day of effort p3

Comments

@Tietew
Copy link
Contributor

Tietew commented Jul 10, 2024

Describe the bug

When I have a StateMachine with logging enabled,
to disable logging still requires an existing LogGroup.

Expected Behavior

This configuration should be correct:

new sfn.StateMachine(stack, 'StateMachine', {
  definitionBody: sfn.DefinitionBody.fromChainable(...),
  logs: { level: sfn.LogLevel.OFF },
});

Current Behavior

TypeScript compile error occurs:

error TS2741: Property 'destination' is missing in type '{ level: sfn.LogLevel.OFF; }' but required in type 'LogOptions'.

Omitting logs property does not modify StateMachine's logging configuration.

Reproduction Steps

Deploy a StateMachine described in Expected Behavior.

Possible Solution

Make LogOptions.destination optional.
CDK should also verify destination is specified (or create one) when level is not OFF.

Additional Information/Context

CloudFormation Documentation says:
https://docs.aws.amazon.com/ja_jp/AWSCloudFormation/latest/UserGuide/aws-properties-stepfunctions-statemachine-loggingconfiguration.html

Destinations
Required, if your log level is not set to OFF.

Workaround (Escape hatch):

const states = new sfn.StateMachine(stack, 'StateMachine', {
  definitionBody: sfn.DefinitionBody.fromChainable(...),
});
(states.node.defaultChild as sfn.CfnStateMachine).addPropertyOverride('LoggingConfiguration', { Level: 'OFF' });

CDK CLI Version

2.148.0

Framework Version

No response

Node.js Version

v20.15.1

OS

Ubuntu

Language

TypeScript

Language Version

No response

Other information

No response
@Tietew Tietew added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Jul 10, 2024
@github-actions github-actions bot added the @aws-cdk/aws-stepfunctions Related to AWS StepFunctions label Jul 10, 2024
@Tietew Tietew mentioned this issue Jul 10, 2024
Merged
1 task
@khushail khushail added needs-reproduction This issue needs reproduction. investigating This issue is being investigated and/or work is in progress to resolve the issue. and removed needs-triage This issue or PR still needs to be triaged. needs-reproduction This issue needs reproduction. labels Jul 10, 2024
@khushail
Copy link
Contributor

khushail commented Jul 10, 2024 •
edited
Loading

Thanks @Tietew for reporting this issue.
I can confirm the issue and appreciate your efforts in PR contribution!

@khushail khushail added p2 p3 effort/small Small work item – less than a day of effort and removed investigating This issue is being investigated and/or work is in progress to resolve the issue. p2 labels Jul 10, 2024
This was referenced Jul 15, 2024
Closed
Closed
@github-actions github-actions bot mentioned this issue Aug 1, 2024
Closed
@wong-a
Copy link
Contributor

wong-a commented Aug 19, 2024

Does setting logs: undefined not work? By default there is no logging

new sfn.StateMachine(stack, 'StateMachine', {
  definitionBody: sfn.DefinitionBody.fromChainable(...),
});

@Tietew
Copy link
Contributor Author

Tietew commented Aug 20, 2024

@wong-a setting logs: undefined does not render LoggingConfiguration resource property. When updating a StateMachine with logging enabled, CloudFormation does not modify logging configuration when LoggingConfiguration resource property does not exist. We need to specify {"LoggingConfiguration":{"Level":"OFF"}} if we want to change logging from enabled to disabled.

@wong-a
Copy link
Contributor

wong-a commented Aug 20, 2024

CloudFormation does not modify logging configuration when LoggingConfiguration resource property does not exist. We need to specify {"LoggingConfiguration":{"Level":"OFF"}} if we want to change logging from enabled to disabled.

If this is true, it sounds like an issue to fix in CloudFormation resource instead of CDK

@wong-a wong-a mentioned this issue Sep 3, 2024
Merged
1 task
@mergify mergify bot closed this as completed in #30816 Feb 18, 2025
mergify bot pushed a commit that referenced this issue Feb 18, 2025
@Tietew
fix(stepfunctions): disabling logging still requires LogGroup (#30816)
a2cd7ae 
### Issue # (if applicable)

Closes #30814.

### Reason for this change

To disable logging on a StateMachine (with logging enabled), we should specify `LogLevel.OFF` to `LogOptions.level`. But cannot remove the LogGroup because `LogOptions.destination` is required.

``` ts
new sfn.StateMachine(this, 'StateMachine', {
  definitionBody: ...,
  logs: { level: sfn.LogLevel.OFF } // allow to disable logging
});
```

### Description of changes

- Make `LogOptions.destination` optional.
- Validate `LogOptions.destination` is present when `LogOptions.level` is not `OFF`.

### Description of how you validated changes

Unit and integ tests that verify `LogOptions.destination` is opitional when `LogOptions.level` is `OFF` and throw an exception otherwise.

### Checklist
- [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md)

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
@github-actions GitHub Actions
Copy link

Comments on closed issues and PRs are hard for our team to see.
If you need help, please open a new issue that references this one.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 18, 2025
yashkh-amzn pushed a commit to yashkh-amzn/aws-cdk that referenced this issue Feb 21, 2025
@Tietew @yashkh-amzn
fix(stepfunctions): disabling logging still requires LogGroup (aws#30816
1859098 
)

### Issue # (if applicable)

Closes aws#30814.

### Reason for this change

To disable logging on a StateMachine (with logging enabled), we should specify `LogLevel.OFF` to `LogOptions.level`. But cannot remove the LogGroup because `LogOptions.destination` is required.

``` ts
new sfn.StateMachine(this, 'StateMachine', {
  definitionBody: ...,
  logs: { level: sfn.LogLevel.OFF } // allow to disable logging
});
```

### Description of changes

- Make `LogOptions.destination` optional.
- Validate `LogOptions.destination` is present when `LogOptions.level` is not `OFF`.

### Description of how you validated changes

Unit and integ tests that verify `LogOptions.destination` is opitional when `LogOptions.level` is `OFF` and throw an exception otherwise.

### Checklist
- [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md)

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
@aws-cdk/aws-stepfunctions Related to AWS StepFunctions bug This issue is a bug. effort/small Small work item – less than a day of effort p3
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix(stepfunctions): disabling logging still requires LogGroup Tietew/aws-cdk
3 participants
@Tietew @wong-a @khushail