winglang / wing

A programming language for the cloud ☁️ A unified programming model, combining infrastructure and runtime code into one language ⚡
https://winglang.io
Other
4.98k stars 196 forks source link

cloudwatch log groups should define retention period #4134

Closed skorfmann closed 3 weeks ago

skorfmann commented 1 year ago

I tried this:

Create and destroy any resource which will create log groups implicitly (e.g. a cloud.Function)

This happened:

Screenshot 2023-09-10 at 14 01 54

Log groups are created with infinity as retention period.

I expected this:

Log groups to define a retention period of e.g. 7 days (any value will do).

Is there a workaround?

No response

Component

SDK

Wing Version

0.29.6

Node.js Version

20.6.0

Platform(s)

No response

Anything else?

tf cloudwatch logs can set a skip destroy, so that these will be kept around. Would probably be nice if there was an option if that's desired or not, but that's another issue.

Community Notes

Chriscbr commented 1 year ago

I think this issue may need some more discussion. I'm not sure if the change in #4303 to add logRetentionDays as an option in cloud.Function is the right design for the Wing Cloud Library, at least for now. We have a tenet that APIs for resources should be focused on functional behavior:

Our APIs are designed around the functional aspects that developers care about for building and testing their applications. Implementations of resources in the SDK are guaranteed to be scalable, highly-available, and fault tolerant by default, so that developers do not need to customize security policies or scaling configuration within their application code. Operational aspects of resources should not leak into the core API surface area, except when they are essential to the functional behavior of the resource and the user's mental model. For example, while the timeout of a serverless function can be considered an operational detail, it is essential to the user's mental model of functions as an ephemeral, stateless resource that should not be used for long-running or stateful workloads. [1]

The duration a function's logs are stored for doesn't affect its functional behavior, nor the user's mental model of the resource, so it doesn't feel right to include it as part of our public API.

I think the better design here would be to set the default log retention to 30 days, and if it's needed, the user (or organization or platform team) can override the target-specific implementation of the resource through compiler plugins, escape hatches, or platform providers.

tsuf239 commented 1 year ago

I think the better design here would be to set the default log retention to 30 days, and if it's needed, the user (or organization or platform team) can override the target-specific implementation of the resource through compiler plugins, escape hatches, or platform providers.

Wing is also designed to make stuff easier, and I wouldn't want our users to have to spend a whole day or two writing a plugin or escape hatch, just to make their log deleted within one week, especially since it takes one property change in other solutions, and it seems like a needed feature, or a common use case...

What about using a global variable/ configuration file? (To put other non-functional yet common customizations that are likely to rise in the future)

tsuf239 commented 11 months ago

Spoke to @Chriscbr a few days ago. We agreed that log retention isn't part of the application's functional behavior, but a side effect of having an app. Since this is an important one (and one of Wing's goals is to make things easier and more accessible for the developer) we may need to supply a simple way for the developer to control certain properties in their app (in a declarative way, or so, in a configuration file, or global variable, for example) I'll bring it up in one of the next team times to decide what will be the best way. (@staycoolcall911 FYI) (another action item is to make the log retention fixed to 30, but I would prefer to see first what is the approach for easy-non-functional properties, to avoid redundant work)

tsuf239 commented 11 months ago

Will be resolved with Wing Custom Platform Providers https://github.com/winglang/wing/issues/3620

github-actions[bot] commented 9 months ago

Hi,

This issue hasn't seen activity in 60 days. Therefore, we are marking this issue as stale for now. It will be closed after 7 days. Feel free to re-open this issue when there's an update or relevant information to be added. Thanks!

staycoolcall911 commented 9 months ago

Since #3620 is done, all that is left is to set the log retention to 30s and we can close this issue.

github-actions[bot] commented 7 months ago

Hi,

This issue hasn't seen activity in 60 days. Therefore, we are marking this issue as stale for now. It will be closed after 7 days. Feel free to re-open this issue when there's an update or relevant information to be added. Thanks!

github-actions[bot] commented 4 months ago

Hi,

This issue hasn't seen activity in 90 days. Therefore, we are marking this issue as stale for now. It will be closed after 7 days. Feel free to re-open this issue when there's an update or relevant information to be added. Thanks!

github-actions[bot] commented 1 month ago

Hi,

This issue hasn't seen activity in 90 days. Therefore, we are marking this issue as stale for now. It will be closed after 7 days. Feel free to re-open this issue when there's an update or relevant information to be added. Thanks!

Chriscbr commented 3 weeks ago

See logRetention here: https://github.com/winglang/wing/blob/9adaea0116fc8d00229d45660b6fdf1456708ec8/packages/%40winglang/sdk/src/cloud/function.ts#L53-L58