From 98313ebf206eec4a4e5375b352fc36849b762323 Mon Sep 17 00:00:00 2001 From: Matthew Fala Date: Sat, 7 Jan 2023 01:05:27 +0000 Subject: [PATCH] Revert "out_datadog: fix/add error handling for all flb_sds calls (#5929)" This reverts commit 300206ae6a0263b322861b72c23fb92b69a1b1cf. --- plugins/out_datadog/datadog.c | 66 ++----------- plugins/out_datadog/datadog_conf.c | 21 +--- plugins/out_datadog/datadog_remap.c | 145 ++++++---------------------- plugins/out_datadog/datadog_remap.h | 4 +- 4 files changed, 46 insertions(+), 190 deletions(-) diff --git a/plugins/out_datadog/datadog.c b/plugins/out_datadog/datadog.c index 59689b18299..cd14f083602 100644 --- a/plugins/out_datadog/datadog.c +++ b/plugins/out_datadog/datadog.c @@ -92,11 +92,10 @@ static int datadog_format(struct flb_config *config, { int i; int ind; - int byte_cnt = 64; + int byte_cnt; int remap_cnt; - int ret; /* for msgpack global structs */ - size_t array_size = 0; + int array_size = 0; size_t off = 0; msgpack_unpacked result; msgpack_sbuffer mp_sbuf; @@ -111,23 +110,13 @@ static int datadog_format(struct flb_config *config, msgpack_object k; msgpack_object v; struct flb_out_datadog *ctx = plugin_context; - struct flb_event_chunk *event_chunk; /* output buffer */ flb_sds_t out_buf; flb_sds_t remapped_tags = NULL; - flb_sds_t tmp = NULL; - - /* in normal flush callback we have the event_chunk set as flush context - * so we don't need to calculate the event len. - * But in test mode the formatter won't get the event_chunk as flush_ctx - */ - if (flush_ctx != NULL) { - event_chunk = flush_ctx; - array_size = event_chunk->total_events; - } else { - array_size = flb_mp_count(data, bytes); - } + + /* Count number of records */ + array_size = flb_mp_count(data, bytes); /* Create temporary msgpack buffer */ msgpack_sbuffer_init(&mp_sbuf); @@ -172,22 +161,6 @@ static int datadog_format(struct flb_config *config, if (!remapped_tags) { remapped_tags = flb_sds_create_size(byte_cnt); - if (!remapped_tags) { - flb_errno(); - msgpack_sbuffer_destroy(&mp_sbuf); - msgpack_unpacked_destroy(&result); - return -1; - } - } else if (flb_sds_len(remapped_tags) < byte_cnt) { - tmp = flb_sds_increase(remapped_tags, flb_sds_len(remapped_tags) - byte_cnt); - if (!tmp) { - flb_errno(); - flb_sds_destroy(remapped_tags); - msgpack_sbuffer_destroy(&mp_sbuf); - msgpack_unpacked_destroy(&result); - return -1; - } - remapped_tags = tmp; } /* @@ -254,11 +227,8 @@ static int datadog_format(struct flb_config *config, * (so they won't be packed as attr) */ if (ctx->remap && (ind = dd_attr_need_remapping(k, v)) >=0 ) { - ret = remapping[ind].remap_to_tag(remapping[ind].remap_tag_name, v, - &remapped_tags); - if (ret < 0) { - flb_plg_error(ctx->ins, "Failed to remap tag: %s, skipping", remapping[ind].remap_tag_name); - } + remapping[ind].remap_to_tag(remapping[ind].remap_tag_name, v, + remapped_tags); continue; } @@ -280,25 +250,9 @@ static int datadog_format(struct flb_config *config, /* here we concatenate ctx->dd_tags and remapped_tags, depending on their presence */ if (remap_cnt) { if (ctx->dd_tags != NULL) { - tmp = flb_sds_cat(remapped_tags, FLB_DATADOG_TAG_SEPERATOR, - strlen(FLB_DATADOG_TAG_SEPERATOR)); - if (!tmp) { - flb_errno(); - flb_sds_destroy(remapped_tags); - msgpack_sbuffer_destroy(&mp_sbuf); - msgpack_unpacked_destroy(&result); - return -1; - } - remapped_tags = tmp; + flb_sds_cat(remapped_tags, FLB_DATADOG_TAG_SEPERATOR, + strlen(FLB_DATADOG_TAG_SEPERATOR)); flb_sds_cat(remapped_tags, ctx->dd_tags, strlen(ctx->dd_tags)); - if (!tmp) { - flb_errno(); - flb_sds_destroy(remapped_tags); - msgpack_sbuffer_destroy(&mp_sbuf); - msgpack_unpacked_destroy(&result); - return -1; - } - remapped_tags = tmp; } dd_msgpack_pack_key_value_str(&mp_pck, FLB_DATADOG_DD_TAGS_KEY, @@ -365,7 +319,7 @@ static void cb_datadog_flush(struct flb_event_chunk *event_chunk, /* Convert input data into a Datadog JSON payload */ ret = datadog_format(config, i_ins, - ctx, event_chunk, + ctx, NULL, event_chunk->tag, flb_sds_len(event_chunk->tag), event_chunk->data, event_chunk->size, &out_buf, &out_size); diff --git a/plugins/out_datadog/datadog_conf.c b/plugins/out_datadog/datadog_conf.c index 68377386c10..def064c48c5 100644 --- a/plugins/out_datadog/datadog_conf.c +++ b/plugins/out_datadog/datadog_conf.c @@ -33,7 +33,6 @@ struct flb_out_datadog *flb_datadog_conf_create(struct flb_output_instance *ins, struct flb_upstream *upstream; const char *api_key; const char *tmp; - flb_sds_t tmp_sds; int ret; char *protocol = NULL; @@ -76,18 +75,12 @@ struct flb_out_datadog *flb_datadog_conf_create(struct flb_output_instance *ins, /* use TLS ? */ if (ins->use_tls == FLB_TRUE) { io_flags = FLB_IO_TLS; - tmp_sds = flb_sds_create("https://"); + ctx->scheme = flb_sds_create("https://"); } else { io_flags = FLB_IO_TCP; - tmp_sds = flb_sds_create("http://"); + ctx->scheme = flb_sds_create("http://"); } - if (!tmp_sds) { - flb_errno(); - flb_datadog_conf_destroy(ctx); - return NULL; - } - ctx->scheme = tmp_sds; flb_plg_debug(ctx->ins, "scheme: %s", ctx->scheme); /* configure URI */ @@ -133,17 +126,11 @@ struct flb_out_datadog *flb_datadog_conf_create(struct flb_output_instance *ins, /* Get network configuration */ if (!ins->host.name) { - tmp_sds = flb_sds_create(FLB_DATADOG_DEFAULT_HOST); + ctx->host = flb_sds_create(FLB_DATADOG_DEFAULT_HOST); } else { - tmp_sds = flb_sds_create(ins->host.name); - } - if (!tmp_sds) { - flb_errno(); - flb_datadog_conf_destroy(ctx); - return NULL; + ctx->host = flb_sds_create(ins->host.name); } - ctx->host = tmp_sds; flb_plg_debug(ctx->ins, "host: %s", ctx->host); if (ins->host.port != 0) { diff --git a/plugins/out_datadog/datadog_remap.c b/plugins/out_datadog/datadog_remap.c index 7599a8f80a2..d524399589c 100644 --- a/plugins/out_datadog/datadog_remap.c +++ b/plugins/out_datadog/datadog_remap.c @@ -28,172 +28,98 @@ const char *ECS_ARN_PREFIX = "arn:aws:ecs:"; const char *ECS_CLUSTER_PREFIX = "cluster/"; const char *ECS_TASK_PREFIX = "task/"; -static int dd_remap_append_kv_to_ddtags(const char *key, - const char *val, size_t val_len, flb_sds_t *dd_tags_buf) +static void dd_remap_append_kv_to_ddtags(const char *key, + const char *val, size_t val_len, flb_sds_t dd_tags) { - flb_sds_t tmp; - - if (flb_sds_len(*dd_tags_buf) != 0) { - tmp = flb_sds_cat(*dd_tags_buf, FLB_DATADOG_TAG_SEPERATOR, strlen(FLB_DATADOG_TAG_SEPERATOR)); - if (!tmp) { - flb_errno(); - return -1; - } - *dd_tags_buf = tmp; + if (flb_sds_len(dd_tags) != 0) { + flb_sds_cat(dd_tags, FLB_DATADOG_TAG_SEPERATOR, strlen(FLB_DATADOG_TAG_SEPERATOR)); } - - tmp = flb_sds_cat(*dd_tags_buf, key, strlen(key)); - if (!tmp) { - flb_errno(); - return -1; - } - *dd_tags_buf = tmp; - - tmp = flb_sds_cat(*dd_tags_buf, ":", 1); - if (!tmp) { - flb_errno(); - return -1; - } - *dd_tags_buf = tmp; - - tmp = flb_sds_cat(*dd_tags_buf, val, val_len); - if (!tmp) { - flb_errno(); - return -1; - } - *dd_tags_buf = tmp; - - return 0; + flb_sds_cat(dd_tags, key, strlen(key)); + flb_sds_cat(dd_tags, ":", 1); + flb_sds_cat(dd_tags, val, val_len); } /* default remapping: just move the key/val pair under dd_tags */ -static int dd_remap_move_to_tags(const char *tag_name, - msgpack_object attr_value, flb_sds_t *dd_tags_buf) +static void dd_remap_move_to_tags(const char *tag_name, + msgpack_object attr_value, flb_sds_t dd_tags) { - return dd_remap_append_kv_to_ddtags(tag_name, attr_value.via.str.ptr, - attr_value.via.str.size, dd_tags_buf); + dd_remap_append_kv_to_ddtags(tag_name, attr_value.via.str.ptr, + attr_value.via.str.size, dd_tags); } /* remapping function for container_name */ -static int dd_remap_container_name(const char *tag_name, - msgpack_object attr_value, flb_sds_t *dd_tags_buf) +static void dd_remap_container_name(const char *tag_name, + msgpack_object attr_value, flb_sds_t dd_tags) { /* remove the first / if present */ unsigned int adjust; - flb_sds_t buf = NULL; - int ret; + flb_sds_t buf; adjust = attr_value.via.str.ptr[0] == '/' ? 1 : 0; buf = flb_sds_create_len(attr_value.via.str.ptr + adjust, attr_value.via.str.size - adjust); - if (!buf) { - flb_errno(); - return -1; - } - ret = dd_remap_append_kv_to_ddtags(tag_name, buf, strlen(buf), dd_tags_buf); + dd_remap_append_kv_to_ddtags(tag_name, buf, strlen(buf), dd_tags); flb_sds_destroy(buf); - if (ret < 0) { - return -1; - } - - return 0; } /* remapping function for ecs_cluster */ -static int dd_remap_ecs_cluster(const char *tag_name, - msgpack_object attr_value, flb_sds_t *dd_tags_buf) +static void dd_remap_ecs_cluster(const char *tag_name, + msgpack_object attr_value, flb_sds_t dd_tags) { - flb_sds_t buf = NULL; + flb_sds_t buf; char *cluster_name; - int ret; buf = flb_sds_create_len(attr_value.via.str.ptr, attr_value.via.str.size); - if (!buf) { - flb_errno(); - return -1; - } cluster_name = strstr(buf, ECS_CLUSTER_PREFIX); if (cluster_name != NULL) { cluster_name += strlen(ECS_CLUSTER_PREFIX); - ret = dd_remap_append_kv_to_ddtags(tag_name, cluster_name, strlen(cluster_name), dd_tags_buf); - if (ret < 0) { - flb_sds_destroy(buf); - return -1; - } + dd_remap_append_kv_to_ddtags(tag_name, cluster_name, strlen(cluster_name), dd_tags); } else { /* * here the input is invalid: not in form of "XXXXXXcluster/"cluster-name * we preverse the original value under tag "cluster_name". */ - ret = dd_remap_append_kv_to_ddtags(tag_name, buf, strlen(buf), dd_tags_buf); - if (ret < 0) { - flb_sds_destroy(buf); - return -1; - } + dd_remap_append_kv_to_ddtags(tag_name, buf, strlen(buf), dd_tags); } flb_sds_destroy(buf); - return 0; } /* remapping function for ecs_task_definition */ -static int dd_remap_ecs_task_definition(const char *tag_name, - msgpack_object attr_value, flb_sds_t *dd_tags_buf) +static void dd_remap_ecs_task_definition(const char *tag_name, + msgpack_object attr_value, flb_sds_t dd_tags) { - flb_sds_t buf = NULL; + flb_sds_t buf; char *split; - int ret; buf = flb_sds_create_len(attr_value.via.str.ptr, attr_value.via.str.size); - if (!buf) { - flb_errno(); - return -1; - } split = strchr(buf, ':'); if (split != NULL) { - ret = dd_remap_append_kv_to_ddtags("task_family", buf, split-buf, dd_tags_buf); - if (ret < 0) { - flb_sds_destroy(buf); - return -1; - } - ret = dd_remap_append_kv_to_ddtags("task_version", split+1, strlen(split+1), dd_tags_buf); - if (ret < 0) { - flb_sds_destroy(buf); - return -1; - } + dd_remap_append_kv_to_ddtags("task_family", buf, split-buf, dd_tags); + dd_remap_append_kv_to_ddtags("task_version", split+1, strlen(split+1), dd_tags); } else { /* * here the input is invalid: not in form of task_name:task_version * we preverse the original value under tag "ecs_task_definition". */ - ret = dd_remap_append_kv_to_ddtags(tag_name, buf, strlen(buf), dd_tags_buf); - if (ret < 0) { - flb_sds_destroy(buf); - return -1; - } + dd_remap_append_kv_to_ddtags(tag_name, buf, strlen(buf), dd_tags); } flb_sds_destroy(buf); - return 0; } /* remapping function for ecs_task_arn */ -static int dd_remap_ecs_task_arn(const char *tag_name, - msgpack_object attr_value, flb_sds_t *dd_tags_buf) +static void dd_remap_ecs_task_arn(const char *tag_name, + msgpack_object attr_value, flb_sds_t dd_tags) { flb_sds_t buf; char *remain; char *split; char *task_arn; - int ret; buf = flb_sds_create_len(attr_value.via.str.ptr, attr_value.via.str.size); - if (!buf) { - flb_errno(); - return -1; - } /* * if the input is invalid, not in the form of "arn:aws:ecs:region:XXXX" @@ -206,11 +132,7 @@ static int dd_remap_ecs_task_arn(const char *tag_name, split = strchr(remain, ':'); if (split != NULL) { - ret = dd_remap_append_kv_to_ddtags("region", remain, split-remain, dd_tags_buf); - if (ret < 0) { - flb_sds_destroy(buf); - return -1; - } + dd_remap_append_kv_to_ddtags("region", remain, split-remain, dd_tags); } } @@ -218,21 +140,16 @@ static int dd_remap_ecs_task_arn(const char *tag_name, if (task_arn != NULL) { /* parse out the task_arn */ task_arn += strlen(ECS_TASK_PREFIX); - ret = dd_remap_append_kv_to_ddtags(tag_name, task_arn, strlen(task_arn), dd_tags_buf); + dd_remap_append_kv_to_ddtags(tag_name, task_arn, strlen(task_arn), dd_tags); } else { /* * if the input is invalid, not in the form of "XXXXXXXXtask/"task-arn * then we preverse the original value under tag "task_arn". */ - ret = dd_remap_append_kv_to_ddtags(tag_name, buf, strlen(buf), dd_tags_buf); + dd_remap_append_kv_to_ddtags(tag_name, buf, strlen(buf), dd_tags); } flb_sds_destroy(buf); - if (ret < 0) { - return -1; - } - - return 0; } /* diff --git a/plugins/out_datadog/datadog_remap.h b/plugins/out_datadog/datadog_remap.h index f7061b0f2ac..910ee9780d8 100644 --- a/plugins/out_datadog/datadog_remap.h +++ b/plugins/out_datadog/datadog_remap.h @@ -22,12 +22,10 @@ #include "datadog.h" -typedef int (*dd_attr_remap_to_tag_fn)(const char*, msgpack_object, flb_sds_t*); - struct dd_attr_tag_remapping { char* origin_attr_name; /* original attribute name */ char* remap_tag_name; /* tag name to remap to */ - dd_attr_remap_to_tag_fn remap_to_tag; /* remapping function */ + void (*remap_to_tag) (const char*, msgpack_object, flb_sds_t); /* remapping function */ }; extern const struct dd_attr_tag_remapping remapping[];