out_logdna: stop duplicating promoted keys in line field - #12120
Conversation
Prefer a string message/log field as the Mezmo ingest "line" text. When neither is present, exclude promoted keys (meta, level/severity, app, file) from the line JSON body by default so they are not duplicated. Legacy full-record serialization remains available via exclude_promoted_keys false. Fixes fluent#11754 Signed-off-by: Dean Chen <862469039@qq.com>
📝 WalkthroughWalkthroughThe LogDNA output formatter now prefers string ChangesLogDNA line behavior
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Record
participant logdna_compose_payload
participant record_find_line_text
participant LogDNA payload
Record->>logdna_compose_payload: provide msgpack record
logdna_compose_payload->>record_find_line_text: search message then log
record_find_line_text-->>logdna_compose_payload: return text or no match
logdna_compose_payload->>LogDNA payload: pack text or serialized JSON line
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 65af712489
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (use_line_text) { | ||
| /* Plain text of the log line (Mezmo ingest "line" field) */ | ||
| msgpack_pack_str(&mp_pck, line_text->via.str.size); | ||
| msgpack_pack_str_body(&mp_pck, | ||
| line_text->via.str.ptr, | ||
| line_text->via.str.size); |
There was a problem hiding this comment.
For any record containing a string message/log plus non-primary fields, this branch emits only that text as line; record_append_primary_keys() has already packed only promoted fields into the outer object, so fields such as host and custom are silently dropped. This also happens with exclude_promoted_keys false, because use_line_text bypasses the legacy full-record branch. For example, the existing JSON_LEVEL_AND_SEVERITY fixture includes host, but the updated assertion no longer verifies it survives. Keep the remaining non-promoted fields in the emitted payload or make this lossy behavior explicitly opt-in.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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_logdna/logdna.c (1)
304-337: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winMissing NULL check on
flb_msgpack_to_json_str()result beforestrlen(); logic now duplicated across two branches.
line_jsonis used withstrlen(line_json)right afterflb_msgpack_to_json_str()in both the newbuild_line_mapbranch (317-326) and the legacy branch (329-335) without checking forNULL(e.g. on allocation failure), which would crash onstrlen(NULL). The pack/free sequence is now duplicated in two places; consider extracting a small helper (pack_json_str_as_line(mp_pck, msgpack_object, ...)) that also guards against aNULLreturn.🛡️ Proposed guard
line_json = flb_msgpack_to_json_str(1024, &mp_line_result.data, config->json_escape_unicode); msgpack_unpacked_destroy(&mp_line_result); msgpack_sbuffer_destroy(&mp_line_sbuf); - len = strlen(line_json); - msgpack_pack_str(&mp_pck, len); - msgpack_pack_str_body(&mp_pck, line_json, len); - flb_free(line_json); + if (line_json) { + len = strlen(line_json); + msgpack_pack_str(&mp_pck, len); + msgpack_pack_str_body(&mp_pck, line_json, len); + flb_free(line_json); + } + else { + msgpack_pack_str(&mp_pck, 0); + msgpack_pack_str_body(&mp_pck, "", 0); + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@plugins/out_logdna/logdna.c` around lines 304 - 337, Update the JSON serialization paths in the build_line_map and legacy branches to handle a NULL result from flb_msgpack_to_json_str before calling strlen, packing, or freeing it; preserve safe behavior on serialization failure. Remove the duplicated line_json packing sequence by extracting or reusing a helper that performs the NULL check and packs the serialized string into mp_pck.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@plugins/out_logdna/logdna.c`:
- Around line 304-337: Update the JSON serialization paths in the build_line_map
and legacy branches to handle a NULL result from flb_msgpack_to_json_str before
calling strlen, packing, or freeing it; preserve safe behavior on serialization
failure. Remove the duplicated line_json packing sequence by extracting or
reusing a helper that performs the NULL check and packs the serialized string
into mp_pck.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 10fea5f1-0b73-416e-b78d-930a766cffb9
📒 Files selected for processing (2)
plugins/out_logdna/logdna.ctests/runtime/out_logdna.c
1 participant
Summary
Fixes #11754.
The
out_logdnaplugin promotes primary keys (meta,level/severity,app,file) to top-level Mezmo ingest fields, but previously still serialized the full record intoline. That produced duplicates (e.g. top-levelmetaplus_metafrom the JSON insideline).This change:
message, thenlog, as the plain-textlinevalue when present as a string (matches Mezmo's "Text of the log line" semantics).exclude_promoted_keystotrue, so when there is nomessage/logtext field the remaining non-primary keys are serialized intolinewithout re-including promoted keys.linebehavior available withexclude_promoted_keys false(when no stringmessage/logis used).Example payload (with message + meta)
Before (duplication):
{ "lines": [{ "meta": {"source": "test"}, "level": "info", "app": "myapp", "timestamp": 12345678, "line": "{\"message\":\"hello world\",\"meta\":{\"source\":\"test\"},\"level\":\"info\",\"app\":\"myapp\"}" }] }After:
{ "lines": [{ "meta": {"source": "test"}, "level": "info", "app": "myapp", "timestamp": 12345678, "line": "hello world" }] }Testing
Environment did not have
cmakeavailable, so full compile/runtime tests were not run here. Runtime coverage is intests/runtime/out_logdna.c.Manual verification
Expected: all tests pass, including:
non_duplication— primary keys not re-serialized intoline;messageused as plain textlog_key_as_line/message_preferred_over_log—messagethenlogpreferencedata_completeness— without message/log, non-primary keys remain inlineJSON; promoted keys excludedbackward_compat—exclude_promoted_keys falsestill serializes full record when no message/log text fieldOptional end-to-end config
Capture the HTTP body (or use a local HTTP sink) and confirm
lineis"hello"andmeta/level/appappear only at the top level of each lines[] entry.Documentation
exclude_promoted_keysdocs should note the new default (true) and that a stringmessage/logfield is preferred forline.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
New Features
messagefield as plain-text line content when available.logfield whenmessageis absent.Bug Fixes
messageandlogfields.