out_azure_blob: fixes superfluous 400s for append blobs#11454
out_azure_blob: fixes superfluous 400s for append blobs#11454dennis-menge wants to merge 1 commit intofluent:masterfrom
Conversation
📝 WalkthroughWalkthroughThe block commit operation for Azure Blob Storage is now gated by two conditions instead of one. Commits execute only when blob_type equals AZURE_BLOB_BLOCKBLOB AND event_type equals FLB_EVENT_TYPE_LOGS. Previously, commits occurred for any Logs event regardless of blob type. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Minimal change affecting a single file with a straightforward conditional logic refinement. The modification is localized and easy to verify against intended behavior. Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Sample config: |
|
Sample output: |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
plugins/out_azure_blob/azure_blob.c (1)
526-531:⚠️ Potential issue | 🟠 MajorPre-existing: missing block commit after the
CREATE_BLOBretry path.When the first
http_send_blobreturnsCREATE_BLOB, the blob is created and the send is retried. If the retry succeeds (FLB_OK), the code does not commit the block for block blob logs — it falls through to cleanup. This means the uploaded block is never committed viaazb_block_blob_commit_block, effectively making it invisible.Suggested fix
else if (ret == CREATE_BLOB) { ret = create_blob(ctx, name); if (ret == FLB_OK) { ret = http_send_blob(config, ctx, ref_name, uri, block_id, event_type, payload_buf, payload_size); + if (ret == FLB_OK && + blob_type == AZURE_BLOB_BLOCKBLOB && + event_type == FLB_EVENT_TYPE_LOGS) { + ret = azb_block_blob_commit_block(ctx, block_id, tag, ms, generated_random_string); + } } }
Signed-off-by: Dennis Menge <dennismenge@microsoft.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
plugins/out_azure_blob/azure_blob.c (1)
526-531:⚠️ Potential issue | 🟠 MajorPre-existing: missing block commit after successful CREATE_BLOB retry for block blobs.
When
http_send_blobreturnsCREATE_BLOB, the blob is created and the send is retried on line 529. If the retry succeeds (ret == FLB_OK), the code falls through to line 532 without callingazb_block_blob_commit_block. For block blob + logs events, this means the block is uploaded but never committed.🔧 Suggested fix
else if (ret == CREATE_BLOB) { ret = create_blob(ctx, name); if (ret == FLB_OK) { ret = http_send_blob(config, ctx, ref_name, uri, block_id, event_type, payload_buf, payload_size); + if (ret == FLB_OK && blob_type == AZURE_BLOB_BLOCKBLOB && event_type == FLB_EVENT_TYPE_LOGS) { + ret = azb_block_blob_commit_block(ctx, block_id, tag, ms, generated_random_string); + } } }
This pull request refines the logic for committing blocks to Azure Blob Storage by ensuring that block commits are performed only for block blobs containing log events, rather than for all log events regardless of blob type.
Fixes #10439
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:
If this is a change to packaging of containers or native binaries then please confirm it works for all targets.
ok-package-testlabel to test for all targets (requires maintainer to do).Documentation
Backporting
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.
Summary by CodeRabbit