-
Notifications
You must be signed in to change notification settings - Fork 1.8k
out_azure_blob: add path templating support comparable to s3 #11178
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.
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
base: master
Are you sure you want to change the base?
Conversation
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
WalkthroughThis pull request implements path templating for Azure Blob output, enabling support for format strings similar to Amazon S3. It introduces path resolution with UUID, time-token, and strftime placeholders, propagates path prefixes through blob URI construction, and adds comprehensive unit tests. Changes
Sequence Diagram(s)sequenceDiagram
participant send_blob as send_blob()
participant azb_resolve_path as azb_resolve_path()
participant path_funcs as Path Functions<br/>(UUID, Time, strftime)
participant uri_builder as URI Builders<br/>(append_blob_uri, etc.)
participant create_blob as create_blob()
send_blob->>azb_resolve_path: tag, timestamp
activate azb_resolve_path
azb_resolve_path->>path_funcs: Apply $UUID templates
azb_resolve_path->>path_funcs: Apply %3N/%9N/%L tokens
azb_resolve_path->>path_funcs: Apply strftime format
azb_resolve_path->>path_funcs: Trim slashes
path_funcs-->>azb_resolve_path: resolved path_prefix
deactivate azb_resolve_path
azb_resolve_path-->>send_blob: path_prefix
send_blob->>uri_builder: path_prefix, tag
activate uri_builder
uri_builder->>uri_builder: effective_path = path_prefix ?: ctx->path
uri_builder->>uri_builder: Build URI: /<effective_path>/<tag>
uri_builder-->>send_blob: URI
deactivate uri_builder
send_blob->>create_blob: path_prefix, blob_name
activate create_blob
create_blob-->>send_blob: success
deactivate create_blob
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Areas requiring extra attention:
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (11)
🚧 Files skipped from review as they are similar to previous changes (4)
🧰 Additional context used🧠 Learnings (7)📚 Learning: 2025-08-29T06:25:27.250ZApplied to files:
📚 Learning: 2025-08-29T06:24:55.855ZApplied to files:
📚 Learning: 2025-08-29T06:25:02.561ZApplied to files:
📚 Learning: 2025-08-31T12:46:11.940ZApplied to files:
📚 Learning: 2025-08-29T06:24:26.170ZApplied to files:
📚 Learning: 2025-08-29T06:25:27.250ZApplied to files:
📚 Learning: 2025-08-29T06:24:44.797ZApplied to files:
🧬 Code graph analysis (6)plugins/out_azure_blob/azure_blob_uri.h (1)
plugins/out_azure_blob/azure_blob_blockblob.h (1)
plugins/out_azure_blob/azure_blob.h (1)
plugins/out_azure_blob/azure_blob_appendblob.h (1)
plugins/out_azure_blob/azure_blob_blockblob.c (2)
plugins/out_azure_blob/azure_blob_uri.c (1)
🔇 Additional comments (13)
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (2)
plugins/out_azure_blob/azure_blob.c (2)
176-218: Only replaces the first$UUIDoccurrence.The
azb_replace_uuidfunction usesstrstrwhich finds only the first occurrence of$UUID. If a user specifies a path likelogs/$UUID/data/$UUID, only the first placeholder will be replaced.Consider whether multiple UUID placeholders should be supported. If so, a loop would be needed. If single replacement is intentional, document this limitation.
347-366: Minor:strftimereturns 0 for both buffer overflow and empty output.The loop relies on
strftimereturning 0 to indicate the buffer is too small. However,strftimealso returns 0 if the output is an empty string (e.g., if the path contains no format specifiers that produce output, though unlikely). The 4096-byte limit prevents infinite looping, so this is safe, but the behavior could be surprising.If you want to distinguish these cases more explicitly:
while (1) { out_len = strftime(buf, buf_size + 1, path, &tm_utc); - if (out_len > 0) { + if (out_len > 0 || buf_size > flb_sds_len(path) * 2) { break; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
plugins/out_azure_blob/azure_blob.c(15 hunks)plugins/out_azure_blob/azure_blob.h(3 hunks)plugins/out_azure_blob/azure_blob_appendblob.c(1 hunks)plugins/out_azure_blob/azure_blob_appendblob.h(1 hunks)plugins/out_azure_blob/azure_blob_blockblob.c(7 hunks)plugins/out_azure_blob/azure_blob_blockblob.h(1 hunks)plugins/out_azure_blob/azure_blob_conf.c(3 hunks)plugins/out_azure_blob/azure_blob_uri.c(1 hunks)plugins/out_azure_blob/azure_blob_uri.h(1 hunks)tests/internal/CMakeLists.txt(2 hunks)tests/internal/azure_blob_path.c(1 hunks)
🧰 Additional context used
🧠 Learnings (7)
📚 Learning: 2025-08-31T12:46:11.940Z
Learnt from: ThomasDevoogdt
Repo: fluent/fluent-bit PR: 9277
File: .github/workflows/pr-compile-check.yaml:147-151
Timestamp: 2025-08-31T12:46:11.940Z
Learning: In fluent-bit CMakeLists.txt, the system library preference flags are defined as FLB_PREFER_SYSTEM_LIB_ZSTD and FLB_PREFER_SYSTEM_LIB_KAFKA with the FLB_ prefix.
Applied to files:
tests/internal/CMakeLists.txtplugins/out_azure_blob/azure_blob.h
📚 Learning: 2025-08-29T06:25:02.561Z
Learnt from: shadowshot-x
Repo: fluent/fluent-bit PR: 10794
File: tests/internal/aws_compress.c:7-7
Timestamp: 2025-08-29T06:25:02.561Z
Learning: In Fluent Bit, ZSTD (zstandard) compression library is bundled directly in the source tree at `lib/zstd-1.5.7` and is built unconditionally as a static library. Unlike optional external dependencies, ZSTD does not use conditional compilation guards like `FLB_HAVE_ZSTD` and is always available. Headers like `<fluent-bit/flb_zstd.h>` can be included directly without guards.
Applied to files:
plugins/out_azure_blob/azure_blob_conf.cplugins/out_azure_blob/azure_blob.h
📚 Learning: 2025-08-29T06:25:27.250Z
Learnt from: shadowshot-x
Repo: fluent/fluent-bit PR: 10794
File: tests/internal/aws_compress.c:93-107
Timestamp: 2025-08-29T06:25:27.250Z
Learning: In Fluent Bit, ZSTD compression is enabled by default and is treated as a core dependency, not requiring conditional compilation guards like `#ifdef FLB_HAVE_ZSTD`. Unlike some other optional components such as ARROW/PARQUET (which use `#ifdef FLB_HAVE_ARROW` guards), ZSTD support is always available and doesn't need build-time conditionals. ZSTD headers are included directly without guards across multiple plugins and core components.
Applied to files:
plugins/out_azure_blob/azure_blob_conf.cplugins/out_azure_blob/azure_blob.h
📚 Learning: 2025-08-29T06:24:55.855Z
Learnt from: shadowshot-x
Repo: fluent/fluent-bit PR: 10794
File: src/aws/flb_aws_compress.c:52-56
Timestamp: 2025-08-29T06:24:55.855Z
Learning: ZSTD compression is always available in Fluent Bit and does not require conditional compilation guards. Unlike Arrow/Parquet which use #ifdef FLB_HAVE_ARROW guards, ZSTD is built unconditionally with flb_zstd.c included directly in src/CMakeLists.txt and a bundled ZSTD library at lib/zstd-1.5.7/.
Applied to files:
plugins/out_azure_blob/azure_blob.h
📚 Learning: 2025-08-29T06:24:26.170Z
Learnt from: shadowshot-x
Repo: fluent/fluent-bit PR: 10794
File: tests/internal/aws_compress.c:39-42
Timestamp: 2025-08-29T06:24:26.170Z
Learning: In Fluent Bit, ZSTD compression support is enabled by default and does not require conditional compilation guards (like #ifdef FLB_HAVE_ZSTD) around ZSTD-related code declarations and implementations.
Applied to files:
plugins/out_azure_blob/azure_blob.h
📚 Learning: 2025-08-29T06:25:27.250Z
Learnt from: shadowshot-x
Repo: fluent/fluent-bit PR: 10794
File: tests/internal/aws_compress.c:93-107
Timestamp: 2025-08-29T06:25:27.250Z
Learning: In Fluent Bit, ZSTD compression is enabled by default and is treated as a core dependency, not requiring conditional compilation guards like `#ifdef FLB_HAVE_ZSTD`. Unlike some other optional components, ZSTD support is always available and doesn't need build-time conditionals.
Applied to files:
plugins/out_azure_blob/azure_blob.h
📚 Learning: 2025-08-29T06:24:44.797Z
Learnt from: shadowshot-x
Repo: fluent/fluent-bit PR: 10794
File: src/aws/flb_aws_compress.c:26-26
Timestamp: 2025-08-29T06:24:44.797Z
Learning: In Fluent Bit, ZSTD support is always available and enabled by default. The build system automatically detects and uses either the system libzstd library or builds the bundled ZSTD version. Unlike other optional dependencies like Arrow which use conditional compilation guards (e.g., FLB_HAVE_ARROW), ZSTD does not require conditional includes or build flags.
Applied to files:
plugins/out_azure_blob/azure_blob.h
🧬 Code graph analysis (8)
plugins/out_azure_blob/azure_blob_uri.h (1)
plugins/out_azure_blob/azure_blob_uri.c (1)
azb_uri_create_blob(137-163)
tests/internal/azure_blob_path.c (4)
src/flb_sds.c (2)
flb_sds_destroy(389-399)flb_sds_create(78-90)src/flb_record_accessor.c (2)
flb_ra_destroy(232-248)flb_ra_create(271-358)plugins/out_azure_blob/azure_blob.c (1)
azb_resolve_path(412-518)include/fluent-bit/flb_time.h (1)
flb_time_set(75-79)
plugins/out_azure_blob/azure_blob_conf.c (1)
src/flb_record_accessor.c (2)
flb_ra_create(271-358)flb_ra_destroy(232-248)
plugins/out_azure_blob/azure_blob_appendblob.c (2)
plugins/out_azure_blob/azure_blob_uri.c (1)
azb_uri_container(107-118)src/flb_sds.c (1)
flb_sds_printf(336-387)
plugins/out_azure_blob/azure_blob_uri.c (1)
src/flb_sds.c (1)
flb_sds_printf(336-387)
plugins/out_azure_blob/azure_blob_blockblob.c (2)
plugins/out_azure_blob/azure_blob_uri.c (1)
azb_uri_container(107-118)src/flb_sds.c (1)
flb_sds_printf(336-387)
plugins/out_azure_blob/azure_blob_appendblob.h (1)
plugins/out_azure_blob/azure_blob_appendblob.c (1)
azb_append_blob_uri(27-53)
plugins/out_azure_blob/azure_blob.h (1)
plugins/out_azure_blob/azure_blob.c (1)
azb_resolve_path(412-518)
🔇 Additional comments (32)
plugins/out_azure_blob/azure_blob.h (1)
27-27: LGTM!The header additions are well-structured:
- The
flb_time.hinclude is required for thestruct flb_timeparameter inazb_resolve_path.- The
path_accessorfield enables tag expansion via record accessor.- The function declaration correctly matches the implementation signature.
Also applies to: 66-66, 171-176
tests/internal/CMakeLists.txt (1)
62-67: LGTM!The CMake integration follows the project's established patterns:
- Conditional inclusion of test file when
FLB_OUT_AZURE_BLOBis enabled mirrors other plugin-specific tests.- Library linking is correctly scoped to only the
flb-it-azure_blob_pathtest target.Also applies to: 227-229
plugins/out_azure_blob/azure_blob_appendblob.h (1)
26-28: LGTM!The function signature update correctly adds
path_prefixparameter to support path templating. Theconst char *type forpath_prefixis appropriate since it's read-only input.plugins/out_azure_blob/azure_blob_conf.c (1)
816-819: LGTM!The cleanup logic correctly destroys the path accessor and nullifies the pointer, following the established pattern for other resources in this function.
plugins/out_azure_blob/azure_blob_uri.h (1)
30-32: LGTM!The updated function signature correctly adds the
path_prefixparameter, consistent with the implementation and the parallel changes inazure_blob_appendblob.h.plugins/out_azure_blob/azure_blob_appendblob.c (1)
27-49: LGTM!The implementation correctly:
- Selects
effective_pathpreferringpath_prefixoverctx->pathwhen provided and non-empty.- Constructs the URI with proper path/tag structure for append blob operations.
- Maintains existing SAS token handling.
The
flb_sds_printfreturn value is not checked, but this follows the existing pattern in the codebase for this function.tests/internal/azure_blob_path.c (7)
13-24: LGTM!The
ctx_cleanuphelper properly handles resource cleanup for bothpathandpath_accessor, with NULL checks before destruction and pointer nullification after.
26-58: LGTM!Good test coverage for basic
$TAGsubstitution with proper setup, assertion, and cleanup flow.
60-94: LGTM!Excellent test for
$TAG[n]indexed tag segment extraction with delimiter parsing.
96-131: LGTM!Comprehensive test for time-based tokens (
%Y,%m,%d,%3N,%9N,%L) using a fixed timestamp for deterministic verification.
133-177: LGTM!Good test for UUID token substitution with validation that:
- The UUID prefix is preserved.
- The generated value is 8 alphanumeric characters.
- No
$UUIDliteral remains in output.
179-209: LGTM!Important edge case test verifying that out-of-bounds tag index (
$TAG[5]with only 2 tag segments) correctly returnsret == 0withresolved == NULL.
211-217: LGTM!Test registration is complete and properly null-terminated.
plugins/out_azure_blob/azure_blob_uri.c (1)
137-162: LGTM!The implementation correctly:
- Adds
path_prefixparameter to enable dynamic path resolution.- Uses the same
effective_pathselection logic asazb_append_blob_urifor consistency.- Properly handles SAS token appending with
?(vs&in appendblob which already has query params).plugins/out_azure_blob/azure_blob_blockblob.h (1)
26-43: LGTM! Consistent API extension for path templating support.The function signatures are updated consistently to accept the new
path_prefixparameter with appropriateconstqualification. The parameter ordering is uniform across all modified functions.plugins/out_azure_blob/azure_blob_blockblob.c (5)
34-61: LGTM! Clean effective_path fallback logic.The function correctly implements the path_prefix fallback pattern: if
path_prefixis provided and non-empty, use it; otherwise fall back toctx->path. The dual check for both NULL and empty string is appropriate.
63-124: LGTM! Proper resource management and path handling.The function correctly handles the path_prefix parameter with proper fallback logic and ensures
encoded_blockidis freed before return in all code paths.
126-165: LGTM! Consistent implementation.The
azb_block_blob_uri_commitfunction follows the same effective_path pattern as other functions in this file.
354-393: LGTM! Proper path_prefix propagation.The function correctly forwards the
path_prefixparameter to the internalazb_block_blob_uri_commitcall.
447-451: LGTM! Correct NULL path_prefix for file parts.Passing
NULLaspath_prefixis appropriate here since file parts use the pre-determinedpathstored in the database rather than dynamically resolved templated paths.plugins/out_azure_blob/azure_blob.c (12)
220-267: LGTM with same single-replacement caveat.The helper correctly handles NULL inputs and memory management. Like
azb_replace_uuid, it only replaces the first occurrence of a token, which is acceptable for the time tokens (%3N,%9N,%L) that are typically used once per path.
269-305: LGTM! Correct sequential token application.The function properly chains time token replacements and handles failures appropriately.
382-410: LGTM! Correct in-place slash trimming.The function properly uses
memmovefor overlapping memory regions and correctly updates the SDS length. Edge cases (NULL input, all-slash paths) are handled.
412-518: LGTM! Well-structured path resolution pipeline.The function implements a clean multi-stage path resolution:
- UUID replacement (before record accessor to preserve
$tokens)- Time token replacement (
%3N,%9N,%L)- strftime formatting
- Record accessor expansion for
$TAGand$TAG[n]- Slash trimming
Memory management is correct across all error paths.
520-542: LGTM! Clean wrapper for tag-based blob creation.The function properly wraps path resolution with blob creation. The defensive NULL check before
flb_sds_destroy(prefix)at lines 529-531 is good practice, even thoughazb_resolve_pathinitializes*out_pathto NULL on entry.
544-555: LGTM! Properly propagates path_prefix.The function correctly forwards the
path_prefixparameter toazb_uri_create_blob.
624-632: LGTM! Correct NULL path_prefix for delete operation.Delete operations target existing blobs by name rather than templated paths, so passing
NULLis appropriate.
836-946: LGTM! Comprehensive path_prefix handling in send_blob.The function correctly:
- Resolves the path at the start using current timestamp
- Propagates
path_prefixto all URI construction and blob operation functions- Cleans up
path_prefixin all exit paths (success, error, and retry)
1663-1670: LGTM! Consistent use of tag-aware blob creation.The callback correctly uses
azb_create_blob_with_tagto ensure path templating is applied when creating blobs.
1794-1800: LGTM! Path templating in chunk ingestion.Consistent with other code paths, this uses
azb_create_blob_with_tagfor path-aware blob creation.
2052-2065: LGTM! Path templating in non-buffered flush.The non-buffered flush path correctly uses
azb_create_blob_with_tagfor consistent path templating behavior.
433-435: path_accessor initialization is correctly implemented.Verification confirms the code works as intended:
path_accessoris conditionally created inazure_blob_conf.c:766when a path template is configured, with proper error handling (lines 767-769). The early return at line 433 is appropriate—if no path template is configured, the function returns 0 (success) with*out_path = NULL, which is the correct behavior.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
plugins/out_azure_blob/azure_blob.c(16 hunks)plugins/out_azure_blob/azure_blob.h(3 hunks)plugins/out_azure_blob/azure_blob_appendblob.c(1 hunks)plugins/out_azure_blob/azure_blob_appendblob.h(1 hunks)plugins/out_azure_blob/azure_blob_blockblob.c(7 hunks)plugins/out_azure_blob/azure_blob_blockblob.h(1 hunks)plugins/out_azure_blob/azure_blob_conf.c(3 hunks)plugins/out_azure_blob/azure_blob_uri.c(1 hunks)plugins/out_azure_blob/azure_blob_uri.h(1 hunks)tests/internal/CMakeLists.txt(2 hunks)tests/internal/azure_blob.c(1 hunks)tests/internal/azure_blob_path.c(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- tests/internal/CMakeLists.txt
- plugins/out_azure_blob/azure_blob_conf.c
- plugins/out_azure_blob/azure_blob.h
- tests/internal/azure_blob_path.c
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-08-29T06:25:27.250Z
Learnt from: shadowshot-x
Repo: fluent/fluent-bit PR: 10794
File: tests/internal/aws_compress.c:93-107
Timestamp: 2025-08-29T06:25:27.250Z
Learning: In Fluent Bit, ZSTD compression is enabled by default and is treated as a core dependency, not requiring conditional compilation guards like `#ifdef FLB_HAVE_ZSTD`. Unlike some other optional components such as ARROW/PARQUET (which use `#ifdef FLB_HAVE_ARROW` guards), ZSTD support is always available and doesn't need build-time conditionals. ZSTD headers are included directly without guards across multiple plugins and core components.
Applied to files:
tests/internal/azure_blob.c
📚 Learning: 2025-08-29T06:24:55.855Z
Learnt from: shadowshot-x
Repo: fluent/fluent-bit PR: 10794
File: src/aws/flb_aws_compress.c:52-56
Timestamp: 2025-08-29T06:24:55.855Z
Learning: ZSTD compression is always available in Fluent Bit and does not require conditional compilation guards. Unlike Arrow/Parquet which use #ifdef FLB_HAVE_ARROW guards, ZSTD is built unconditionally with flb_zstd.c included directly in src/CMakeLists.txt and a bundled ZSTD library at lib/zstd-1.5.7/.
Applied to files:
tests/internal/azure_blob.c
🧬 Code graph analysis (5)
plugins/out_azure_blob/azure_blob_appendblob.c (1)
src/flb_sds.c (1)
flb_sds_printf(336-387)
plugins/out_azure_blob/azure_blob_appendblob.h (1)
plugins/out_azure_blob/azure_blob_appendblob.c (1)
azb_append_blob_uri(27-53)
tests/internal/azure_blob.c (6)
src/flb_config.c (2)
flb_config_init(233-486)flb_config_exit(488-672)src/flb_upstream.c (1)
flb_upstream_create(290-363)src/flb_sds.c (2)
flb_sds_create(78-90)flb_sds_destroy(389-399)plugins/out_azure_blob/azure_blob_blockblob.c (1)
azb_block_blob_uri(63-124)src/flb_http_client.c (3)
flb_http_client(814-859)flb_http_get_header(1002-1019)flb_http_client_destroy(1688-1695)plugins/out_azure_blob/azure_blob_http.c (1)
azb_http_client_setup(293-366)
plugins/out_azure_blob/azure_blob_uri.c (1)
src/flb_sds.c (1)
flb_sds_printf(336-387)
plugins/out_azure_blob/azure_blob.c (3)
plugins/out_azure_blob/azure_blob_uri.c (1)
azb_uri_create_blob(137-163)plugins/out_azure_blob/azure_blob_appendblob.c (1)
azb_append_blob_uri(27-53)plugins/out_azure_blob/azure_blob_blockblob.c (1)
azb_block_blob_uri(63-124)
🔇 Additional comments (21)
plugins/out_azure_blob/azure_blob_appendblob.h (1)
26-28: LGTM: API signature updated for path templating support.The addition of
path_prefixparameter aligns with the broader path templating changes across the Azure Blob module.Minor note: Lines 27-28 use tabs for indentation while the codebase generally uses 4-space indentation. This is a cosmetic issue that can be addressed if desired.
plugins/out_azure_blob/azure_blob_uri.h (1)
30-32: LGTM: Header declaration updated for path_prefix support.The signature change is consistent with the implementation and other URI-building functions in this PR.
plugins/out_azure_blob/azure_blob_appendblob.c (1)
27-46: LGTM: Path prefix logic correctly implemented.The
effective_pathcomputation properly falls back toctx->pathwhenpath_prefixis NULL or empty, maintaining backward compatibility while enabling path templating.plugins/out_azure_blob/azure_blob_uri.c (1)
137-162: LGTM: Consistent path_prefix handling in azb_uri_create_blob.The implementation follows the same pattern as
azb_append_blob_uri, properly computingeffective_pathand using it for URI construction with appropriate SAS token handling.tests/internal/azure_blob.c (1)
42-89: Test fixture setup and teardown are well-structured.The fixture properly initializes and cleans up resources (config, upstream, connection) in the correct order, with appropriate error handling during initialization.
plugins/out_azure_blob/azure_blob_blockblob.h (1)
26-43: LGTM: Block blob function signatures updated for path templating.All three function declarations (
azb_block_blob_blocklist_uri,azb_block_blob_uri,azb_block_blob_commit_block) consistently add thepath_prefixparameter, aligning with the implementation and broader path templating support.plugins/out_azure_blob/azure_blob_blockblob.c (4)
34-61: LGTM: Consistent path_prefix handling in blocklist URI builder.The
effective_pathpattern is correctly applied, with proper fallback toctx->pathwhenpath_prefixis NULL or empty.
63-124: LGTM: Block blob URI construction properly updated.The function correctly handles both path-prefixed and non-prefixed cases, maintaining all existing functionality including timestamp handling, compression extensions, and SAS token appending.
354-366: LGTM: Commit block function properly propagates path_prefix.The
path_prefixparameter is correctly passed through toazb_block_blob_uri_commit.
447-447: Correct: NULL path_prefix for file parts commit.Passing NULL here is appropriate since file parts committed via
azb_block_blob_commit_file_partsuse the storedpathfromctxrather than dynamically resolved templating.plugins/out_azure_blob/azure_blob.c (11)
184-254: LGTM: UUID replacement implementation is correct.The function properly:
- Counts occurrences before allocation
- Returns the original path unchanged if no $UUID tokens exist (line 207)
- Correctly calculates result length accounting for size difference between token and replacement
- Properly frees the original path and returns the new SDS string
260-306: LGTM: Token replacement helper is well-implemented.The function correctly handles edge cases (NULL input, token not found) and properly manages memory by destroying the input SDS and returning a new one.
311-347: LGTM: Time token expansion correctly chains replacements.The function properly chains
azb_simple_replacecalls, which handle memory cleanup of intermediate SDS strings.
352-433: LGTM: strftime application with proper buffer growth.The implementation correctly handles buffer sizing with safeguards against infinite loops (empty_threshold and 4096 byte limit). Memory is properly freed in all code paths.
438-466: LGTM: Slash trimming correctly modifies SDS in-place.The function properly uses
memmovefor potentially overlapping memory regions and updates the SDS length metadata.
471-577: LGTM: Path resolution function with proper resource cleanup.The function correctly handles:
- Early return when templating is disabled
- Fallback to current time when no timestamp provided
- Sequential application of UUID, time tokens, strftime, and record accessor
- Proper cleanup of msgpack and record accessor resources on all paths
579-601: LGTM: Blob creation wrapper with path resolution.The function properly resolves the path prefix and cleans up on failure. Note that
flb_sds_destroyalready handles NULL safely, so the NULL check at line 588 is defensive but harmless.
895-1004: LGTM: send_blob properly manages path_prefix lifecycle.The function correctly:
- Resolves path_prefix at the beginning using the event timestamp
- Passes path_prefix through all URI building and blob creation calls
- Cleans up path_prefix on all exit paths (early returns and final cleanup)
691-691: Correct: delete_blob passes NULL for path_prefix.Deletion uses the stored path from context rather than dynamically resolved templating, so NULL is appropriate here.
1722-1724: LGTM: Ingest callback uses azb_create_blob_with_tag.The callback correctly uses the new wrapper function to resolve the path template when creating blobs.
2111-2113: LGTM: Flush callback uses azb_create_blob_with_tag.Consistent integration with the path templating system.
fixes fluent#7515 Signed-off-by: Nico Berlee <nico.berlee@on2it.net>
fixes #7515
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:
see valgrind log
valgrind.log
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
Release Notes
New Features
Tests
✏️ Tip: You can customize this high-level summary in your review settings.