Stackdriver Trace and Custom Log Name Support #2683

tpetrov-lp · 2020-10-13T20:06:51Z

Adds trace to stackdriver output. This is necessary for stackdriver to correlate request logs with app logs. See https://medium.com/google-cloud/combining-correlated-log-lines-in-google-stackdriver-dd23284aeb29
Adds support for overriding the log name sent to stackdriver. By default, it's projects/<project id>/logs/<tag>, but <tag> should be replaceable by a field from the log entry, because this will allow to group the logs properly and visualize in the logs dropdown in stackdriver (that's especially useful for some engineers monitoring stackdriver).
Removed severity from the JSON payload sent to stackdriver, because it is a duplicate of the value in the root.

Testing
It was tested by modifying the hello world app to write to stackdriver. Take a look at hello_world.txt. The debug log + the output from valgrind can be found at valgrind.txt

Latest from base as of Oct 6, 2020

Fluent-bit from base as of Oct 13, 2020

tpetrov-lp · 2020-10-13T20:12:41Z

plugins/out_stackdriver/stackdriver.c

        monitored_resource_key,
        local_resource_id_key,
        ctx->labels_key,
+        ctx->trace_key,


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.

… correlate request logs with app logs. See https://medium.com/google-cloud/combining-correlated-log-lines-in-google-stackdriver-dd23284aeb29 Signed-off-by: Todor Petrov <[email protected]>

…moval from the jsonPayload. Signed-off-by: Todor Petrov <[email protected]>

… so there is no need to be duplicated. (this might count as a breaking change, so if needed a flag might be added whether to remove it or not from the jsonPayload) Signed-off-by: Todor Petrov <[email protected]>

…nt to stackdriver - Added support for overriding the logName sent to stackdriver. By default, it's `projects/<project id>/logs/<tag>`, but `<tag>` should be replacable by a field from the log entry, because this will allow to group the logs properly and visualize in the logs dropdown in stackdriver. Signed-off-by: Todor Petrov <[email protected]>

Sync with base origin/master as of Oct 16, 2020

…ver-trace-support

tpetrov-lp · 2020-10-16T17:15:20Z

@edsiper @koleini @fujimotos @PettitWesley can you guys, please take a look. I am not sure if I have to do anything else rather than opening a PR and how long it usually takes for someone to take a look but wanted to check what your thoughts are. My preference is to keep our fork in sync with the original repo, rather than writing customisations which will not make their way in the base repo. That's why your input will be highly appreciated.

PettitWesley · 2020-10-17T03:28:29Z

I believe there are some folks from Google who help maintain the stackdriver output. I think @JeffLuoo is one of them.

JeffLuoo · 2020-10-17T03:37:12Z

Hi @tpetrov-lp Thanks for the contribution!
cc @StevenYCChou Hi Yen-Cheng Do you have any idea on this PR?

tpetrov-lp · 2020-10-19T12:32:49Z

While, waiting for @StevenYCChou 's feedback, I also remembered that the stackdriver doc should be updated after this PR. What's the process for this?

StevenYCChou · 2020-10-19T14:23:23Z

@qingling128 @davidbtucker Can you take a look on this? This PR also includes change for log name which we have discussed before. Thanks!

JeffLuoo · 2020-10-19T18:24:04Z

@tpetrov-lp The documentation for the stackdriver plugin is here: https://github.com/fluent/fluent-bit-docs/blob/master/pipeline/outputs/stackdriver.md

In order to update the doc, just submit a PR on the fluent bit doc repo here: https://github.com/fluent/fluent-bit-docs

qingling128 · 2020-10-19T21:24:46Z

plugins/out_stackdriver/stackdriver.h

 #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"


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?

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.

qingling128 · 2020-10-19T21:34:10Z

tests/runtime/out_stackdriver.c

    flb_sds_destroy(res_data);
 }

+static void cb_check_trace_common_case(void *ctx, int ffd,


For the reference, we have similar implementation at the Fluentd side to extract logging.googleapis.com/trace:

The value of this field should be formatted as projects/[PROJECT-ID]/traces/[TRACE-ID], so it can be used by the Logs Explorer and the Trace Viewer to group log entries and display them in line with traces. If `autoformat_stackdriver_trace` is true and [V] matches the format of ResourceTrace traceId, the LogEntry trace field has the value projects/[PROJECT-ID]/traces/[V].

Related documentation:
https://cloud.google.com/trace/docs/reference/v1/rest/v1/projects.traces#Trace
https://cloud.google.com/logging/docs/reference/v2/rest/v2/LogEntry
https://cloud.google.com/logging/docs/agent/configuration#special-fields

I reviewed the docs you shared and I think what we can do is to add autoformat_stackdriver_trace in the stackdriver output configuration. If it is set to true, then we can format the trace as projects/[PROJECT-ID]/traces/[TRACE-ID], where TRACE-ID is record['trace_key_value'].

E.g.

Output is configured like:

[OUTPUT] Name stackdriver Match stackdriver.* Resource k8s_container Trace_Key tag

Then the log entry will have trace=record['tag']. However if we have:

[OUTPUT] Name stackdriver Match stackdriver.* Resource k8s_container Trace_Key tag Autoformat_Stackdriver_Trace true

then trace=projects/[PROJECT-ID]/traces/[TRACE-ID] and [TRACE-ID]=record['tag']

Is this what you suggest?

Currently we have short traces without project id and then web requests are coming from istio where the trace is also short and stackdriver properly aggregates everything. So we don't necessarily want to have the full trace format.

What do you think?

@qingling128

Sounds good to me (Sorry I somehow missed the email notification).

Done! @qingling128 can you take a look? This PR has been open for quite a long time, I'll be very grateful and happy if we can close it soon.

Just LGTM'd

- Added support for `autoformat_stackdriver_trace` to format the trace as `projects/[PROJECT-ID]/traces/[TRACE-ID]` when enabled Signed-off-by: Todor Petrov <[email protected]>

tpetrov-lp · 2020-11-14T10:09:52Z

@qingling128 @JeffLuoo @StevenYCChou @PettitWesley do we need someone else to take a look or once @qingling128 is happy with this PR, it will be ready for merge? If we need someone else, it will be good to start this review in parallel, because it's been open for a long time now and it will be nice if we can close it sooner than later. Thanks a lot for the cooperation, guys!

edsiper · 2020-11-15T05:22:08Z

if @qingling128 is ok with this PR I can merge it.

qingling128

LGTM

edsiper · 2020-11-17T20:45:54Z

thanks!

- Add trace to stackdriver output. This is necessary for stackdriver to correlate request logs with app logs. See https://medium.com/google-cloud/combining-correlated-log-lines-in-google-stackdriver-dd23284aeb29 - Added a default label for trace_key which is needed for the proper removal from the jsonPayload. Signed-off-by: Todor Petrov <[email protected]>

tpetrov-lp · 2020-11-24T18:26:41Z

thanks too! I still need to update the docs repo, because we added some new arguments.

tpetrov-lp added 2 commits October 7, 2020 17:05

Merge pull request #1 from fluent/master

3849ae8

Latest from base as of Oct 6, 2020

Merge pull request #2 from fluent/master

d1c5b21

Fluent-bit from base as of Oct 13, 2020

tpetrov-lp requested review from edsiper, fujimotos and koleini as code owners October 13, 2020 20:06

tpetrov-lp commented Oct 13, 2020

View reviewed changes

Add trace to stackdriver output. This is necessary for stackdriver to…

237d727

… correlate request logs with app logs. See https://medium.com/google-cloud/combining-correlated-log-lines-in-google-stackdriver-dd23284aeb29 Signed-off-by: Todor Petrov <[email protected]>

tpetrov-lp force-pushed the stackdriver-trace-support branch from d2b9935 to 237d727 Compare October 13, 2020 20:18

Added a default label for trace_key which is needed for the proper re…

6d886bc

…moval from the jsonPayload. Signed-off-by: Todor Petrov <[email protected]>

tpetrov-lp force-pushed the stackdriver-trace-support branch from 8c898ef to 6d886bc Compare October 15, 2020 15:09

tpetrov-lp added 2 commits October 16, 2020 17:30

Removed severity from jsonPayload, because it is moved to the root,…

3a0d2cc

… so there is no need to be duplicated. (this might count as a breaking change, so if needed a flag might be added whether to remove it or not from the jsonPayload) Signed-off-by: Todor Petrov <[email protected]>

tpetrov-lp changed the title ~~Stackdriver Trace Support~~ Stackdriver Trace and Custom Log Name Support Oct 16, 2020

tpetrov-lp and others added 2 commits October 16, 2020 18:58

Merge pull request #3 from fluent/master

73df60c

Sync with base origin/master as of Oct 16, 2020

Merge branch 'master' of github.com:Leanplum/fluent-bit into stackdri…

0a2c653

…ver-trace-support

qingling128 reviewed Oct 19, 2020

View reviewed changes

- Added default logname key logging.googleapis.com/logName

97e64e5

- Added support for `autoformat_stackdriver_trace` to format the trace as `projects/[PROJECT-ID]/traces/[TRACE-ID]` when enabled Signed-off-by: Todor Petrov <[email protected]>

tpetrov-lp force-pushed the stackdriver-trace-support branch from e016ccf to 97e64e5 Compare November 14, 2020 10:00

qingling128 approved these changes Nov 17, 2020

View reviewed changes

edsiper merged commit b7c7081 into fluent:master Nov 17, 2020

Stackdriver Trace and Custom Log Name Support #2683

Stackdriver Trace and Custom Log Name Support #2683

Uh oh!

Conversation

tpetrov-lp commented Oct 13, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tpetrov-lp Oct 13, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tpetrov-lp commented Oct 16, 2020

Uh oh!

PettitWesley commented Oct 17, 2020

Uh oh!

JeffLuoo commented Oct 17, 2020

Uh oh!

tpetrov-lp commented Oct 19, 2020

Uh oh!

StevenYCChou commented Oct 19, 2020

Uh oh!

JeffLuoo commented Oct 19, 2020

Uh oh!

qingling128 Oct 19, 2020

Choose a reason for hiding this comment

Uh oh!

tpetrov-lp Oct 20, 2020

Choose a reason for hiding this comment

Uh oh!

tpetrov-lp Nov 14, 2020

Choose a reason for hiding this comment

Uh oh!

qingling128 Oct 19, 2020

Choose a reason for hiding this comment

Uh oh!

tpetrov-lp Oct 20, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

qingling128 Nov 10, 2020

Choose a reason for hiding this comment

Uh oh!

tpetrov-lp Nov 14, 2020

Choose a reason for hiding this comment

Uh oh!

qingling128 Nov 17, 2020

Choose a reason for hiding this comment

Uh oh!

tpetrov-lp commented Nov 14, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

edsiper commented Nov 15, 2020

Uh oh!

qingling128 left a comment

Choose a reason for hiding this comment

Uh oh!

edsiper commented Nov 17, 2020

Uh oh!

tpetrov-lp commented Nov 24, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

tpetrov-lp commented Oct 13, 2020 •

edited

Loading

tpetrov-lp Oct 13, 2020 •

edited

Loading

tpetrov-lp Oct 20, 2020 •

edited

Loading

tpetrov-lp commented Nov 14, 2020 •

edited

Loading