Skip to content

Stackdriver Trace and Custom Log Name Support #2683

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
Nov 17, 2020
Merged

Stackdriver Trace and Custom Log Name Support #2683

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
3849ae8
Merge pull request #1 from fluent/master
tpetrov-lp Oct 7, 2020
d1c5b21
Merge pull request #2 from fluent/master
tpetrov-lp Oct 13, 2020
237d727
Add trace to stackdriver output. This is necessary for stackdriver to…
tpetrov-lp Oct 13, 2020
6d886bc
Added a default label for trace_key which is needed for the proper re…
tpetrov-lp Oct 15, 2020
3a0d2cc
Removed severity from `jsonPayload`, because it is moved to the root,…
tpetrov-lp Oct 15, 2020
3912b90
- Added unit tests for the removed severity field from jsonPayload se…
tpetrov-lp Oct 16, 2020
73df60c
Merge pull request #3 from fluent/master
tpetrov-lp Oct 16, 2020
0a2c653
Merge branch 'master' of github.com:Leanplum/fluent-bit into stackdri…
tpetrov-lp Oct 16, 2020
97e64e5
- Added default logname key `logging.googleapis.com/logName`
tpetrov-lp Nov 14, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
76 changes: 71 additions & 5 deletions plugins/out_stackdriver/stackdriver.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -994,6 +994,18 @@ static int get_msgpack_obj(msgpack_object * subobj, const msgpack_object * o,
return -1;
}

static int get_string(flb_sds_t * s, const msgpack_object * o, const flb_sds_t key)
{
msgpack_object tmp;
if (get_msgpack_obj(&tmp, o, key, flb_sds_len(key), MSGPACK_OBJECT_STR) == 0) {
*s = flb_sds_create_len(tmp.via.str.ptr, tmp.via.str.size);
return 0;
}

*s = 0;
return -1;
}

static int get_severity_level(severity_t * s, const msgpack_object * o,
const flb_sds_t key)
{
Expand All @@ -1006,6 +1018,7 @@ static int get_severity_level(severity_t * s, const msgpack_object * o,
return -1;
}


static int get_stream(msgpack_object_map map)
{
int i;
Expand Down Expand Up @@ -1104,8 +1117,13 @@ static int pack_json_payload(int insert_id_extracted,
monitored_resource_key,
local_resource_id_key,
ctx->labels_key,
ctx->severity_key,
ctx->trace_key,
Copy link
Contributor Author

@tpetrov-lp tpetrov-lp Oct 13, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ctx->severity_key is added to the list to remove the duplicate in jsonPayload, but this can be treated as a breaking change if someone relies on it in jsonPayload. If we follow the semantic versioning, this will require changing the major version. Please, advise if we need any special flag which controls whether or not to remove the severity key from the payload.

ctx->log_name_key,
stream
/* more special fields are required to be added */
/* more special fields are required to be added, but, if this grows with more
than a few records, it might need to be converted to flb_hash
*/
};

if (insert_id_extracted == FLB_TRUE) {
Expand Down Expand Up @@ -1138,7 +1156,7 @@ static int pack_json_payload(int insert_id_extracted,
* check length of key to avoid partial matching
* e.g. labels key = labels && kv->key = labelss
*/
if (flb_sds_cmp(removed, kv->key.via.str.ptr, len) == 0) {
if (removed && flb_sds_cmp(removed, kv->key.via.str.ptr, len) == 0) {
to_remove += 1;
break;
}
Expand Down Expand Up @@ -1215,7 +1233,7 @@ static int pack_json_payload(int insert_id_extracted,
len = kv->key.via.str.size;
for (j = 0; j < len_to_be_removed; j++) {
removed = to_be_removed[j];
if (flb_sds_cmp(removed, kv->key.via.str.ptr, len) == 0) {
if (removed && flb_sds_cmp(removed, kv->key.via.str.ptr, len) == 0) {
key_not_found = 0;
break;
}
Expand Down Expand Up @@ -1264,6 +1282,7 @@ static int stackdriver_format(struct flb_config *config,
char path[PATH_MAX];
char time_formatted[255];
const char *newtag;
const char *new_log_name;
msgpack_object *obj;
msgpack_object *labels_ptr;
msgpack_unpacked result;
Expand All @@ -1276,6 +1295,14 @@ static int stackdriver_format(struct flb_config *config,
int severity_extracted = FLB_FALSE;
severity_t severity;

/* Parameters for trace */
int trace_extracted = FLB_FALSE;
flb_sds_t trace;

/* Parameters for log name */
int log_name_extracted = FLB_FALSE;
flb_sds_t log_name;

/* Parameters for insertId */
msgpack_object insert_id_obj;
insert_id_status in_status;
Expand Down Expand Up @@ -1581,7 +1608,8 @@ static int stackdriver_format(struct flb_config *config,
* "labels": "...",
* "logName": "...",
* "jsonPayload": {...},
* "timestamp": "..."
* "timestamp": "...",
* "trace": "..."
* }
*/
entry_size = 3;
Expand All @@ -1594,6 +1622,21 @@ static int stackdriver_format(struct flb_config *config,
entry_size += 1;
}

/* Extract trace */
trace_extracted = FLB_FALSE;
if (ctx->trace_key
&& get_string(&trace, obj, ctx->trace_key) == 0) {
trace_extracted = FLB_TRUE;
entry_size += 1;
}

/* Extract log name */
log_name_extracted = FLB_FALSE;
if (ctx->log_name_key
&& get_string(&log_name, obj, ctx->log_name_key) == 0) {
log_name_extracted = FLB_TRUE;
}

/* Extract insertId */
in_status = validate_insert_id(&insert_id_obj, obj);
if (in_status == INSERTID_VALID) {
Expand Down Expand Up @@ -1668,6 +1711,17 @@ static int stackdriver_format(struct flb_config *config,
msgpack_pack_int(&mp_pck, severity);
}

/* Add trace into the log entry */
if (trace_extracted == FLB_TRUE) {
msgpack_pack_str(&mp_pck, 5);
msgpack_pack_str_body(&mp_pck, "trace", 5);

len = flb_sds_len(trace);
msgpack_pack_str(&mp_pck, len);
msgpack_pack_str_body(&mp_pck, trace, len);
flb_sds_destroy(trace);
}

/* Add insertId field into the log entry */
if (insert_id_extracted == FLB_TRUE) {
msgpack_pack_str(&mp_pck, 8);
Expand Down Expand Up @@ -1729,9 +1783,21 @@ static int stackdriver_format(struct flb_config *config,
newtag = "stderr";
}
}

if (log_name_extracted == FLB_FALSE) {
new_log_name = newtag;
}
else {
new_log_name = log_name;
}

/* logName */
len = snprintf(path, sizeof(path) - 1,
"projects/%s/logs/%s", ctx->project_id, newtag);
"projects/%s/logs/%s", ctx->project_id, new_log_name);

if (log_name_extracted == FLB_TRUE) {
flb_sds_destroy(log_name);
}

msgpack_pack_str(&mp_pck, 7);
msgpack_pack_str_body(&mp_pck, "logName", 7);
Expand Down
4 changes: 4 additions & 0 deletions plugins/out_stackdriver/stackdriver.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,8 @@
#define MONITORED_RESOURCE_KEY "logging.googleapis.com/monitored_resource"
#define LOCAL_RESOURCE_ID_KEY "logging.googleapis.com/local_resource_id"
#define DEFAULT_LABELS_KEY "logging.googleapis.com/labels"
#define DEFAULT_SEVERITY_KEY "logging.googleapis.com/severity"
Copy link
Collaborator

@qingling128 qingling128 Oct 19, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we keep the log_name_key in the same logging.googleapis.com namespace (similar to the other Google Cloud Logging LogEntry fields https://cloud.google.com/logging/docs/reference/v2/rest/v2/LogEntry) for consistence and also to help avoid conflicting with user / custom labels?

Copy link
Contributor Author

@tpetrov-lp tpetrov-lp Oct 20, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking originally that if we don't have a log_name_key set, then we should use the default way of setting the logName, but now when I think again, if we don't have it under logging.googleapis.com/logName, it will work the same way. so yes, I can add this one.

Copy link
Contributor Author

@tpetrov-lp tpetrov-lp Nov 14, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

#define DEFAULT_TRACE_KEY "logging.googleapis.com/trace"
#define DEFAULT_INSERT_ID_KEY "logging.googleapis.com/insertId"
#define SOURCELOCATION_FIELD_IN_JSON "logging.googleapis.com/sourceLocation"
#define HTTPREQUEST_FIELD_IN_JSON "logging.googleapis.com/http_request"
Expand Down Expand Up @@ -108,6 +110,8 @@ struct flb_stackdriver {
/* other */
flb_sds_t resource;
flb_sds_t severity_key;
flb_sds_t trace_key;
flb_sds_t log_name_key;

/* oauth2 context */
struct flb_oauth2 *o;
Expand Down
20 changes: 19 additions & 1 deletion plugins/out_stackdriver/stackdriver_conf.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -268,8 +268,24 @@ struct flb_stackdriver *flb_stackdriver_conf_create(struct flb_output_instance *
if (tmp) {
ctx->severity_key = flb_sds_create(tmp);
}
else {
ctx->severity_key = flb_sds_create(DEFAULT_SEVERITY_KEY);
}

tmp = flb_output_get_property("trace_key", ins);
if (tmp) {
ctx->trace_key = flb_sds_create(tmp);
}
else {
ctx->trace_key = flb_sds_create(DEFAULT_TRACE_KEY);
}

tmp = flb_output_get_property("log_name_key", ins);
if (tmp) {
ctx->log_name_key = flb_sds_create(tmp);
}

if (flb_sds_cmp(ctx->resource, "k8s_container",
if (flb_sds_cmp(ctx->resource, "k8s_container",
flb_sds_len(ctx->resource)) == 0 ||
flb_sds_cmp(ctx->resource, "k8s_node",
flb_sds_len(ctx->resource)) == 0 ||
Expand Down Expand Up @@ -344,6 +360,8 @@ int flb_stackdriver_conf_destroy(struct flb_stackdriver *ctx)
flb_sds_destroy(ctx->token_uri);
flb_sds_destroy(ctx->resource);
flb_sds_destroy(ctx->severity_key);
flb_sds_destroy(ctx->trace_key);
flb_sds_destroy(ctx->log_name_key);
flb_sds_destroy(ctx->labels_key);
flb_sds_destroy(ctx->tag_prefix);

Expand Down
10 changes: 10 additions & 0 deletions tests/runtime/data/stackdriver/stackdriver_test_log_name.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
#define LOG_NAME_OVERRIDE "[" \
"1591111124," \
"{" \
"\"custom_log_name_key\": \"custom_log_name\"" \
"}]"

#define LOG_NAME_NO_OVERRIDE "[" \
"1591111124," \
"{" \
"}]"
6 changes: 6 additions & 0 deletions tests/runtime/data/stackdriver/stackdriver_test_trace.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
#define TRACE_COMMON_CASE "[" \
"1591111124," \
"{" \
"\"trace\": \"test-trace-id-xyz\"" \
"}]"

Loading