out_datadog: added support for site configuration field #10688
Conversation
lucastemb
force-pushed
the
lucastemb/AGNTLOG-199
branch
from
7048281 to
b73661e
Compare
WalkthroughAdds a new optional "site" string configuration to the Datadog output plugin. The struct gains a site field. During config creation, if site is set and Host is not, the host is built as "http-intake.logs."; otherwise existing Host/default logic applies. Memory management and debug logs are updated accordingly. Changes
Sequence Diagram(s)sequenceDiagram
participant User Config
participant Datadog Conf Create
participant Context (ctx)
User Config->>Datadog Conf Create: Initialize with parameters
Datadog Conf Create->>Context (ctx): Store site (if provided)
alt Explicit Host provided
Datadog Conf Create->>Context (ctx): Use provided Host
else Site provided, no Host
Datadog Conf Create->>Context (ctx): Build Host = "http-intake.logs." + site
else Neither Host nor Site
Datadog Conf Create->>Context (ctx): Use default Host
end
Datadog Conf Create-->>User Config: Return ctx
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: CodeRabbit UI 💡 Knowledge Base configuration:
You can enable these settings in your CodeRabbit configuration. 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
|
Example Config File: [SERVICE]
Flush 1
Log_Level trace
Parsers_File parsers.conf
[INPUT]
Name dummy
Tag dummy
Dummy {"message": "Testing support for site confiruation ", "test": "AGNTLOG-206", "container_id": "abc123def456", "container_name": "/ecs-web-service", "container_image": "nginx:1.24-alpine", "ecs_cluster": "arn:aws:ecs:us-west-2:123456789012:cluster/production-cluster", "ecs_task_definition": "web-service:42", "ecs_task_arn": "arn:aws:ecs:us-west-2:123456789012:task/production-cluster/abc123def456-7890-1234-5678-901234567890"}
[OUTPUT]
Name datadog
Match *
apikey ${DD_API_KEY}
dd_hostname foobar
provider ecs
dd_source fluent-bit
dd_service test-service
dd_tags env:test,version:1.0
compress gzip
site datadoghq.com``` |
|
Valgrind Output: ==1403772==
==1403772== HEAP SUMMARY:
==1403772== in use at exit: 0 bytes in 0 blocks
==1403772== total heap usage: 3,982 allocs, 3,982 frees, 2,807,105 bytes allocated
==1403772==
==1403772== All heap blocks were freed -- no leaks are possible
==1403772==
==1403772== Use --track-origins=yes to see where uninitialised values come from
==1403772== For lists of detected and suppressed errors, rerun with: -s
==1403772== ERROR SUMMARY: 17 errors from 5 contexts (suppressed: 0 from 0) |
|
Debug Log Output: [2025/08/04 15:13:48] [ info] Configuration:
[2025/08/04 15:13:48] [ info] flush time | 1.000000 seconds
[2025/08/04 15:13:48] [ info] grace | 5 seconds
[2025/08/04 15:13:48] [ info] daemon | 0
[2025/08/04 15:13:48] [ info] ___________
[2025/08/04 15:13:48] [ info] inputs:
[2025/08/04 15:13:48] [ info] dummy
[2025/08/04 15:13:48] [ info] ___________
[2025/08/04 15:13:48] [ info] filters:
[2025/08/04 15:13:48] [ info] ___________
[2025/08/04 15:13:48] [ info] outputs:
[2025/08/04 15:13:48] [ info] datadog.0
[2025/08/04 15:13:48] [ info] ___________
[2025/08/04 15:13:48] [ info] collectors:
[2025/08/04 15:13:48] [ info] [fluent bit] version=4.1.0, commit=b73661eaa5, pid=1403772
[2025/08/04 15:13:48] [debug] [engine] coroutine stack size: 24576 bytes (24.0K)
[2025/08/04 15:13:48] [ info] [storage] ver=1.5.3, type=memory, sync=normal, checksum=off, max_chunks_up=128
[2025/08/04 15:13:48] [ info] [simd ] disabled
[2025/08/04 15:13:48] [ info] [cmetrics] version=1.0.5
[2025/08/04 15:13:48] [ info] [ctraces ] version=0.6.6
[2025/08/04 15:13:48] [ info] [input:dummy:dummy.0] initializing
[2025/08/04 15:13:48] [ info] [input:dummy:dummy.0] storage_strategy='memory' (memory only)
[2025/08/04 15:13:48] [debug] [dummy:dummy.0] created event channels: read=29 write=30
[2025/08/04 15:13:48] [debug] [datadog:datadog.0] created event channels: read=31 write=32
[2025/08/04 15:13:48] [debug] [output:datadog:datadog.0] scheme: http://
[2025/08/04 15:13:48] [debug] [output:datadog:datadog.0] site parameter set to: datadoghq.com
[2025/08/04 15:13:48] [debug] [output:datadog:datadog.0] uri: /api/v2/logs
[2025/08/04 15:13:48] [debug] [output:datadog:datadog.0] using site for host construction: datadoghq.com
[2025/08/04 15:13:48] [debug] [output:datadog:datadog.0] host constructed from site: http-intake.logs.datadoghq.com
[2025/08/04 15:13:48] [debug] [output:datadog:datadog.0] port: 80
[2025/08/04 15:13:48] [debug] [output:datadog:datadog.0] json_date_key: timestamp
[2025/08/04 15:13:48] [debug] [output:datadog:datadog.0] compress_gzip: 1
[2025/08/04 15:13:48] [ info] [sp] stream processor started
[2025/08/04 15:13:48] [ info] [engine] Shutdown Grace Period=5, Shutdown Input Grace Period=2
[2025/08/04 15:13:48] [trace] [sched] 0 timer coroutines destroyed
[2025/08/04 15:13:48] [trace] [sched] 0 timer coroutines destroyed
[2025/08/04 15:13:48] [trace] [sched] 0 timer coroutines destroyed
[2025/08/04 15:13:49] [trace] [input chunk] update output instances with new chunk size diff=403, records=1, input=dummy.0
[2025/08/04 15:13:49] [trace] [sched] 0 timer coroutines destroyed
[2025/08/04 15:13:49] [trace] [sched] 0 timer coroutines destroyed
[2025/08/04 15:13:49] [trace] [sched] 0 timer coroutines destroyed
[2025/08/04 15:13:49] [trace] [sched] 0 timer coroutines destroyed
[2025/08/04 15:13:50] [trace] [task 0x5ea0970] created (id=0)
[2025/08/04 15:13:50] [debug] [task] created task=0x5ea0970 id=0 OK
[2025/08/04 15:13:50] [trace] [input chunk] update output instances with new chunk size diff=403, records=1, input=dummy.0
[2025/08/04 15:13:50] [trace] [upstream] get new connection for http-intake.logs.datadoghq.com:80, net setup:
net.connect_timeout = 10 seconds
net.source_address = any
net.keepalive = enabled
net.keepalive_idle_timeout = 30 seconds
net.max_worker_connections = 0
[2025/08/04 15:13:50] [trace] [net] connection #39 in process to http-intake.logs.datadoghq.com:80 |
lucastemb
requested review from
nokute78,
edsiper,
PettitWesley,
niedbalski,
patrick-stephens,
celalettin1286,
fujimotos,
leonardo-albertovich and
koleini
as code owners
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 20
🔭 Outside diff range comments (3)
.github/workflows/unit-tests.yaml (1)
4-13: Unit-test workflow is not triggered on pushes to the new4.0branchYou added
4.0to thepull_request.brancheslist (Line 25) but forgot to add it underpush.branches.
Direct pushes (e.g., backports by maintainers) will therefore skip CI.push: branches: - master - 3.2 - 3.1 - 3.0 + - 4.0 - 2.2Adding the branch ensures the same safety net for both PRs and straight commits.
src/opentelemetry/flb_opentelemetry_utils.c (1)
79-119: Enhance error reporting for type mismatchesThe function now returns -2 for type mismatches, but the error handling doesn't distinguish between different error types. Consider using more specific error codes or adding error context to help with debugging.
Additionally, the nil handling for stringValue (lines 98-105) sets
internal_typeto bothMSGPACK_OBJECT_NILandMSGPACK_OBJECT_STR. This appears to be a logic error.Apply this diff to fix the nil handling logic:
if (strncasecmp(kv_key->ptr, "stringValue", kv_key->size) == 0) { if (kv_value->type == MSGPACK_OBJECT_NIL) { internal_type = MSGPACK_OBJECT_NIL; } else if (kv_value->type != MSGPACK_OBJECT_STR) { /* If the value is not a string, we cannot process it */ return -2; } + else { - internal_type = MSGPACK_OBJECT_STR; + internal_type = MSGPACK_OBJECT_STR; + } }plugins/out_chronicle/chronicle.c (1)
968-969: Fix undefined variable usageThe code attempts to destroy
tokenon line 969, buttokenis only declared and initialized inside theif (ctx->ins->test_mode == FLB_FALSE)block starting at line 947.int need_loop = FLB_TRUE; const int retry_limit = 8; int retries = 0; const size_t one_mebibyte = 1024 * 1024; +flb_sds_t token = NULL; flb_plg_trace(ctx->ins, "flushing bytes %zu", event_chunk->size); /* Get upstream connection */ u_conn = flb_upstream_conn_get(ctx->u); if (!u_conn) { FLB_OUTPUT_RETURN(FLB_RETRY); } if (ctx->ins->test_mode == FLB_FALSE) { /* Get or renew Token */ token = get_google_token(ctx); if (!token) { flb_plg_error(ctx->ins, "cannot retrieve oauth2 token"); flb_upstream_conn_release(u_conn); FLB_OUTPUT_RETURN(FLB_RETRY); } }
🧹 Nitpick comments (14)
README.md (2)
22-22: Add alt text for the ecosystem image.The ecosystem image is missing alt text, which impacts accessibility and SEO.
Apply this diff to add descriptive alt text:
- +
71-71: Fix heading level hierarchy.The "Requirements" heading should be h3 instead of h4 to maintain proper heading hierarchy.
Apply this diff to fix the heading level:
-#### Requirements +### Requirementsplugins/in_windows_exporter_metrics/we_metric.h (1)
40-47: Initialize the newuse_secondary_valuefield in helper macros
use_secondary_valueis not set by theWE_PERFLIB_METRIC_SOURCEandWE_PERFLIB_TERMINATOR_SOURCEmacros. Although omitted designators default to 0, making the initialization explicit avoids future errors if the field order changes or stricter compiler flags are enabled.#define WE_PERFLIB_METRIC_SOURCE(parent_name_, name_, raw_label_set_) \ { \ .parent = NULL, \ .parent_name = parent_name_, \ .name = name_, \ .raw_label_set = raw_label_set_, \ .label_set = NULL, \ - .label_set_size = 0 \ + .label_set_size = 0, \ + .use_secondary_value = 0 \ } #define WE_PERFLIB_TERMINATOR_SOURCE() \ WE_PERFLIB_METRIC_SOURCE(NULL, NULL, NULL)plugins/in_windows_exporter_metrics/CMakeLists.txt (1)
12-13: Nit: keep the source list alphabetically sorted for easier future maintenance
we_cache.candwe_wmi_tcp.cland out of alphabetical order (betweenwe_metric.candwe_perflib.c).
While this does not affect functionality, a consistent order makes merge conflicts less likely.we_metric.c + we_cache.c we_perflib.c ... - we_wmi_tcp.c + we_wmi_tcp.c(no functional change)
Also applies to: 22-23
include/fluent-bit/flb_network.h (1)
102-105: Struct layout change may break binary compatibility for out-of-tree plugins
proxy_env_ignoreis inserted in the middle ofstruct flb_net_setup, changing its size and the offset of every following field.
Any third-party plugin compiled against older headers but loaded by a newer Fluent Bit binary will now read wrong fields and crash.To maintain a (soft) ABI, append new fields at the end of the struct and document the bump in
PLUGIN_INTERFACE_VERSION, or provide a feature/size check macro that out-of-tree modules can use.Please verify whether ABI stability is a project requirement; if so, move the field to the tail of the struct before releasing.
cmake/plugins_options.cmake (1)
48-49: Consider default-OFF for newFLB_IN_PROMETHEUS_TEXTFILEto keep footprint smallThe textfile input plugin is niche compared to scrape/remote-write. Enabling it by default pulls in additional code paths and tests for every downstream user, including embedded targets that value small binaries.
If space or compile-time is a concern, make the option
OFFby default (similar toFLB_IN_WINLOG,FLB_IN_UNIX_SOCKET, etc.) and let interested users opt-in.No action needed if the project accepts the larger default surface.
plugins/in_windows_exporter_metrics/we_net.c (1)
138-140: Consider improving the metric description.The metric specification is correct, but the description could be more grammatically precise.
- WE_PERFLIB_GAUGE_SPEC("output_queue_length_packets", - "A length of output queue packets", - "nic"), + WE_PERFLIB_GAUGE_SPEC("output_queue_length_packets", + "Length of output queue packets", + "nic"),plugins/in_windows_exporter_metrics/we_wmi_paging_file.c (1)
141-161: Correct metric updates with proper memory managementThe implementation correctly retrieves the paging file name, sets all metrics with the file label, and properly frees the allocated string. The calculation of free_megabytes as (limit - current usage) is accurate.
Consider adding a null check after line 141 to handle potential allocation failures from
we_wmi_get_property_str_value.plugins/in_windows_exporter_metrics/we_metric.c (1)
204-207: Redundant null check before freeThe null check on line 204 is redundant since
flb_free(which callsfree) safely handles NULL pointers. However, the explicit check doesn't cause any issues and makes the intent clear.- if (sources[source_index].name != NULL) { - flb_free(sources[source_index].name); - } + flb_free(sources[source_index].name);src/opentelemetry/flb_opentelemetry_utils.c (1)
484-517: LGTM! Robust handling of missing valuesThe implementation correctly handles missing values as empty strings and validates that present values are of the expected map type. This aligns with OpenTelemetry's semantics for unset attributes.
Consider adding a debug log when a value is missing to aid in troubleshooting:
if (value_index == -1) { /* * if value is missing basically is 'unset' and handle as Empty() in OTel world, in * this case we just pack an empty string value */ + flb_debug("[opentelemetry] Missing value for key '%.*s', using empty string", + entry->ptr[key_index].val.via.str.size, + entry->ptr[key_index].val.via.str.ptr); }plugins/in_windows_exporter_metrics/we_perflib.c (1)
825-867: LGTM! Elegant handling of single-instance objectsThe implementation correctly identifies and handles single-instance objects (like Cache and System) that lack PERF_INSTANCE_DEFINITION blocks by creating a synthetic "_Total" instance.
Consider adding a comment explaining the detection logic:
/* * If the total size of instance data is exactly equal to the * size of the first counter block, we can infer this is a single-instance * object that lacks a PERF_INSTANCE_DEFINITION block. + * + * This pattern is used by Windows performance objects that represent + * system-wide resources with no sub-instances (e.g., Cache, System). */plugins/in_windows_exporter_metrics/we_wmi_tcp.c (1)
279-282: Consider checking operational state before API callsThe operational state is checked after making potentially expensive API calls. Consider moving the check before calling
we_tcp_get_state_metrics.int we_wmi_tcp_update(struct flb_we *ctx) { + if (!ctx->wmi_tcp->operational) { + flb_plg_error(ctx->ins, "WMI TCP collector not yet in operational state"); + return -1; + } + uint64_t timestamp = 0; IEnumWbemClassObject* enumerator = NULL; IWbemClassObject *class_obj = NULL; ULONG ret = 0; double val = 0; HRESULT hr; char *ipv4_label = "ipv4"; char *ipv6_label = "ipv6"; - if (!ctx->wmi_tcp->operational) { - flb_plg_error(ctx->ins, "WMI TCP collector not yet in operational state"); - return -1; - } - /* Collect the new state-based metrics first. This does not require WMI coinitialization. */ we_tcp_get_state_metrics(ctx, ipv4_label); we_tcp_get_state_metrics(ctx, ipv6_label);plugins/in_windows_exporter_metrics/we_logical_disk.c (1)
290-292: Buffer overflow risk in wcscpyUsing
wcsncpywithout ensuring null termination can lead to issues. The code correctly null-terminates by removing the last character, but consider using a safer approach.- wcsncpy(temp_guid_path, volume_guid_path, MAX_PATH - 1); - temp_guid_path[wcslen(temp_guid_path) - 1] = L'\0'; + wcsncpy(temp_guid_path, volume_guid_path, MAX_PATH - 1); + temp_guid_path[MAX_PATH - 1] = L'\0'; + size_t len = wcslen(temp_guid_path); + if (len > 0) { + temp_guid_path[len - 1] = L'\0'; + }tests/runtime/out_chronicle.c (1)
7-7: Consider checking mutex initialization result.While
PTHREAD_MUTEX_INITIALIZERtypically doesn't fail, for robustness you might want to usepthread_mutex_init()and check its return value.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (68)
.github/workflows/cron-unstable-build.yaml(1 hunks).github/workflows/staging-release.yaml(2 hunks).github/workflows/unit-tests.yaml(1 hunks)CMakeLists.txt(1 hunks)MAINTENANCE.md(1 hunks)README.md(2 hunks)cmake/plugins_options.cmake(1 hunks)fluent-bit-4.1.0.bb(1 hunks)include/fluent-bit/flb_input.h(2 hunks)include/fluent-bit/flb_network.h(1 hunks)include/fluent-bit/flb_opentelemetry.h(2 hunks)include/fluent-bit/flb_output.h(1 hunks)include/fluent-bit/tls/flb_tls.h(3 hunks)lib/cmetrics/CMakeLists.txt(1 hunks)lib/cmetrics/src/cmt_cat.c(1 hunks)lib/cmetrics/src/cmt_histogram.c(1 hunks)packaging/distros/debian/Dockerfile(2 hunks)plugins/CMakeLists.txt(1 hunks)plugins/filter_aws/aws.c(3 hunks)plugins/filter_aws/aws.h(3 hunks)plugins/in_nginx_exporter_metrics/nginx.c(2 hunks)plugins/in_nginx_exporter_metrics/nginx.h(1 hunks)plugins/in_prometheus_textfile/CMakeLists.txt(1 hunks)plugins/in_prometheus_textfile/prometheus_textfile.c(1 hunks)plugins/in_prometheus_textfile/prometheus_textfile.h(1 hunks)plugins/in_syslog/syslog_prot.c(4 hunks)plugins/in_windows_exporter_metrics/CMakeLists.txt(2 hunks)plugins/in_windows_exporter_metrics/we.c(27 hunks)plugins/in_windows_exporter_metrics/we.h(9 hunks)plugins/in_windows_exporter_metrics/we_cache.c(1 hunks)plugins/in_windows_exporter_metrics/we_cache.h(1 hunks)plugins/in_windows_exporter_metrics/we_cpu.c(2 hunks)plugins/in_windows_exporter_metrics/we_logical_disk.c(10 hunks)plugins/in_windows_exporter_metrics/we_metric.c(4 hunks)plugins/in_windows_exporter_metrics/we_metric.h(1 hunks)plugins/in_windows_exporter_metrics/we_net.c(2 hunks)plugins/in_windows_exporter_metrics/we_perflib.c(6 hunks)plugins/in_windows_exporter_metrics/we_wmi_memory.c(1 hunks)plugins/in_windows_exporter_metrics/we_wmi_paging_file.c(3 hunks)plugins/in_windows_exporter_metrics/we_wmi_tcp.c(1 hunks)plugins/in_windows_exporter_metrics/we_wmi_tcp.h(1 hunks)plugins/in_winevtlog/pack.c(3 hunks)plugins/out_chronicle/chronicle.c(15 hunks)plugins/out_chronicle/chronicle_conf.c(1 hunks)plugins/out_datadog/datadog.c(1 hunks)plugins/out_datadog/datadog.h(1 hunks)plugins/out_datadog/datadog_conf.c(5 hunks)plugins/out_file/file.c(4 hunks)plugins/out_kafka/kafka_config.c(0 hunks)snap/snapcraft.yaml(1 hunks)src/aws/flb_aws_credentials_ec2.c(2 hunks)src/flb_input.c(6 hunks)src/flb_input_chunk.c(2 hunks)src/flb_network.c(2 hunks)src/flb_output.c(6 hunks)src/flb_pack.c(2 hunks)src/flb_upstream.c(1 hunks)src/opentelemetry/flb_opentelemetry_logs.c(1 hunks)src/opentelemetry/flb_opentelemetry_utils.c(6 hunks)src/tls/flb_tls.c(3 hunks)src/tls/openssl.c(6 hunks)tests/internal/data/opentelemetry/test_cases.json(4 hunks)tests/internal/opentelemetry.c(2 hunks)tests/internal/pack.c(2 hunks)tests/runtime/CMakeLists.txt(2 hunks)tests/runtime/data/prometheus_textfile/metrics.prom(1 hunks)tests/runtime/in_prometheus_textfile.c(1 hunks)tests/runtime/out_chronicle.c(1 hunks)
💤 Files with no reviewable changes (1)
- plugins/out_kafka/kafka_config.c
🧰 Additional context used
🧬 Code Graph Analysis (18)
plugins/out_chronicle/chronicle_conf.c (2)
src/flb_output.c (1)
flb_output_get_property(1091-1094)src/flb_sds.c (1)
flb_sds_create(78-90)
src/flb_input_chunk.c (3)
lib/cmetrics/src/cmt_counter.c (1)
cmt_counter_add(119-135)src/flb_input.c (1)
flb_input_name(790-797)include/fluent-bit/flb_compat.h (1)
usleep(128-132)
src/flb_output.c (5)
src/flb_sds.c (1)
flb_sds_destroy(389-399)src/flb_input.c (1)
prop_key_check(450-461)src/flb_utils.c (2)
flb_utils_set_plugin_string_property(1810-1834)flb_utils_bool(671-685)src/tls/flb_tls.c (3)
flb_tls_set_use_enterprise_store(309-316)flb_tls_set_certstore_name(300-307)flb_tls_load_system_certificates(178-181)include/fluent-bit/flb_mem.h (1)
flb_free(126-128)
src/aws/flb_aws_credentials_ec2.c (1)
src/flb_output.c (1)
flb_output_upstream_set(1534-1625)
plugins/in_syslog/syslog_prot.c (1)
src/flb_log_event_encoder.c (1)
flb_log_event_encoder_reset(31-40)
plugins/in_windows_exporter_metrics/we_wmi_memory.c (1)
lib/cmetrics/src/cmt_gauge.c (1)
cmt_gauge_create(27-81)
src/opentelemetry/flb_opentelemetry_logs.c (1)
src/flb_log_event_encoder.c (1)
flb_log_event_encoder_destroy(99-116)
plugins/in_windows_exporter_metrics/we_cache.h (1)
plugins/in_windows_exporter_metrics/we_cache.c (3)
we_cache_init(277-322)we_cache_exit(324-335)we_cache_update(357-370)
plugins/in_windows_exporter_metrics/we_wmi_paging_file.c (2)
lib/cmetrics/src/cmt_gauge.c (2)
cmt_gauge_create(27-81)cmt_gauge_set(94-109)include/fluent-bit/flb_mem.h (1)
flb_free(126-128)
include/fluent-bit/tls/flb_tls.h (1)
src/tls/flb_tls.c (2)
flb_tls_set_certstore_name(300-307)flb_tls_set_use_enterprise_store(309-316)
plugins/in_windows_exporter_metrics/we_metric.c (1)
include/fluent-bit/flb_mem.h (2)
flb_free(126-128)flb_calloc(84-96)
tests/internal/pack.c (2)
src/flb_pack.c (1)
flb_pack_json(332-338)include/fluent-bit/flb_mem.h (1)
flb_free(126-128)
src/opentelemetry/flb_opentelemetry_utils.c (1)
src/flb_log_event_encoder_primitives.c (1)
flb_log_event_encoder_append_string(386-408)
plugins/out_datadog/datadog_conf.c (2)
src/flb_output.c (1)
flb_output_get_property(1091-1094)src/flb_sds.c (3)
flb_sds_create(78-90)flb_sds_cat(120-141)flb_sds_destroy(389-399)
plugins/in_windows_exporter_metrics/we_perflib.c (2)
src/flb_hash_table.c (1)
flb_hash_table_add(401-494)lib/cmetrics/src/cmt_gauge.c (1)
cmt_gauge_set(94-109)
src/tls/openssl.c (2)
include/fluent-bit/flb_mem.h (1)
flb_free(126-128)lib/monkey/mk_core/external/winpthreads.c (2)
pthread_mutex_lock(168-172)pthread_mutex_unlock(174-178)
src/tls/flb_tls.c (1)
include/fluent-bit/flb_mem.h (1)
flb_free(126-128)
plugins/in_windows_exporter_metrics/we_wmi_tcp.c (3)
include/fluent-bit/flb_mem.h (2)
flb_free(126-128)flb_calloc(84-96)lib/cmetrics/src/cmt_gauge.c (2)
cmt_gauge_set(94-109)cmt_gauge_create(27-81)lib/cmetrics/src/cmt_counter.c (2)
cmt_counter_create(26-81)cmt_counter_set(138-161)
🪛 markdownlint-cli2 (0.17.2)
README.md
22-22: Images should have alternate text (alt text)
(MD045, no-alt-text)
71-71: Heading levels should only increment by one level at a time
Expected: h3; Actual: h4
(MD001, heading-increment)
🔇 Additional comments (131)
plugins/out_datadog/datadog.h (1)
64-64: LGTM! Clean struct member addition.The
sitefield is properly positioned within the Configuration section and uses the appropriateflb_sds_ttype consistent with other string configuration fields in the struct.plugins/out_datadog/datadog.c (1)
584-587: LGTM! Well-structured configuration parameter addition.The
siteconfiguration parameter is properly defined with:
- Correct type (
FLB_CONFIG_MAP_STR) for string input- Appropriate default (
NULL) making it optional- Proper struct mapping using
offsetof- Clear, informative description with examples
The description accurately reflects the implementation behavior of prepending 'http-intake.logs.' to construct the hostname.
plugins/out_datadog/datadog_conf.c (3)
43-43: LGTM! Consistent variable declaration.The
sitevariable declaration follows the existing pattern and is properly positioned with other local variables.
102-114: LGTM! Robust site parameter parsing with proper error handling.The implementation correctly:
- Parses the site parameter early for use in host construction
- Handles memory allocation failures with appropriate cleanup
- Provides helpful debug logging for both success and failure cases
- Uses the standard
flb_sds_createfunction for string creation
266-268: LGTM! Proper memory cleanup for site parameter.The cleanup code correctly follows the established pattern:
- Null pointer check before destruction
- Uses appropriate
flb_sds_destroyfunction for SDS strings- Consistent with other SDS field cleanup in the function
packaging/distros/debian/Dockerfile (2)
20-24: LGTM! Necessary infrastructure update for Debian buster.The repository URL migration from
deb.debian.organdsecurity.debian.orgtoarchive.debian.orgis correctly implemented and aligns with the Debian announcement referenced in the comments. The sed commands are properly structured.
51-55: LGTM! Consistent implementation for ARM64 variant.The repository URL patches are correctly applied to the ARM64 Debian buster variant, maintaining consistency with the amd64 implementation above.
plugins/filter_aws/aws.h (2)
30-31: LGTM! Well-structured constant definitions.The new entity-prefixed constants follow the existing naming conventions and pattern, with proper key and length definitions for both EC2 instance ID and account ID.
Also applies to: 42-43
117-121: LGTM! Clear documentation and appropriate field addition.The
enable_entityflag is well-documented, explaining its purpose for appending the 'aws_entity' prefix to relevant keys. The field placement and type are appropriate for the struct.README.md (1)
1-140: Excellent README restructuring!The restructuring transforms this from a technical reference into an effective project landing page. The new format is more approachable, better organized, and provides clear navigation paths to detailed documentation. The focus on key features, production usage stats, and streamlined installation instructions significantly improves the user experience.
snap/snapcraft.yaml (1)
3-3: LGTM! Version bump aligns with 4.1.0 release.The version update from '4.0.5' to '4.1.0' is consistent with the broader Fluent Bit 4.1.0 release cycle mentioned in the maintenance documentation.
plugins/in_nginx_exporter_metrics/nginx.h (1)
33-33: LGTM! Clean addition for configurable scrape intervals.The
scrape_intervalfield is appropriately typed and positioned, with a clear descriptive comment. This enhancement allows for dynamic configuration of metrics collection intervals.lib/cmetrics/CMakeLists.txt (1)
9-10: Patch version bump looks correctThe increment to 1.0.5 aligns with the overall Fluent Bit 4.1.0 release. No further action required.
plugins/in_prometheus_textfile/CMakeLists.txt (1)
1-4: Build script looks goodThe new CMake stub follows the existing plugin pattern and registers the source correctly. 👍
fluent-bit-4.1.0.bb (1)
18-22: Version bump acknowledged
PVupdated to 4.1.0 andSRCREVtied to the tag—consistent with the core change set. Nothing else to flag.CMakeLists.txt (1)
8-12: Core version updated appropriatelyMinor bumped to 1 and patch reset to 0, producing
4.1.0. Ensure release notes reference the new version, but the CMake definitions themselves are fine.tests/runtime/data/prometheus_textfile/metrics.prom (1)
1-4: LGTM – sample metrics file is syntactically correctThe help line, type declaration, labels and values all follow the Prometheus text exposition format. Good addition for the new runtime test.
lib/cmetrics/src/cmt_cat.c (1)
111-111: LGTM! Correct histogram bucket iteration.This change fixes the loop to iterate over all histogram buckets including the implicit +Inf bucket. Histogram implementations typically have
count + 1buckets (where the last one represents +Inf), and the memory allocation on line 105 already accounts for this with(histogram->buckets->count + 1). The loop condition now correctly matches the allocated memory size.tests/runtime/CMakeLists.txt (2)
62-62: LGTM! Proper test registration for Prometheus textfile input plugin.The test registration follows the established pattern and is correctly placed within the input plugins section under the
FLB_OUT_LIBguard.
240-240: LGTM! Proper test registration for Chronicle output plugin.The test registration follows the established pattern and is correctly placed within the output plugins section under the
FLB_IN_LIBguard.plugins/CMakeLists.txt (1)
279-279: LGTM! Proper plugin registration for Prometheus textfile input.The plugin registration follows the established pattern and is appropriately placed among other Prometheus-related input plugins.
plugins/in_windows_exporter_metrics/we_wmi_memory.c (1)
239-241: LGTM! Correct metric naming for paged pool allocations.This fix corrects the metric name from "pool_nonpaged_allocs_total" to "pool_paged_allocs_total" and updates the description accordingly. The change aligns with:
- The WMI property being queried: "PoolPagedAllocs" (line 504)
- The variable name:
pool_paged_allocs_total(line 246)- The metric description referencing "paged pool"
This appears to fix a copy-paste error in the original implementation.
lib/cmetrics/src/cmt_histogram.c (1)
39-39: Critical memory allocation bug fix - LGTM!This change correctly allocates memory for
count + 1doubles instead ofcountdoubles plus one byte. The fix aligns with the comment on Line 38 indicating an implicit bucket for +inf is added, requiring space for the additional bucket boundary.src/opentelemetry/flb_opentelemetry_logs.c (1)
551-557: Good error handling for invalid resource attributes - LGTM!The new error handling properly catches failures from
flb_otel_utils_json_payload_append_converted_kvlist, sets the appropriate error status, cleans up the temporary encoder, and returns early. This follows the same pattern as other error handling in the function and prevents processing invalid resource attributes.plugins/out_chronicle/chronicle_conf.c (1)
214-266: Good test mode handling and enhanced credential fallback logic - LGTM!The conditional wrapper around credentials loading when
ins->test_mode == FLB_FALSEis appropriate for testing scenarios. The enhanced fallback logic forservice_account_emailandservice_account_secretto check environment variables (SERVICE_ACCOUNT_EMAILandSERVICE_ACCOUNT_SECRET) when configuration properties are not set provides better flexibility for credential management.plugins/in_windows_exporter_metrics/we_net.c (1)
82-84: New network metric addition - LGTM!The addition of "output_queue_length_packets" metric mapping to the "Output Queue Length" Windows performance counter is appropriate and follows the existing pattern.
src/flb_input_chunk.c (3)
916-920: LGTM: Proper retry failure trackingThe counter increment for retry failures is correctly placed when the retry limit is exceeded, using appropriate timestamp and label values.
929-932: LGTM: Correct retry attempt trackingThe retry counter is properly incremented when the ring buffer write fails, accurately tracking each retry attempt before the sleep and retry logic.
941-944: LGTM: Complete ring buffer metrics instrumentationThe successful write counter properly complements the retry and failure counters, providing comprehensive ring buffer operation tracking. The placement after successful write is correct.
.github/workflows/staging-release.yaml (1)
568-568: LGTM: Proper matrix-based Windows runner configurationThe matrix strategy correctly enables the job to run on multiple Windows runner versions (windows-latest and windows-2025) with appropriate tag naming that includes the Windows version. This enhances CI coverage for Windows builds.
Also applies to: 577-581
tests/internal/pack.c (2)
1048-1104: LGTM! Excellent test coverage for large uint64 values.This test function effectively validates the correct packing and unpacking of large unsigned 64-bit integers, particularly boundary values like
2^63and2^64-1. The implementation follows established patterns in the test file with proper error handling, memory management, and clear test case structure.
1124-1124: LGTM! Proper test registration.The test is correctly added to the TEST_LIST following the established naming convention and format.
tests/internal/opentelemetry.c (2)
631-634: Excellent diagnostic improvements for test failures.These additions provide valuable debugging information when legacy format validation fails by showing the expected versus actual JSON strings for group metadata, group body, and log body comparisons. This will significantly improve the developer experience when diagnosing test failures.
Also applies to: 638-641, 645-648
708-708: Minor formatting improvement.The added blank line improves code readability in the error handling section.
src/aws/flb_aws_credentials_ec2.c (1)
172-192: Well-implemented fix for EC2 IMDS upstream configuration.This change correctly addresses the issue where
flb_output_upstream_set()overwrites the upstream's network configuration with the output instance settings. The solution properly restores the EC2 IMDS-specific timeout and keepalive settings that are critical for reliable communication with the metadata service.The approach is sound:
- Allow
flb_output_upstream_set()to complete its integration work- Restore the three essential EC2 IMDS network settings
- Clear documentation explains the necessity of this restoration
The restored values (
FLB_AWS_IMDS_TIMEOUTfor timeouts andFLB_FALSEfor keepalive) match the original configuration set inflb_ec2_provider_create(), ensuring consistency.include/fluent-bit/flb_output.h (1)
372-375: LGTM! Windows TLS certificate store fields properly integrated.The addition of
tls_win_certstore_nameandtls_win_use_enterprise_certstorefields is well-structured and follows existing conventions. The conditional compilation ensures Windows-specific functionality is properly isolated.plugins/in_winevtlog/pack.c (4)
192-197: Improved time zone handling with dynamic time zone information.The switch from
TIME_ZONE_INFORMATIONtoDYNAMIC_TIME_ZONE_INFORMATIONand the corresponding API update toGetDynamicTimeZoneInformation()provides better support for dynamic time zones and historical time zone changes. The addition of_tzset()ensures proper timezone initialization.
204-204: Enhanced time conversion API usage.Using
SystemTimeToTzSpecificLocalTimeEx()instead ofSystemTimeToTzSpecificLocalTime()provides more robust time conversion with the dynamic time zone information structure, improving accuracy for historical timestamps.
212-212: Proper daylight saving time flag initialization.Setting the
tm_isdstfield to-1instead of0allows the system to automatically determine whether daylight saving time is in effect, which is more accurate than assuming it's not active.Also applies to: 250-250
239-239: Consistent timezone initialization in pack_filetime.The addition of
_tzset()call ensures consistent timezone initialization across bothpack_systemtime()andpack_filetime()functions, improving reliability of timestamp processing.plugins/in_nginx_exporter_metrics/nginx.c (2)
2343-2347: LGTM! Well-configured scrape interval parameter.The configuration map entry is properly structured with an appropriate time type, reasonable default value (5s), and clear description. This enhances user configurability by replacing the previously hardcoded interval.
2274-2274: LGTM! Proper usage of configurable scrape interval.The usage of
ctx->scrape_intervalin the collector timer setup correctly implements the configurable scrape interval feature, allowing users to control the frequency of NGINX metrics collection.plugins/in_prometheus_textfile/prometheus_textfile.h (2)
20-24: LGTM! Clean header file structure.The header guards are properly implemented and the minimal include is appropriate for the plugin's needs.
25-30: LGTM! Well-designed structure for Prometheus textfile plugin.The structure definition includes all necessary fields with appropriate types and clear comments. The design follows Fluent Bit input plugin conventions and supports the plugin's requirements for configurable scrape intervals and multiple file paths.
include/fluent-bit/flb_input.h (2)
382-383: LGTM! Well-designed ring buffer configuration fields.The addition of
ring_buffer_sizeandring_buffer_windowfields to the input instance struct is well-structured, using appropriate data types (size_t for size, uint8_t for percentage) and consistent naming conventions.
429-432: LGTM! Comprehensive ring buffer metrics tracking.The new CMetrics counters for ring buffer operations (
cmt_ring_buffer_writes,cmt_ring_buffer_retries,cmt_ring_buffer_retry_failures) provide excellent observability into ring buffer performance and will be valuable for monitoring and debugging.include/fluent-bit/flb_opentelemetry.h (2)
30-54: LGTM! Excellent refactoring from macros to enum.Converting the error codes from individual macros to a single enumerated type improves type safety, maintainability, and IDE support. The new
FLB_OTEL_RESOURCE_INVALID_ATTRIBUTEerror code is properly integrated with sequential numbering starting from 1.
70-70: LGTM! Proper error mapping integration.The new error code is correctly added to the
otel_error_maparray with its string representation, maintaining consistency with the existing error handling infrastructure.plugins/in_syslog/syslog_prot.c (4)
219-219: LGTM! Proper encoder initialization.Resetting the log encoder at the start of processing ensures a clean state for each processing cycle, which is essential for the new batched approach.
281-285: LGTM! Efficient batched log appending.The conditional bulk append at the end of processing is an excellent optimization. It reduces system calls by batching all encoded logs into a single append operation, improving performance while maintaining correctness.
306-306: LGTM! Consistent encoder reset for UDP processing.The encoder reset in the UDP processing function maintains consistency with the TCP processing approach, ensuring proper state management.
328-332: LGTM! Consistent batched approach for UDP.The UDP processing function correctly implements the same batched log appending pattern as the TCP version, ensuring consistent performance optimization across both protocols.
src/tls/flb_tls.c (3)
219-222: LGTM! Proper Windows-specific field initialization.The initialization of
certstore_nameanduse_enterprise_storefields is correctly placed within the Windows conditional block, ensuring clean state and platform-specific behavior.
268-272: LGTM! Proper memory cleanup for Windows fields.The cleanup of the
certstore_namefield is correctly implemented within the Windows conditional block inflb_tls_destroy, preventing memory leaks.
299-317: LGTM! Well-structured Windows TLS API functions.Both
flb_tls_set_certstore_nameandflb_tls_set_use_enterprise_storefunctions are properly implemented with null checks and delegate to the TLS backend API, following the established pattern of other TLS configuration functions.MAINTENANCE.md (1)
1-41: LGTM! Comprehensive and well-structured maintenance policy.This maintenance policy document provides excellent clarity on:
- Branch strategy and version management
- Clear maintainer responsibilities and timelines
- Contribution guidelines for maintained versions
- Professional formatting with proper tables and callouts
The policy establishes a clear maintenance timeline (v4.0 until Dec 31, 2025) and defines acceptable change criteria, which will help contributors and maintainers make informed decisions.
plugins/in_windows_exporter_metrics/we_wmi_tcp.h (1)
1-30: LGTM! Clean interface definition for TCP metrics subsystem.The header file follows good practices with proper include guards, licensing, and a clear API consisting of the standard lifecycle functions (init, update, exit). The function naming is consistent with other components in the Windows Exporter plugin.
plugins/out_file/file.c (3)
48-52: Good platform-specific path separator abstraction.The macro definitions provide clean abstraction for platform-specific path separators, improving code readability and maintainability.
377-446: Robust Windows directory creation implementation.The Windows-specific
mkpathimplementation handles several important cases correctly:
- Path normalization (forward slashes to backslashes)
- Drive letter detection and handling
- Recursive parent directory creation using
PathRemoveFileSpecA- Proper error handling with Windows error codes
- Comprehensive debug logging
The implementation is complex but necessary for proper Windows path handling.
518-523: Correct use of platform-specific path separators.The replacement of hardcoded forward slashes with
FLB_PATH_SEPARATORensures proper path construction on both Windows and Unix-like platforms.src/flb_network.c (2)
155-155: LGTM! Proper initialization of proxy environment ignore flag.The initialization of
proxy_env_ignoretoFLB_FALSEis consistent with other boolean field initializations in theflb_net_setup_initfunction.
1096-1099: Verify the necessity of always enabling EDNS.The DNS flag handling is improved by using bitwise OR to combine flags instead of overwriting them. However, the change to always enable
ARES_FLAG_EDNSseems unrelated to the proxy environment ignore feature mentioned in the PR objectives.Please verify that enabling EDNS by default is intentional and won't cause compatibility issues with older DNS infrastructure.
plugins/in_windows_exporter_metrics/we_cache.h (1)
1-30: LGTM! Well-structured interface for cache metrics collection.The header file follows the established pattern for Windows Exporter metric collectors with:
- Proper include guards and licensing
- Clean API with standard lifecycle functions (init, exit, update)
- Consistent naming convention with other collectors
- Function signatures match the implementations shown in the relevant code snippets
plugins/in_windows_exporter_metrics/we_cpu.c (2)
142-157: LGTM! Clean addition of new CPU metrics.The four new CPU performance metrics follow the existing pattern and naming conventions. The use of the
,secondvaluesuffix forprocessor_rtc_totalis correctly handled by the perflib processing code mentioned in the AI summary.
210-230: LGTM! Well-documented metric specifications.The metric specifications provide clear and accurate descriptions for each new CPU performance metric. The documentation properly explains what each metric measures and maintains consistency with existing specifications.
plugins/filter_aws/aws.c (1)
1317-1322: LGTM! Well-documented configuration option.The new
enable_entityconfiguration option is properly documented and follows the existing pattern. The description clearly explains its purpose and effect.src/flb_pack.c (2)
118-153: LGTM! Excellent numeric token handling improvement.The new
pack_numeric_tokenfunction properly centralizes and enhances numeric JSON token packing with several improvements:
- Proper overflow detection: Uses
errnoto detectERANGEfromstrtoll/strtoull- Correct type handling: Distinguishes between signed/unsigned integers and chooses appropriate msgpack types
- Fallback strategy: Falls back to double precision when integer parsing fails
- Uint64 support: Properly handles large unsigned 64-bit integers using
msgpack_pack_uint64The logic correctly handles the case where unsigned values fit in int64 range (Line 146-147) vs requiring uint64 (Line 150).
256-256: LGTM! Clean refactoring to use centralized function.The replacement of inline numeric packing logic with the new
pack_numeric_tokenfunction call improves code maintainability and consistency.tests/internal/data/opentelemetry/test_cases.json (5)
89-89: Test case rename reflects new behaviorThe rename from "scope_kvlist_error" to "scope_kvlist_empty_value" appropriately reflects the change from error handling to accepting empty values in key-value lists.
94-94: Improved test data clarityThe attribute key name "empty-key" is more descriptive and clear about what's being tested compared to "incomplete-kv-pair".
103-109: Correct expected output for empty attribute valuesThe expected output correctly shows that attributes with missing values are parsed as empty strings rather than causing errors. This is a reasonable approach for handling incomplete key-value pairs.
954-954: Valid test data enhancementAdding the "value" field makes the test data structure more explicit and complete, even though it still expects an error due to the wrong value type.
1204-1228: Well-structured test for null string handlingThe new test case properly validates that null string values in attributes are converted to empty strings in the output, which is consistent with the OpenTelemetry parser's behavior for handling missing/null values.
tests/runtime/in_prometheus_textfile.c (3)
7-21: Thread-safe callback implementationThe callback correctly uses mutex locking for thread-safe access to the shared counter and properly frees the allocated record memory.
36-58: Ensure consistent resource cleanupThe function properly handles errors and cleans up resources. The implementation looks correct.
68-113: Well-structured test with proper timeout handlingThe test correctly configures the Prometheus textfile plugin, starts Fluent Bit, and polls for output with a reasonable timeout. The use of flb_time functions for accurate timing is appropriate.
plugins/in_windows_exporter_metrics/we_wmi_paging_file.c (1)
48-76: Improved metrics with per-file labelingThe new gauge metrics with "file" labels provide better granularity for monitoring individual paging files. The metric names and descriptions are clear and follow Prometheus naming conventions.
plugins/in_windows_exporter_metrics/we_metric.c (2)
222-222: Critical type correctionsThe type corrections from
we_perflib_metric_spectowe_perflib_metric_sourceare essential fixes that prevent potential memory corruption and ensure type safety.Also applies to: 249-249
260-275: Well-implemented secondvalue flag handlingThe implementation correctly:
- Creates a writable copy of the source name to avoid modifying read-only memory
- Detects the ",secondvalue" suffix and sets the appropriate flag
- Removes the suffix by truncating at the comma
- Includes proper error handling and cleanup on allocation failure
This is a clean solution for handling the secondary value configuration.
plugins/in_prometheus_textfile/prometheus_textfile.c (4)
44-83: Correct Unix glob implementationThe Unix glob implementation properly handles all error cases (GLOB_NOSPACE, GLOB_NOMATCH, GLOB_ABORTED) and correctly manages memory with globfree. The use of cfl_array for storing results is appropriate.
214-291: Well-structured metrics collectionThe collect_metrics function properly:
- Handles glob patterns for each configured path
- Validates files are regular files before reading
- Uses cmetrics for parsing with appropriate error handling
- Cleans up resources (content, files array) correctly
- Provides helpful debug/error messages
303-333: Proper plugin initializationThe initialization correctly sets up the plugin context, configuration mapping, and collector timer. Error handling and resource cleanup are properly implemented.
359-372: Clear configuration optionsThe configuration map properly defines:
path: Comma-separated list for multiple file patternsscrape_interval: Time-based configuration with sensible 10s defaultBoth options are appropriately marked as required.
src/opentelemetry/flb_opentelemetry_utils.c (2)
238-244: LGTM! Nil handling is appropriateThe implementation correctly handles nil values by appending an empty string, which aligns with OpenTelemetry's approach to unset values.
381-383: LGTM! Improved error propagationThe explicit error handling for unwrapping failures enhances the robustness of the function.
include/fluent-bit/tls/flb_tls.h (1)
96-99: LGTM! Well-structured Windows certificate store supportThe Windows-specific certificate store configuration is properly implemented with:
- Backend function pointers for runtime configuration
- Context fields to store the settings
- Public API functions for external configuration
- Appropriate platform conditionals
Also applies to: 109-112, 134-137
plugins/in_windows_exporter_metrics/we_cache.c (1)
277-322: Remove redundant null check formetric_specsThe helper
we_initialize_perflib_metric_specsreturns a non-zero error code on any allocation or initialization failure and only sets*out_specswhen it succeeds. After a successful return (result == 0),ctx->cache.metric_specsis guaranteed to be non-NULL. Adding a separate null check forctx->cache.metric_specsis therefore unnecessary and inconsistent with the other exporters (e.g., CPU, network, logical_disk), which follow the same pattern.Likely an incorrect or invalid review comment.
src/tls/openssl.c (1)
314-329: LGTM! Flexible certificate store selectionThe implementation properly:
- Uses the configured certificate store name with a sensible default
- Supports both standard and enterprise certificate stores
- Provides helpful debug logging with the store name
Also applies to: 383-384
plugins/in_windows_exporter_metrics/we_wmi_tcp.c (2)
90-94: Check malloc return value before useThe function should check if
flb_mallocreturns NULL before using the allocated memory to avoid potential null pointer dereference.The error handling is correct - it checks for NULL and logs an error. Good defensive programming.
110-118: No array bounds issue:state_indexalways falls within 0–12
Thewindows_state_to_index()function handles every TCP state and uses adefaultto return 12, so all return values are in the 0–12 range. Thestate_counts[13]array and the loopfor (i = 0; i < 13; i++)are therefore safe as written.plugins/in_windows_exporter_metrics/we_logical_disk.c (2)
169-184: LGTM! Clean metric initializationThe initialization of
size_bytesandfree_bytesgauges is well-structured with proper error handling.
407-422: Good error handling structureThe update function properly handles errors from both Perflib and Windows API parts, maintaining functionality even if one part fails.
plugins/out_chronicle/chronicle.c (7)
496-513: LGTM! Proper test mode handlingThe conditional token retrieval and log type validation based on test mode is well-implemented.
523-524: Good improvement: Using boolean flag instead of counterReplacing the counter-based approach with a boolean flag for tracking log key presence is cleaner and more intuitive.
549-551: Good defensive programming: Early memory cleanupAdding
flb_free(val_buf)before returning NULL prevents memory leaks in error cases.
667-673: Well-designed entry structureThe
chronicle_entrystructure cleanly encapsulates log data with proper linked list integration.
861-865: Excellent memory managementThe cleanup of entries after packing them into the msgpack buffer is properly implemented, preventing memory leaks.
947-956: Conditional token handling for test modeGood practice to skip token retrieval in test mode while maintaining proper error handling for production mode.
1110-1110: Good resource cleanupProperly destroying the token synchronization mutex in the exit callback prevents resource leaks.
src/flb_output.c (5)
86-95: Windows-specific TLS properties properly documentedThe new TLS Windows certificate store configuration options are well-documented and appropriately Windows-specific.
187-191: Proper cleanup of Windows TLS fieldsThe conditional cleanup of
tls_win_certstore_nameis correctly implemented with platform-specific guards.
997-1005: Well-structured Windows TLS property parsingThe property parsing for Windows certificate store configuration follows the established pattern and includes proper memory management.
1391-1423: Comprehensive Windows TLS initializationThe Windows-specific TLS initialization properly handles both enterprise certificate store and custom certificate store configurations with appropriate error handling and logging.
1595-1623: Well-implemented proxy environment variable handlingThe logic to restore original host information when
proxy_env_ignoreis true is correctly implemented, including proper cleanup of proxy credentials.plugins/in_windows_exporter_metrics/we.c (13)
100-108: LGTM! Timer callback follows established pattern.The new cache metrics timer callback is correctly implemented following the same pattern as other collectors.
190-198: LGTM! TCP timer callback properly implemented.The TCP metrics timer callback follows the established pattern consistently.
287-292: LGTM! Cache update callback correctly implemented.The callback follows the established pattern for metric update callbacks.
350-355: LGTM! TCP update callback properly integrated.The callback is correctly implemented following the existing pattern.
376-377: LGTM! Callbacks properly registered in the table.The new cache and tcp callbacks are correctly added to the callbacks table in the appropriate positions.
Also applies to: 384-384
414-414: LGTM! File descriptors properly initialized.The new collector file descriptors are correctly initialized to -1.
Also applies to: 423-423
629-653: LGTM! Cache collector initialization follows established pattern.The cache collector initialization correctly handles both default and custom scrape intervals, with proper error handling.
915-917: LGTM! Exit cleanup properly handles new collectors.The exit function correctly calls the exit functions for both cache and TCP collectors.
Also applies to: 939-941
967-969: LGTM! Timer-based cleanup properly implemented.The exit function correctly handles cleanup for timer-based collectors.
Also applies to: 994-996
1025-1027: LGTM! Pause functionality correctly implemented.The pause function properly handles the new cache and TCP collectors.
Also applies to: 1052-1054
1155-1159: LGTM! Configuration map entries properly added.The new configuration entries for cache and TCP scrape intervals follow the established pattern with appropriate descriptions.
Also applies to: 1206-1210
1214-1214: LGTM! Default metrics list updated.The cache and tcp metrics are correctly added to the default enabled metrics list.
657-657: All metric indices correctly aligned withne_callbacksentries
Verified thatmetric_idxassignments (0–14) map exactly to the 15 callbacks defined inne_callbacks[]. No adjustments are needed.tests/runtime/out_chronicle.c (2)
39-84: LGTM! Callback functions properly validate JSON output.The callback functions correctly check for expected JSON structure and content, with proper null checks and memory cleanup.
134-167: LGTM! Test functions follow consistent pattern.The test functions properly set up Fluent Bit contexts, configure plugins, push records, and verify callback invocation with proper cleanup.
plugins/in_windows_exporter_metrics/we.h (6)
85-86: LGTM! Perflib object extended with byte length fields.The new fields properly track object size information needed for enhanced metric processing.
118-120: LGTM! Logical disk counters extended with size metrics.The new gauge fields properly support disk size and free space metrics.
122-128: LGTM! Cache counters structure properly defined.The new cache counters structure follows the established pattern for perflib-based metrics.
208-210: LGTM! Paging file counters updated with labeled metrics.The structure properly reflects the change from individual gauges to labeled metrics for better flexibility.
234-249: LGTM! TCP counters structure well-designed.The TCP counters structure properly separates IPv4/IPv6 WMI queries and includes comprehensive connection and segment metrics.
316-316: LGTM! Context structure properly extended.All new fields for cache and TCP collectors are correctly added to the main context structure, maintaining consistency with existing collectors.
Also applies to: 325-325, 332-332, 341-341, 353-353, 362-362
src/flb_input.c (6)
59-73: Documentation accurately reflects the new configurable ring buffer system.The updated comments clearly explain the new default capacity, configurability options, and the relationship between capacity (number of slots) and internal byte conversion.
130-139: LGTM! Configuration map entries are properly structured.The new ring buffer configuration properties are correctly defined with appropriate types, default values, and clear descriptions.
421-426: LGTM! Default initialization properly uses configurable values.The initialization correctly sets default values from macros and uses instance-specific
ring_buffer_sizefor ring buffer creation, enabling per-instance customization.
725-749: LGTM! Property handling includes proper validation and ring buffer recreation.The implementation correctly validates input values, converts capacity to bytes, and recreates the ring buffer when capacity changes. The validation ranges are appropriate (capacity > 0, window 1-100%).
1240-1267: LGTM! Ring buffer metrics counters follow established patterns.The new metrics counters are properly initialized with descriptive names and consistent labeling, providing valuable observability for ring buffer operations.
1394-1394: LGTM! Event loop registration now uses configurable window value.The change from hardcoded macro to instance-specific
ring_buffer_windowproperly enables per-instance window configuration.
| - name: 4.0 run | ||
| if: github.event_name == 'schedule' && github.event.schedule=='0 24 * * *' | ||
| run: | | ||
| echo "cron_branch=4.0" >> $GITHUB_ENV | ||
| shell: bash | ||
|
|
There was a problem hiding this comment.
Fix invalid cron schedule expression.
The cron expression 0 24 * * * uses an invalid hour value (24). Hours in cron expressions range from 0-23. This schedule will never trigger.
Choose a valid hour for the 4.0 branch build. For example:
- - name: 4.0 run
- if: github.event_name == 'schedule' && github.event.schedule=='0 24 * * *'
+ - name: 4.0 run
+ if: github.event_name == 'schedule' && github.event.schedule=='0 0 * * *'
run: |
echo "cron_branch=4.0" >> $GITHUB_ENV
shell: bashAlso add the corresponding schedule entry to the schedule section:
schedule:
- cron: "0 6 * * *" # master build
- cron: "0 12 * * *" # 3.2 build
- cron: "0 18 * * *" # master build
+ - cron: "0 0 * * *" # 4.0 build📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - name: 4.0 run | |
| if: github.event_name == 'schedule' && github.event.schedule=='0 24 * * *' | |
| run: | | |
| echo "cron_branch=4.0" >> $GITHUB_ENV | |
| shell: bash | |
| - name: 4.0 run | |
| if: github.event_name == 'schedule' && github.event.schedule=='0 0 * * *' | |
| run: | | |
| echo "cron_branch=4.0" >> $GITHUB_ENV | |
| shell: bash |
🤖 Prompt for AI Agents
In .github/workflows/cron-unstable-build.yaml around lines 67 to 72, the cron
schedule expression uses an invalid hour value "24" which is outside the valid
range 0-23. Replace "0 24 * * *" with a valid cron expression such as "0 0 * *
*" to run at midnight. Additionally, add the corresponding valid schedule entry
under the "schedule" section of the workflow file to ensure the cron trigger is
properly configured.
plugins/filter_aws/aws.c
Outdated
| } | ||
| } | ||
|
|
||
| ctx->metadata_retrieved = FLB_TRUE; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove redundant metadata_retrieved assignment.
Line 952 sets ctx->metadata_retrieved = FLB_TRUE but this was already set at line 926 when metadata_fetched == FLB_TRUE. This redundant assignment could mask cases where some metadata groups failed to fetch.
- ctx->metadata_retrieved = FLB_TRUE;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ctx->metadata_retrieved = FLB_TRUE; |
🤖 Prompt for AI Agents
In plugins/filter_aws/aws.c at line 952, remove the redundant assignment setting
ctx->metadata_retrieved to FLB_TRUE, as it was already set at line 926 when
metadata_fetched is FLB_TRUE. This prevents masking partial metadata fetch
failures by avoiding overwriting the flag unnecessarily.
plugins/filter_aws/aws.c
Outdated
| ret = flb_log_event_encoder_append_body_values( | ||
| &log_encoder, | ||
| FLB_LOG_EVENT_CSTRING_VALUE(FLB_FILTER_AWS_ENTITY_ACCOUNT_ID_KEY), | ||
| FLB_LOG_EVENT_STRING_VALUE(ctx->account_id, | ||
| ctx->account_id_len)); |
There was a problem hiding this comment.
Fix inconsistent error handling in entity field appending.
The second flb_log_event_encoder_append_body_values call doesn't check the return value, creating inconsistent error handling within the same code block.
ret = flb_log_event_encoder_append_body_values(
&log_encoder,
FLB_LOG_EVENT_CSTRING_VALUE(FLB_FILTER_AWS_ENTITY_INSTANCE_ID_KEY),
FLB_LOG_EVENT_STRING_VALUE(ctx->instance_id,
ctx->instance_id_len));
- ret = flb_log_event_encoder_append_body_values(
+ if (ret == FLB_EVENT_ENCODER_SUCCESS) {
+ ret = flb_log_event_encoder_append_body_values(
&log_encoder,
FLB_LOG_EVENT_CSTRING_VALUE(FLB_FILTER_AWS_ENTITY_ACCOUNT_ID_KEY),
FLB_LOG_EVENT_STRING_VALUE(ctx->account_id,
ctx->account_id_len));
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ret = flb_log_event_encoder_append_body_values( | |
| &log_encoder, | |
| FLB_LOG_EVENT_CSTRING_VALUE(FLB_FILTER_AWS_ENTITY_ACCOUNT_ID_KEY), | |
| FLB_LOG_EVENT_STRING_VALUE(ctx->account_id, | |
| ctx->account_id_len)); | |
| ret = flb_log_event_encoder_append_body_values( | |
| &log_encoder, | |
| FLB_LOG_EVENT_CSTRING_VALUE(FLB_FILTER_AWS_ENTITY_INSTANCE_ID_KEY), | |
| FLB_LOG_EVENT_STRING_VALUE(ctx->instance_id, | |
| ctx->instance_id_len)); | |
| if (ret == FLB_EVENT_ENCODER_SUCCESS) { | |
| ret = flb_log_event_encoder_append_body_values( | |
| &log_encoder, | |
| FLB_LOG_EVENT_CSTRING_VALUE(FLB_FILTER_AWS_ENTITY_ACCOUNT_ID_KEY), | |
| FLB_LOG_EVENT_STRING_VALUE(ctx->account_id, | |
| ctx->account_id_len)); | |
| } |
🤖 Prompt for AI Agents
In plugins/filter_aws/aws.c around lines 1140 to 1144, the second call to
flb_log_event_encoder_append_body_values does not check its return value,
causing inconsistent error handling. Modify the code to capture the return value
of this second call and add a conditional check to handle errors consistently,
similar to the first call, ensuring that any failure is properly detected and
handled.
| } | ||
|
|
||
| if (strchr(p1, '*')) { | ||
| if (read_glob_win(path, list) == NULL) { |
There was a problem hiding this comment.
Incorrect parameter in recursive call
The recursive call to read_glob_win on line 189 passes the original path parameter instead of the constructed buf path, which would cause incorrect pattern matching for nested wildcards.
- if (read_glob_win(path, list) == NULL) {
+ if (read_glob_win(buf, list) == NULL) {🤖 Prompt for AI Agents
In plugins/in_prometheus_textfile/prometheus_textfile.c at line 189, the
recursive call to read_glob_win incorrectly passes the original path parameter
instead of the constructed buf path. Update the recursive call to pass buf as
the argument to ensure correct pattern matching for nested wildcards.
src/flb_upstream.c
Outdated
| { | ||
| FLB_CONFIG_MAP_BOOL, "net.proxy_env_ignore", "FALSE", | ||
| 0, FLB_TRUE, offsetof(struct flb_net_setup, proxy_env_ignore), | ||
| "Ignore the environment variables HTTP_PROXY, HTTPS_PROXY and NO_PROXY when " | ||
| }, |
There was a problem hiding this comment.
Fix the incomplete configuration description.
The description string for the net.proxy_env_ignore option is incomplete and ends abruptly with "when ". This will confuse users trying to understand the option's purpose.
Apply this diff to complete the description:
- "Ignore the environment variables HTTP_PROXY, HTTPS_PROXY and NO_PROXY when "
+ "Ignore the environment variables HTTP_PROXY, HTTPS_PROXY and NO_PROXY when establishing connections"🤖 Prompt for AI Agents
In src/flb_upstream.c around lines 139 to 143, the description string for the
net.proxy_env_ignore configuration option is incomplete and ends abruptly with
"when ". Complete the description by providing the full explanation of what
happens when this option is enabled, ensuring it clearly states that it causes
the environment variables HTTP_PROXY, HTTPS_PROXY, and NO_PROXY to be ignored.
| else { | ||
| printf("Unsupported key type: %d\n", map->ptr[0].key.type); | ||
| } |
There was a problem hiding this comment.
Remove debug printf statement
Debug output should use the logging framework instead of printf to stdout.
Replace the printf with proper logging or remove it entirely:
else {
- printf("Unsupported key type: %d\n", map->ptr[0].key.type);
+ flb_debug("[opentelemetry] Unsupported key type: %d", map->ptr[0].key.type);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| else { | |
| printf("Unsupported key type: %d\n", map->ptr[0].key.type); | |
| } | |
| else { | |
| flb_debug("[opentelemetry] Unsupported key type: %d", map->ptr[0].key.type); | |
| } |
🤖 Prompt for AI Agents
In src/opentelemetry/flb_opentelemetry_utils.c around lines 152 to 154, replace
the debug printf statement that outputs "Unsupported key type" with a call to
the project's logging framework or remove it entirely. Identify the appropriate
logging function used in the codebase and use it to log the message instead of
printing directly to stdout.
src/tls/openssl.c
Outdated
| #if defined(FLB_SYSTEM_WINDOWS) | ||
| static int tls_set_certstore_name(struct flb_tls *tls, const char *certstore_name) | ||
| { | ||
| struct tls_context *ctx = tls->ctx; | ||
|
|
||
| pthread_mutex_lock(&ctx->mutex); | ||
|
|
||
| ctx->certstore_name = flb_strdup(certstore_name); | ||
|
|
||
| pthread_mutex_unlock(&ctx->mutex); | ||
|
|
||
| return 0; | ||
| } | ||
|
|
||
| static int tls_set_use_enterprise_store(struct flb_tls *tls, int use_enterprise) | ||
| { | ||
| struct tls_context *ctx = tls->ctx; | ||
|
|
||
| pthread_mutex_lock(&ctx->mutex); | ||
|
|
||
| ctx->use_enterprise_store = !!use_enterprise; | ||
|
|
||
| pthread_mutex_unlock(&ctx->mutex); | ||
|
|
||
| return 0; | ||
| } | ||
| #endif |
There was a problem hiding this comment.
Add input validation for certificate store configuration
The setter functions should validate input parameters and handle edge cases like empty strings or excessively long names.
static int tls_set_certstore_name(struct flb_tls *tls, const char *certstore_name)
{
struct tls_context *ctx = tls->ctx;
+
+ if (!certstore_name || strlen(certstore_name) == 0) {
+ return -1;
+ }
+
+ /* Windows certificate store names have practical length limits */
+ if (strlen(certstore_name) > 256) {
+ return -1;
+ }
pthread_mutex_lock(&ctx->mutex);
+
+ if (ctx->certstore_name) {
+ flb_free(ctx->certstore_name);
+ }
ctx->certstore_name = flb_strdup(certstore_name);
+
+ if (!ctx->certstore_name) {
+ pthread_mutex_unlock(&ctx->mutex);
+ return -1;
+ }
pthread_mutex_unlock(&ctx->mutex);
return 0;
}🤖 Prompt for AI Agents
In src/tls/openssl.c around lines 799 to 825, the functions
tls_set_certstore_name and tls_set_use_enterprise_store lack input validation.
Add checks to ensure certstore_name is not NULL, not an empty string, and does
not exceed a reasonable length before duplicating it. For
tls_set_use_enterprise_store, validate that use_enterprise is within expected
values (e.g., 0 or 1). Return an error code if validation fails and avoid
modifying the context in that case.
| "scope_kvlist_wrong_value_type": { | ||
| "input": { | ||
| "resourceLogs": [{ | ||
| "scopeLogs": [{ | ||
| "scope": { | ||
| "attributes": [{"key": "empty-key"}] | ||
| }, | ||
| "logRecords": [{ | ||
| "timeUnixNano": "1", | ||
| "body": {"stringValue": "test"} | ||
| }] | ||
| }] | ||
| }] | ||
| }, | ||
| "expected": { | ||
| "group_metadata": {"schema":"otlp","resource_id":0,"scope_id":0}, | ||
| "group_body": {"resource":{}, "scope":{"attributes":{"empty-key":""}}}, | ||
| "log_metadata": {"otlp":{}}, | ||
| "log_body": {"log": "test"} | ||
| } | ||
| }, |
There was a problem hiding this comment.
Remove duplicate test case
The test case "scope_kvlist_wrong_value_type" has identical input and expected output to "scope_kvlist_empty_value" (lines 89-109). This duplication provides no additional test coverage and should be removed or modified to test a different scenario.
🤖 Prompt for AI Agents
In tests/internal/data/opentelemetry/test_cases.json around lines 111 to 131,
the test case "scope_kvlist_wrong_value_type" duplicates the input and expected
output of "scope_kvlist_empty_value" from lines 89 to 109. Remove the
"scope_kvlist_wrong_value_type" test case entirely or modify its input and
expected output to cover a different scenario, ensuring no redundant test cases
exist.
|
There is a merge in this PR that messed up the original intent of the change, we cannot merge this. Please cleanup the PR where only the changes are done in plugins/out_datadog. |
Signed-off-by: Lucas Tembras <[email protected]>
Signed-off-by: Lucas Tembras <[email protected]>
lucastemb
force-pushed
the
lucastemb/AGNTLOG-199
branch
from
b73661e to
64226fe
Compare
@edsiper Just finished cleaning it up 👍 Let me know if you need anything else. |
|
|
||
| { | ||
| FLB_CONFIG_MAP_STR, "site", NULL, 0, FLB_FALSE, offsetof(struct flb_out_datadog, site), | ||
| "DataDog site for telemetry data (e.g., 'datadoghq.eu', 'datadoghq.com', 'us3.datadoghq.com'). The plugin will construct the full hostname by prepending 'http-intake.logs.' to this value." |
There was a problem hiding this comment.
line is too long, up to 120 is fine (from the source code perspective), just continue the string in another line
There was a problem hiding this comment.
if you set FLB_TRUE in the 5th parameter, the value will be written to context->site
| @@ -98,6 +99,20 @@ struct flb_out_datadog *flb_datadog_conf_create(struct flb_output_instance *ins, | |||
| return NULL; | |||
| } | |||
|
|
|||
| /* Parse site parameter early so it's available for host construction */ | |||
| tmp = flb_output_get_property("site", ins); | |||
There was a problem hiding this comment.
if the config map is adjusted, here you will only need to check if ctx->site is not null and you avoid the allocations set
| return NULL; | ||
| } | ||
| flb_plg_debug(ctx->ins, "site parameter set to: %s", ctx->site); | ||
| } else { |
| tmp_sds = flb_sds_cat(tmp_sds, (const char *)ctx->site, flb_sds_len(ctx->site)); | ||
| ctx->host = tmp_sds; | ||
| flb_plg_debug(ctx->ins, "host constructed from site: %s", ctx->host); | ||
| } else { |
| @@ -222,6 +263,9 @@ int flb_datadog_conf_destroy(struct flb_out_datadog *ctx) | |||
| if (ctx->upstream) { | |||
| flb_upstream_destroy(ctx->upstream); | |||
| } | |||
| if (ctx->site){ | |||
There was a problem hiding this comment.
if config map writes to the context this is not needed
Add
siteParameter to Fluent Bit Plugin for Regional Data RoutingEnter
[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:
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