out_logdna: add hostname as primary field and runtime tests - #11517
Open
agup006 wants to merge 1 commit into
Open
out_logdna: add hostname as primary field and runtime tests#11517agup006 wants to merge 1 commit into
agup006 wants to merge 1 commit into
Conversation
📝 WalkthroughWalkthroughThe LogDNA output plugin now promotes record-level ChangesLogDNA hostname handling
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
🧪 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 |
agup006
marked this pull request as ready for review
March 3, 2026 21:40
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/runtime/out_logdna.c (1)
58-68: Assert setup/push return values to prevent false-positive passes.The tests currently don’t verify several API call results (
flb_service_set,flb_input_set,flb_output_set,flb_lib_push, and ffd creation). If one fails, the test can pass without actually validating behavior.Proposed hardening diff
void flb_test_record_hostname_field() { int ret; @@ ctx = flb_create(); - flb_service_set(ctx, "flush", "1", "grace", "1", NULL); + TEST_CHECK(ctx != NULL); + ret = flb_service_set(ctx, "flush", "1", "grace", "1", NULL); + TEST_CHECK(ret == 0); in_ffd = flb_input(ctx, (char *) "lib", NULL); - flb_input_set(ctx, in_ffd, "tag", "test", NULL); + TEST_CHECK(in_ffd >= 0); + ret = flb_input_set(ctx, in_ffd, "tag", "test", NULL); + TEST_CHECK(ret == 0); out_ffd = flb_output(ctx, (char *) "logdna", NULL); - flb_output_set(ctx, out_ffd, - "match", "test", - "api_key", "dummy-api-key", - NULL); + TEST_CHECK(out_ffd >= 0); + ret = flb_output_set(ctx, out_ffd, + "match", "test", + "api_key", "dummy-api-key", + NULL); + TEST_CHECK(ret == 0); @@ - flb_lib_push(ctx, in_ffd, - (char *) "[12345678, {\"hostname\":\"record-host\",\"message\":\"hello\"}]", - size); + ret = flb_lib_push(ctx, in_ffd, + (char *) "[12345678, {\"hostname\":\"record-host\",\"message\":\"hello\"}]", + size); + TEST_CHECK(ret > 0); @@ void flb_test_default_hostname_field() { int ret; @@ ctx = flb_create(); - flb_service_set(ctx, "flush", "1", "grace", "1", NULL); + TEST_CHECK(ctx != NULL); + ret = flb_service_set(ctx, "flush", "1", "grace", "1", NULL); + TEST_CHECK(ret == 0); in_ffd = flb_input(ctx, (char *) "lib", NULL); - flb_input_set(ctx, in_ffd, "tag", "test", NULL); + TEST_CHECK(in_ffd >= 0); + ret = flb_input_set(ctx, in_ffd, "tag", "test", NULL); + TEST_CHECK(ret == 0); out_ffd = flb_output(ctx, (char *) "logdna", NULL); - flb_output_set(ctx, out_ffd, - "match", "test", - "api_key", "dummy-api-key", - "hostname", "config-host", - NULL); + TEST_CHECK(out_ffd >= 0); + ret = flb_output_set(ctx, out_ffd, + "match", "test", + "api_key", "dummy-api-key", + "hostname", "config-host", + NULL); + TEST_CHECK(ret == 0); @@ - flb_lib_push(ctx, in_ffd, - (char *) "[12345678, {\"message\":\"hello\"}]", - size); + ret = flb_lib_push(ctx, in_ffd, + (char *) "[12345678, {\"message\":\"hello\"}]", + size); + TEST_CHECK(ret > 0);Also applies to: 78-80, 95-107, 116-118
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/runtime/out_logdna.c` around lines 58 - 68, The test doesn't check return values and can false-pass; add assertions after each creation/config call to fail the test on error: verify ctx from flb_create is non-NULL, verify in_ffd/out_ffd from flb_input and flb_output are non-NULL, and assert flb_service_set, flb_input_set, flb_output_set and flb_lib_push return success (non-error/expected return code) so failures abort the test; update all occurrences around flb_create, flb_service_set, flb_input, flb_input_set, flb_output, flb_output_set and flb_lib_push (including the other ranges noted) to check and handle their return values.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@tests/runtime/out_logdna.c`:
- Around line 58-68: The test doesn't check return values and can false-pass;
add assertions after each creation/config call to fail the test on error: verify
ctx from flb_create is non-NULL, verify in_ffd/out_ffd from flb_input and
flb_output are non-NULL, and assert flb_service_set, flb_input_set,
flb_output_set and flb_lib_push return success (non-error/expected return code)
so failures abort the test; update all occurrences around flb_create,
flb_service_set, flb_input, flb_input_set, flb_output, flb_output_set and
flb_lib_push (including the other ranges noted) to check and handle their return
values.
Contributor
|
The patch seems good but DCO complains Signed-off glitch: Could you use the correct Signed-off signature? |
Contributor
|
This PR is stale because it has been open 45 days with no activity. Remove stale label or comment or this will be closed in 10 days. |
agup006
force-pushed
the
codex/add-hostname-field-to-fluentbit-log-dna-plugin
branch
from
August 10, 2026 17:08
bb6e91e to
a55bd63
Compare
Promote a record's "hostname" key to a top-level field, falling back to the hostname resolved at configuration time when the record does not carry one. This follows the same handling as the other primary keys, so with exclude_promoted_keys enabled the value is not duplicated into the line body. Signed-off-by: Anurag Gupta <agup006@gmail.com>
agup006
force-pushed
the
codex/add-hostname-field-to-fluentbit-log-dna-plugin
branch
from
August 10, 2026 17:26
a55bd63 to
118f3ed
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/runtime/out_logdna.c (1)
758-758: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse the required control-statement brace style.
Move each
ifopening brace to the next line.
tests/runtime/out_logdna.c#L758-L758: move theifopening brace to the next line.tests/runtime/out_logdna.c#L802-L802: move theifopening brace to the next line.tests/runtime/out_logdna.c#L820-L820: move theifopening brace to the next line.tests/runtime/out_logdna.c#L865-L865: move theifopening brace to the next line.tests/runtime/out_logdna.c#L883-L883: move theifopening brace to the next line.tests/runtime/out_logdna.c#L887-L887: move theifopening brace to the next line.tests/runtime/out_logdna.c#L891-L891: move theifopening brace to the next line.tests/runtime/out_logdna.c#L936-L936: move theifopening brace to the next line.🤖 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 `@tests/runtime/out_logdna.c` at line 758, In tests/runtime/out_logdna.c, update the control-statement brace style for the if statements at lines 758, 802, 820, 865, 883, 887, 891, and 936 by placing each opening brace on the following line; preserve all conditions and bodies unchanged.Source: Coding guidelines
🤖 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.
Nitpick comments:
In `@tests/runtime/out_logdna.c`:
- Line 758: In tests/runtime/out_logdna.c, update the control-statement brace
style for the if statements at lines 758, 802, 820, 865, 883, 887, 891, and 936
by placing each opening brace on the following line; preserve all conditions and
bodies unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 0599adaa-37ea-44ee-82e8-2a461043e5ed
📒 Files selected for processing (2)
plugins/out_logdna/logdna.ctests/runtime/out_logdna.c
🚧 Files skipped from review as they are similar to previous changes (1)
- plugins/out_logdna/logdna.c
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
hostnamelike other well-known primary fields so a record-levelhostnamecan be used when present.hostname.Description
record_append_primary_keysinplugins/out_logdna/logdna.cto recognize thehostnamekey in records and append it to the payload when present.hostname, inject the plugin-configured/environment hostname fromctx->_hostnameinto the record payload.tests/runtime/out_logdna.cwith two cases:record_hostname_field(verifies record-level hostname passthrough) anddefault_hostname_field(verifies default hostname injection from output config).tests/runtime/CMakeLists.txtso the test binaryflb-rt-out_logdnais built with runtime tests enabled.Testing
cmake -DFLB_DEV=On -DFLB_TESTS_RUNTIME=On -DFLB_TESTS_INTERNAL=On ../and built the runtime test targetflb-rt-out_logdnasuccessfully.bin/flb-rt-out_logdna, which ran both test cases and reported success.valgrind --leak-check=full bin/flb-rt-out_logdnaand observed no memory leaks or errors (all tests passed).Codex Task
Summary by CodeRabbit
New Features
Tests