out_logdna: add hostname as primary field and runtime tests#11517
Open
out_logdna: add hostname as primary field and runtime tests#11517
Conversation
Signed-off-by: Codex <codex@openai.com>
📝 WalkthroughWalkthroughThis PR adds hostname field support to the LogDNA output plugin, enabling hostname extraction from log records with automatic fallback to configured defaults. Comprehensive runtime tests validate both scenarios—hostname present in record and hostname from configuration. Changes
Poem
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 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 |
![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? |
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
Release Notes
New Features
Tests