out_cloudwatch_logs: Only create log group if it does not already exist #4826

PettitWesley · 2022-02-15T08:08:32Z

Signed-off-by: Wesley Pettit wppttt@amazon.com

Enter [N/A] in the box, if an item is not applicable to your change.

Testing
Before we can approve your change; please submit the following in a comment:

Example configuration file for the change
Debug log output from testing the change

Attached Valgrind output that shows no leaks or memory corruption was found

If this is a change to packaging of containers or native binaries then please confirm it works for all targets.

Attached local packaging test output showing all targets (including any new ones) build.

Documentation

Documentation required for this feature

Backporting

Backport to latest stable release.

Fluent Bit is licensed under Apache 2.0, by submitting this pull request I understand that this code will be released under the terms of that license.

PettitWesley · 2022-02-15T08:17:20Z

Now, if the group already exists, it won't even try to create it, it just creates the stream

Fluent Bit v1.8.13
* Copyright (C) 2015-2021 The Fluent Bit Authors
* Fluent Bit is a CNCF sub-project under the umbrella of Fluentd
* https://fluentbit.io

[2022/02/15 08:15:05] [ info] [engine] started (pid=11476)
[2022/02/15 08:15:05] [ info] [storage] version=1.1.6, initializing...
[2022/02/15 08:15:05] [ info] [storage] in-memory
[2022/02/15 08:15:05] [ info] [storage] normal synchronization mode, checksum disabled, max_chunks_up=128
[2022/02/15 08:15:05] [ info] [cmetrics] version=0.2.2
[2022/02/15 08:15:05] [ info] [sp] stream processor started
[2022/02/15 08:15:10] [ info] [output:cloudwatch_logs:cloudwatch_logs.0] Creating log stream from-fluent-bit-dummy in log group fluent-bit-is-the-best-cloudwatch-group-throttle
[2022/02/15 08:15:10] [ info] [output:cloudwatch_logs:cloudwatch_logs.0] Log Stream from-fluent-bit-dummy already exists

However, if stream creation fails on the group not existing, then we try to create it:

Fluent Bit v1.8.13
* Copyright (C) 2015-2021 The Fluent Bit Authors
* Fluent Bit is a CNCF sub-project under the umbrella of Fluentd
* https://fluentbit.io

[2022/02/15 08:14:23] [ info] [engine] started (pid=11215)
[2022/02/15 08:14:23] [ info] [storage] version=1.1.6, initializing...
[2022/02/15 08:14:23] [ info] [storage] in-memory
[2022/02/15 08:14:23] [ info] [storage] normal synchronization mode, checksum disabled, max_chunks_up=128
[2022/02/15 08:14:23] [ info] [cmetrics] version=0.2.2
[2022/02/15 08:14:23] [ info] [sp] stream processor started
[2022/02/15 08:14:28] [ info] [output:cloudwatch_logs:cloudwatch_logs.0] Creating log stream from-fluent-bit-dummy in log group fluent-bit-is-the-best-cloudwatch-group-throttle
[2022/02/15 08:14:28] [ info] [output:cloudwatch_logs:cloudwatch_logs.0] Log Group fluent-bit-is-the-best-cloudwatch-group-throttle not found. Will attempt to create it.
[2022/02/15 08:14:28] [ info] [output:cloudwatch_logs:cloudwatch_logs.0] Creating log group fluent-bit-is-the-best-cloudwatch-group-throttle
[2022/02/15 08:14:28] [ info] [output:cloudwatch_logs:cloudwatch_logs.0] Created log group fluent-bit-is-the-best-cloudwatch-group-throttle
[2022/02/15 08:14:28] [ info] [output:cloudwatch_logs:cloudwatch_logs.0] Creating log stream from-fluent-bit-dummy in log group fluent-bit-is-the-best-cloudwatch-group-throttle
[2022/02/15 08:14:28] [ info] [output:cloudwatch_logs:cloudwatch_logs.0] Created log stream from-fluent-bit-dummy

This reduces unnecessary calls to CreateLogGroup API.

If auto_create_group is disabled, the user gets a useful warning:

[2022/02/15 08:15:28] [ info] [output:cloudwatch_logs:cloudwatch_logs.0] Creating log stream from-fluent-bit-dummy in log group fluent-bit-is-the-best-cloudwatch-group-throttle-2
[2022/02/15 08:15:28] [error] [output:cloudwatch_logs:cloudwatch_logs.0] Log Group fluent-bit-is-the-best-cloudwatch-group-throttle-2 not found and `auto_create_group` disabled.
[2022/02/15 08:15:28] [ warn] [engine] failed to flush chunk '11589-1644912924.153454467.flb', retry in 10 seconds: task_id=0, input=dummy.0 > output=cloudwatch_logs.0 (out_id=0)
[2022/02/15 08:15:33] [ info] [output:cloudwatch_logs:cloudwatch_logs.0] Creating log stream from-fluent-bit-dummy in log group fluent-bit-is-the-best-cloudwatch-group-throttle-2
[2022/02/15 08:15:33] [error] [output:cloudwatch_logs:cloudwatch_logs.0] Log Group fluent-bit-is-the-best-cloudwatch-group-throttle-2 not found and `auto_create_group` disabled.
[2022/02/15 08:15:33] [ warn] [engine] failed to flush chunk '11589-1644912928.444537263.flb', retry in 6 seconds: task_id=1, input=dummy.0 > output=cloudwatch_logs.0 (out_id=0)

matthewfala · 2022-02-15T18:11:23Z

plugins/out_cloudwatch_logs/cloudwatch_api.c

+                            return -1;
+                        } else {
+                            /* retry stream creation */
+                            goto retry_create_stream;


Should there be a retry limit? This could be an infinite loop if the resource continues to fail.

What is the advantage of using goto statement here vs return create_log_stream(ctx, stream);? Is it for efficiency so the function stack doesn't need to be reinitialized?

For the second comment... the goto is just because I didn't think of making it recursive... can change it

For the infinite loop... can it? Because the call to create_stream only happens again if the CreateLogGroup succeeds but then create stream again fails with the resource not found exception. Which shouldn't be able to happen repeatedly.

The algo is:

Try to create stream.

If stream fails because group doesn't exist, then create the group. Otherwise, goto 4.

If group creation succeeds, goto 1. Otherwise, goto 4.

Exit

I think it can't be infinite unless I guess the CW API has a bug where it continually returns the same failure incorrectly... I could add a bool to make sure we only try once. But I'm not sure its necessary?

Good point. Makes sense.

Because the call to create_stream only happens again if the CreateLogGroup succeeds but then create stream again fails with the resource not found exception. Which shouldn't be able to happen repeatedly.

Just a suggestion - I think it might be better to add a bool here so that we are sure that the creation won't happen again and again. It might be very low possibility but just in case.

matthewfala · 2022-02-15T18:52:21Z

plugins/out_cloudwatch_logs/cloudwatch_api.c

+                            return -1;
+                        } else {
+                            /* retry stream creation */
+                            goto retry_create_stream;


Good point. Makes sense.

zhonghui12 · 2022-02-15T19:26:08Z

plugins/out_cloudwatch_logs/cloudwatch_api.c

+                            return -1;
+                        } else {
+                            /* retry stream creation */
+                            goto retry_create_stream;


Because the call to create_stream only happens again if the CreateLogGroup succeeds but then create stream again fails with the resource not found exception. Which shouldn't be able to happen repeatedly.

Just a suggestion - I think it might be better to add a bool here so that we are sure that the creation won't happen again and again. It might be very low possibility but just in case.

PettitWesley · 2022-02-16T03:17:19Z

@zhonghui12 @matthewfala Addressed comments.

matthewfala · 2022-02-16T21:43:21Z

plugins/out_cloudwatch_logs/cloudwatch_logs.c

@@ -391,14 +391,8 @@ static void cb_cloudwatch_flush(const void *data, size_t bytes,

    ctx->buf->put_events_calls = 0;

-    if (ctx->create_group == FLB_TRUE && ctx->group_created == FLB_FALSE) {
-        ret = create_log_group(ctx);


ret unused now. Causing tests to fail.

matthewfala · 2022-02-16T21:43:56Z

@zhonghui12 @matthewfala Addressed comments.

Great! That looks good.

…st to prevent throttling Signed-off-by: Wesley Pettit <wppttt@amazon.com>

Signed-off-by: Wesley Pettit <wppttt@amazon.com>

github-actions bot added the docs-required label Feb 15, 2022

PettitWesley mentioned this pull request Feb 15, 2022

out_cloudwatch_logs: Only create log group if it does not already exist #4827

Merged

6 tasks

matthewfala reviewed Feb 15, 2022

View reviewed changes

matthewfala approved these changes Feb 15, 2022

View reviewed changes

zhonghui12 approved these changes Feb 15, 2022

View reviewed changes

DrewZhang13 approved these changes Feb 15, 2022

View reviewed changes

PettitWesley force-pushed the create-log-group-fix branch from 632914b to 2860afe Compare February 16, 2022 03:16

matthewfala suggested changes Feb 16, 2022

View reviewed changes

PettitWesley force-pushed the create-log-group-fix branch from 2860afe to 14e7110 Compare February 17, 2022 02:22

out_cloudwatch_logs: Only create log group if it does not already exi…

152fe58

…st to prevent throttling Signed-off-by: Wesley Pettit <wppttt@amazon.com>

PettitWesley force-pushed the create-log-group-fix branch from 14e7110 to 152fe58 Compare March 1, 2022 04:49

PettitWesley merged commit d36af1f into fluent:1.8 Mar 1, 2022

PettitWesley added a commit to PettitWesley/fluent-bit that referenced this pull request Mar 2, 2022

out_cloudwatch_logs: fix build issue from fluent#4826

118163b

Signed-off-by: Wesley Pettit <wppttt@amazon.com>

PettitWesley added a commit that referenced this pull request Mar 2, 2022

out_cloudwatch_logs: fix build issue from #4826

360670a

Signed-off-by: Wesley Pettit <wppttt@amazon.com>

matthewfala mentioned this pull request Mar 24, 2022

Bump to 2.23.2 aws/aws-for-fluent-bit#319

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

out_cloudwatch_logs: Only create log group if it does not already exist #4826

out_cloudwatch_logs: Only create log group if it does not already exist #4826

PettitWesley commented Feb 15, 2022

PettitWesley commented Feb 15, 2022

matthewfala Feb 15, 2022

matthewfala Feb 15, 2022

PettitWesley Feb 15, 2022

matthewfala Feb 15, 2022

zhonghui12 Feb 15, 2022

matthewfala Feb 15, 2022

zhonghui12 Feb 15, 2022

PettitWesley commented Feb 16, 2022

matthewfala Feb 16, 2022

PettitWesley Feb 17, 2022

matthewfala commented Feb 16, 2022

out_cloudwatch_logs: Only create log group if it does not already exist #4826

out_cloudwatch_logs: Only create log group if it does not already exist #4826

Conversation

PettitWesley commented Feb 15, 2022

PettitWesley commented Feb 15, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

PettitWesley commented Feb 16, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

matthewfala commented Feb 16, 2022