Skip to content

Commit

Permalink
Code Review
Browse files Browse the repository at this point in the history 
Signed-off-by: Greg Eales <[email protected]>
  • Loading branch information
@0x006EA1E5
0x006EA1E5 committed Nov 28, 2024
1 parent 3182f0b commit 1f9301d
Show file tree
Hide file tree
Showing 3 changed files with 88 additions and 52 deletions.
58 changes: 36 additions & 22 deletions plugins/out_loki/loki.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -423,10 +423,11 @@ static void pack_kv(struct flb_loki *ctx,
}
}

/* Similar to pack_kv above, except will only use msgpack_objects of type
* MSGPACK_OBJECT_MAP, and will iterate over the keys adding each entry as a separate
* item. Non-string map values are serialised to JSON, as Loki requires all values to be
* strings.
/*
* Similar to pack_kv above, except will only use msgpack_objects of type
* MSGPACK_OBJECT_MAP, and will iterate over the keys adding each entry as a
* separate item. Non-string map values are serialised to JSON, as Loki requires
* all values to be strings.
*/
static void pack_maps(struct flb_loki *ctx,
msgpack_packer *mp_pck,
Expand Down Expand Up @@ -460,17 +461,20 @@ static void pack_maps(struct flb_loki *ctx,
if (flb_ra_get_kv_pair(kv->ra_key, *map, &start_key, &out_key, &out_val)
== MSGPACK_UNPACK_CONTINUE) {

/* we require the value to be a map, or it doesn't make sense as this is
* adding a map's key / values */
/*
* we require the value to be a map, or it doesn't make sense as
* this is adding a map's key / values
*/
if (out_val->type != MSGPACK_OBJECT_MAP || out_val->via.map.size <= 0) {
flb_plg_debug(ctx->ins, "No valid map data found for key %s",
kv->ra_key->pattern);
} else {
kv->ra_key->pattern);
}
else {
accessed_map = out_val->via.map;

/* for each entry in the accessed map... */
for (accessed_map_index = 0; accessed_map_index < accessed_map.size;
++accessed_map_index) {
accessed_map_index++) {

/* get the entry */
accessed_map_kv = accessed_map.ptr[accessed_map_index];
Expand All @@ -479,21 +483,25 @@ static void pack_maps(struct flb_loki *ctx,
flb_mp_map_header_append(mh);

pack_label_key(mp_pck, (char*) accessed_map_kv.key.via.str.ptr,
accessed_map_kv.key.via.str.size);
/* Does this need optimising? For example, to handle bool as
* non-JSON? */
accessed_map_kv.key.via.str.size);
/*
* Does this need optimising? For example, to handle
* bool as non-JSON?
*/
if (accessed_map_kv.val.type == MSGPACK_OBJECT_STR) {
msgpack_pack_str_with_body(mp_pck,
accessed_map_kv.val.via.str.ptr,
accessed_map_kv.val.via.str.size);
accessed_map_kv.val.via.str.ptr,
accessed_map_kv.val.via.str.size);
}
/* convert value to JSON, as Loki expects a string value */
/*
* convert value to JSON, as Loki expects a string value
*/
else {
accessed_map_val_json = flb_msgpack_to_json_str(1024,
&accessed_map_kv.val);
if (accessed_map_val_json) {
msgpack_pack_str_with_body(mp_pck, accessed_map_val_json,
strlen(accessed_map_val_json));
strlen(accessed_map_val_json));
flb_free(accessed_map_val_json);
}
}
Expand All @@ -513,9 +521,13 @@ static flb_sds_t pack_structured_metadata(struct flb_loki *ctx,
/* Initialize dynamic map header */
flb_mp_map_header_init(&mh, mp_pck);
if (ctx->structured_metadata_map_keys) {
pack_maps(ctx, mp_pck, tag, tag_len, map, &mh, &ctx->structured_metadata_map_keys_list);
pack_maps(ctx, mp_pck, tag, tag_len, map, &mh,
&ctx->structured_metadata_map_keys_list);
}
// explicit structured_metadata entries override structured_metadata_map_keys entries
/*
* explicit structured_metadata entries override
* structured_metadata_map_keys entries
* */
if (ctx->structured_metadata) {
pack_kv(ctx, mp_pck, tag, tag_len, map, &mh, &ctx->structured_metadata_list);
}
Expand Down Expand Up @@ -897,11 +909,13 @@ static int parse_labels(struct flb_loki *ctx)
entry = mk_list_entry(head, struct flb_slist_entry, _head);
if (entry->str[0] != '$') {
flb_plg_error(ctx->ins,
"invalid structured metadata map key, the name must start with '$'");
"invalid structured metadata map key, the name must start "
"with '$'");
return -1;
}

ret = flb_loki_kv_append(ctx, &ctx->structured_metadata_map_keys_list, entry->str, NULL);
ret = flb_loki_kv_append(ctx, &ctx->structured_metadata_map_keys_list,
entry->str, NULL);
if (ret == -1) {
return -1;
}
Expand Down Expand Up @@ -1656,7 +1670,7 @@ static flb_sds_t loki_compose_payload(struct flb_loki *ctx,
&log_decoder,
&log_event)) == FLB_EVENT_DECODER_SUCCESS) {
msgpack_pack_array(&mp_pck, ctx->structured_metadata ||
ctx->structured_metadata_map_keys ? 3 : 2);
ctx->structured_metadata_map_keys ? 3 : 2);

/* Append the timestamp */
pack_timestamp(&mp_pck, &log_event.timestamp);
Expand Down Expand Up @@ -1693,7 +1707,7 @@ static flb_sds_t loki_compose_payload(struct flb_loki *ctx,
msgpack_pack_array(&mp_pck, 1);

msgpack_pack_array(&mp_pck, ctx->structured_metadata ||
ctx->structured_metadata_map_keys ? 3 : 2);
ctx->structured_metadata_map_keys ? 3 : 2);

/* Append the timestamp */
pack_timestamp(&mp_pck, &log_event.timestamp);
Expand Down
14 changes: 7 additions & 7 deletions plugins/out_loki/loki.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -88,14 +88,14 @@ struct flb_loki {
char *tcp_host;
int out_line_format;
int out_drop_single_key;
int ra_used; /* number of record accessor label keys */
struct flb_record_accessor *ra_k8s; /* kubernetes record accessor */
struct mk_list labels_list; /* list of flb_loki_kv nodes */
struct mk_list structured_metadata_list; /* list of flb_loki_kv nodes */
int ra_used; /* number of record accessor label keys */
struct flb_record_accessor *ra_k8s; /* kubernetes record accessor */
struct mk_list labels_list; /* list of flb_loki_kv nodes */
struct mk_list structured_metadata_list; /* list of flb_loki_kv nodes */
struct mk_list structured_metadata_map_keys_list; /* list of flb_loki_kv nodes */
struct mk_list remove_keys_derived; /* remove_keys with label RAs */
struct flb_mp_accessor *remove_mpa; /* remove_keys multi-pattern accessor */
struct flb_record_accessor *ra_tenant_id_key; /* dynamic tenant id key */
struct mk_list remove_keys_derived; /* remove_keys with label RAs */
struct flb_mp_accessor *remove_mpa; /* remove_keys multi-pattern accessor */
struct flb_record_accessor *ra_tenant_id_key; /* dynamic tenant id key */

struct cfl_list dynamic_tenant_list;
pthread_mutex_t dynamic_tenant_list_lock;
Expand Down
68 changes: 45 additions & 23 deletions tests/runtime/out_loki.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -739,6 +739,7 @@ static void cb_check_float_value(void *ctx, int ffd,
flb_sds_destroy(out_js);
}


#define JSON_FLOAT "[12345678, {\"float\":1.3}]"
void flb_test_float_value()
{
Expand Down Expand Up @@ -804,10 +805,14 @@ static void cb_check_structured_metadata_value(void *ctx, int ffd,
flb_sds_destroy(out_js);
}

#define JSON_MAP "[12345678, {\"log\": \"This is an interesting log message!\", \"map1\": {\"key1\": \"value1\", \"key2\": \"value2\", \"key_nested_object_1\": {\"sub_key1\": \"sub_value1\", \"sub_key2\": false}}, \"map2\": {\"key4\": \"value1\", \"key5\": false}, \"map3\": {\"key1\": \"map3_value1\", \"key2\": \"map3_value2\"}}]"
void flb_test_structured_metadata_map_params(char *log_level, char *remove_keys,
char *structured_metadata, char *structured_metadata_map_keys, char *input_log_json,
char *expected_sub_str)
#define JSON_MAP "[12345678, {\"log\": \"This is an interesting log message!\", " \
"\"map1\": {\"key1\": \"value1\", \"key2\": \"value2\", \"key_nested_object_1\": " \
"{\"sub_key1\": \"sub_value1\", \"sub_key2\": false}}, \"map2\": {\"key4\": " \
"\"value1\", \"key5\": false}, \"map3\": {\"key1\": \"map3_value1\", \"key2\": " \
"\"map3_value2\"}}]"
void flb_test_structured_metadata_map_params(char *remove_keys,
char *structured_metadata, char *structured_metadata_map_keys, char *input_log_json,
char *expected_sub_str)
{
int ret;
size_t size = strlen(input_log_json);
Expand All @@ -818,7 +823,7 @@ void flb_test_structured_metadata_map_params(char *log_level, char *remove_keys,
/* Create context, flush every second (some checks omitted here) */
ctx = flb_create();
flb_service_set(ctx, "flush", "1", "grace", "1",
"log_level", log_level ? log_level : "error",
"log_level", "error",
NULL);

/* Lib input mode */
Expand All @@ -830,11 +835,11 @@ void flb_test_structured_metadata_map_params(char *log_level, char *remove_keys,
flb_output_set(ctx, out_ffd,
"match", "test",
"line_format", "key_value",
"remove_keys", remove_keys ? remove_keys : "",
"remove_keys", remove_keys,
"drop_single_key", "on",
"labels", "service_name=my_service_name",
"structured_metadata", structured_metadata ? structured_metadata : "",
"structured_metadata_map_keys", structured_metadata_map_keys ? structured_metadata_map_keys : "",
"structured_metadata", structured_metadata,
"structured_metadata_map_keys", structured_metadata_map_keys,
NULL);

/* Enable test mode */
Expand All @@ -855,61 +860,75 @@ void flb_test_structured_metadata_map_params(char *log_level, char *remove_keys,
flb_destroy(ctx);
}

#define LOG_LEVEL "error"
void flb_test_structured_metadata_map_single_map() {
flb_test_structured_metadata_map_params(
LOG_LEVEL,
"map1, map2, map3",
"",
"$map1",
JSON_MAP,
"{\"key1\":\"value1\",\"key2\":\"value2\",\"key_nested_object_1\":\"{\\\"sub_key1\\\":\\\"sub_value1\\\",\\\"sub_key2\\\":false}\"}");
"{\"key1\":\"value1\",\"key2\":\"value2\","
"\"key_nested_object_1\":\"{\\\"sub_key1\\\":\\\"sub_value1\\\","
"\\\"sub_key2\\\":false}\"}");
}

void flb_test_structured_metadata_map_two_maps() {
flb_test_structured_metadata_map_params(
LOG_LEVEL,
"map1, map2, map3",
"",
"$map1,$map2",
JSON_MAP,
"{\"key1\":\"value1\",\"key2\":\"value2\",\"key_nested_object_1\":\"{\\\"sub_key1\\\":\\\"sub_value1\\\",\\\"sub_key2\\\":false}\",\"key4\":\"value1\",\"key5\":\"false\"}");
"{\"key1\":\"value1\",\"key2\":\"value2\","
"\"key_nested_object_1\":\"{\\\"sub_key1\\\":\\\"sub_value1\\\","
"\\\"sub_key2\\\":false}\",\"key4\":\"value1\",\"key5\":\"false\"}");
}

void flb_test_structured_metadata_map_sub_map() {
flb_test_structured_metadata_map_params(
"map1, map2, map3",
"",
"$map1['key_nested_object_1']",
JSON_MAP,
"\"This is an interesting log message!\",{\"sub_key1\":\"sub_value1\","
"\"sub_key2\":\"false\"}");
}

void flb_test_structured_metadata_map_both_with_non_map_value() {
flb_test_structured_metadata_map_params(
LOG_LEVEL,
"map1, map2, map3",
"$map2",
"$map1,$map2",
JSON_MAP,
"{\"key1\":\"value1\",\"key2\":\"value2\",\"key_nested_object_1\":\"{\\\"sub_key1\\\":\\\"sub_value1\\\",\\\"sub_key2\\\":false}\",\"key4\":\"value1\",\"key5\":\"false\",\"map2\":\"{\\\"key4\\\":\\\"value1\\\",\\\"key5\\\":false}\"}");
"{\"key1\":\"value1\",\"key2\":\"value2\","
"\"key_nested_object_1\":\"{\\\"sub_key1\\\":\\\"sub_value1\\\","
"\\\"sub_key2\\\":false}\",\"key4\":\"value1\",\"key5\":\"false\","
"\"map2\":\"{\\\"key4\\\":\\\"value1\\\",\\\"key5\\\":false}\"}");
}

/* key1 is overridden by the explicit value given to structured_metadata */
void flb_test_structured_metadata_map_value_explicit_override_map_key() {
flb_test_structured_metadata_map_params(
LOG_LEVEL,
"map1, map2, map3",
"key1=value_explicit",
"$map1,$map2",
JSON_MAP,
"{\"key2\":\"value2\",\"key_nested_object_1\":\"{\\\"sub_key1\\\":\\\"sub_value1\\\",\\\"sub_key2\\\":false}\",\"key4\":\"value1\",\"key5\":\"false\",\"key1\":\"value_explicit\"}");
"{\"key2\":\"value2\","
"\"key_nested_object_1\":\"{\\\"sub_key1\\\":\\\"sub_value1\\\","
"\\\"sub_key2\\\":false}\",\"key4\":\"value1\",\"key5\":\"false\","
"\"key1\":\"value_explicit\"}");
}

void flb_test_structured_metadata_explicit_only_no_map() {
flb_test_structured_metadata_map_params(
LOG_LEVEL,
"map1, map2, map3",
"key1=value_explicit",
"",
JSON_MAP,
"[\"12345678000000000\",\"This is an interesting log message!\",{\"key1\":\"value_explicit\"}]");
"[\"12345678000000000\","
"\"This is an interesting log message!\",{\"key1\":\"value_explicit\"}]");
}

void flb_test_structured_metadata_explicit_only_map() {
flb_test_structured_metadata_map_params(
LOG_LEVEL,
"map1, map2, map3",
"$map2",
"",
Expand All @@ -919,17 +938,18 @@ void flb_test_structured_metadata_explicit_only_map() {

void flb_test_structured_metadata_map_and_explicit() {
flb_test_structured_metadata_map_params(
LOG_LEVEL,
"map1, map2, map3",
"key_explicit=value_explicit",
"$map1",
JSON_MAP,
"[\"12345678000000000\",\"This is an interesting log message!\",{\"key1\":\"value1\",\"key2\":\"value2\",\"key_nested_object_1\":\"{\\\"sub_key1\\\":\\\"sub_value1\\\",\\\"sub_key2\\\":false}\",\"key_explicit\":\"value_explicit\"}]");
"[\"12345678000000000\",\"This is an interesting log message!\","
"{\"key1\":\"value1\",\"key2\":\"value2\","
"\"key_nested_object_1\":\"{\\\"sub_key1\\\":\\\"sub_value1\\\","
"\\\"sub_key2\\\":false}\",\"key_explicit\":\"value_explicit\"}]");
}

void flb_test_structured_metadata_map_single_missing_map() {
flb_test_structured_metadata_map_params(
LOG_LEVEL,
"map1, map2, map3",
"",
"$missing_map",
Expand All @@ -955,6 +975,8 @@ TEST_LIST = {
flb_test_structured_metadata_map_single_map},
{"structured_metadata_map_two_maps",
flb_test_structured_metadata_map_two_maps},
{"structured_metadata_map_sub_map",
flb_test_structured_metadata_map_sub_map},
{"structured_metadata_map_both_with_non_map_value",
flb_test_structured_metadata_map_both_with_non_map_value},
{"structured_metadata_map_value_explicit_override_map_key",
Expand Down

0 comments on commit 1f9301d

Please sign in to comment.