out_azure_blob: add log_key option #9791
Conversation
tomekwilk
requested review from
edsiper,
fujimotos,
koleini and
leonardo-albertovich
as code owners
|
@edsiper Can you please give us an update? |
|
memory leak test after rewrite: |
|
Hi @tomekwilk @lockewritesdocs, |
WalkthroughAdds optional msgpack log_key extraction to send only a single field value, updates formatter/control-flow and function signature to support this, exposes a Changes
Sequence Diagram(s)sequenceDiagram
participant In as Input
participant AZB as AzureBlob Plugin
participant Fmt as Formatter
participant AZ as Azure Blob Service
In->>AZB: Flush event (msgpack, tag, bytes)
AZB->>Fmt: azure_blob_format(config, ins, ctx, flush_ctx, event_type, tag, data, bytes)
alt log_key configured
Fmt->>Fmt: cb_azb_msgpack_extract_log_key -> locate field via record accessor
Fmt->>Fmt: Convert value to string/number, produce out_data/out_size
Note right of Fmt: Errors if missing/unsupported types
else
Fmt->>Fmt: Format record(s) as JSON lines -> out_data/out_size
end
Fmt-->>AZB: out_data, out_size
AZB->>AZ: Upload formatted payload
AZ-->>AZB: Response
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used🧬 Code graph analysis (2)plugins/out_azure_blob/azure_blob.c (4)
plugins/out_azure_blob/azure_blob_conf.c (1)
🔇 Additional comments (4)
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 |
|
I rebased the PR to resolve the merge conflicts after recent master changes. This PR is waiting to be re-reviewed and merged. Not sure if there is anything else for me to do. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
plugins/out_azure_blob/azure_blob.c (2)
34-35: Good: using Record Accessor APIs.Including flb_record_accessor.h and flb_ra_key.h aligns with prior guidance to avoid manual map walking.
70-75: Call flb_errno() before flb_plg_error() on RA creation failure.Swap the calls so errno is captured before logging.
Apply:
- if (!ra) { - flb_plg_error(ctx->ins, "invalid record accessor pattern '%s'", ctx->log_key); - flb_errno(); - return NULL; - } + if (!ra) { + flb_errno(); + flb_plg_error(ctx->ins, "invalid record accessor pattern '%s'", ctx->log_key); + return NULL; + }
🧹 Nitpick comments (2)
plugins/out_azure_blob/azure_blob.c (2)
177-186: Safer behavior: fallback to JSON when extraction yields no output.Avoid dropping data if log_key is missing/unsupported; gracefully fall back.
Apply:
- if (ctx->log_key) { - out_buf = cb_azb_msgpack_extract_log_key(ctx, data, bytes); - } - else { + if (ctx->log_key) { + out_buf = cb_azb_msgpack_extract_log_key(ctx, data, bytes); + if (!out_buf) { + flb_plg_warn(ctx->ins, "log_key='%s' yielded no data; falling back to JSON lines", ctx->log_key); + } + } + if (!out_buf) { out_buf = flb_pack_msgpack_to_json_format(data, bytes, FLB_PACK_JSON_FORMAT_LINES, FLB_PACK_JSON_DATE_ISO8601, ctx->date_key, config->json_escape_unicode); }
1897-1904: Clarify that log_key uses Record Accessor syntax.Config text says “key name,” but code uses record accessor. Recommend noting RA path examples (e.g.,
log,kubernetes['labels']['app']) to set user expectations. Also document newline-delimited output when multiple records are present.I can update the docs snippet accordingly if desired.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
plugins/out_azure_blob/azure_blob.c(6 hunks)plugins/out_azure_blob/azure_blob.h(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
plugins/out_azure_blob/azure_blob.c (4)
src/flb_record_accessor.c (3)
flb_ra_create(271-358)flb_ra_get_value_object(803-814)flb_ra_destroy(232-248)src/flb_sds.c (4)
flb_sds_create_size(92-95)flb_sds_copy(260-281)flb_sds_cat(120-141)flb_sds_printf(336-387)src/flb_ra_key.c (1)
flb_ra_key_value_destroy(842-851)src/flb_pack.c (1)
flb_pack_msgpack_to_json_format(1169-1450)
|
Hello @edsiper , @adrinaula , This PR tackles an issue that we've also recently faced. Would be interested to contribute if need be :) . Thanks in Advance, |
|
@tomekwilk Eduardo requested a change, can you take a look at fixing? |
Signed-off-by: Tomasz Wilk <[email protected]>
6 participants
This PR is based on PR #3668 but addresses Azure blob storage. The azure_blob plugin was modify to accept 'log_key' option. By default the entire log record is sent to storage. When 'log_key' option is specified in the output plugin configuration, then only the value of the key is sent to the storage blob.
Addresses #9721
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:
Documentation
Doc PR fluent/fluent-bit-docs#1540
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.
By default the entire record is sent to azure blob storage. Here is an example of a sample configuration and default output
Configuration
Record without log_key
{"@timestamp":"2025-01-02T16:56:02.906357Z","name":"Fluent Bit","year":2020}if the 'log_key' is specified then only the specific key value is sent to azure blob storage
Sample configuration with log_key
Record with log_key set to name
Fluent BitExample Valgrind output
Addresses #9721
Summary by CodeRabbit