Skip to content

Add documentation and example for adding request ID to logs. #65

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.

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 6 commits into from
Mar 31, 2020

Conversation

agocs
Copy link
Contributor

@agocs agocs commented Mar 27, 2020

What does this PR do?

This adds documentation around how to add the AWS request ID to logs when using a custom logger, in order to achieve trace-log correlation.

@agocs agocs requested a review from a team as a code owner March 27, 2020 19:44
@codecov-io
Copy link

codecov-io commented Mar 27, 2020

Codecov Report

Merging #65 into master will not change coverage by %.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master      #65   +/-   ##
=======================================
  Coverage   90.72%   90.72%           
=======================================
  Files          25       25           
  Lines         744      744           
  Branches      122      122           
=======================================
  Hits          675      675           
  Misses         43       43           
  Partials       26       26           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3d02db4...ec0296c. Read the comment docs.

Copy link
Contributor

@pinkerton pinkerton left a comment

Choose a reason for hiding this comment

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

Copy looks good. I defer to someone on the engineering side to verify the technical details and code example :)

@agocs
Copy link
Contributor Author

agocs commented Mar 27, 2020

The example is based on https://getpino.io/#/?id=usage , for reference.

@@ -136,6 +136,36 @@ sendDistributionMetric(

If your Lambda function is associated with a VPC, you need to ensure it has access to the [public internet](https://aws.amazon.com/premiumsupport/knowledge-center/internet-access-lambda-function/).

Copy link
Contributor

Choose a reason for hiding this comment

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

I think this title can be misleading, because this only applies when you are not using datadog tracer.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should have a section Trace & Log Correlation and break it down by using datadog tracer and using xray tracer.

Copy link
Contributor

@tianchu tianchu Mar 27, 2020

Choose a reason for hiding this comment

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

Let's move the description for DD_LOGS_INJECTION to this new section with a link, since it's getting a little bit complicated.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Using the datadog tracer, trace-log correlation should just work, right? Or is there something special they need to do?

Copy link
Contributor

Choose a reason for hiding this comment

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

Correct, it just works if I'm using datadog tracer. However, reading this section as a user, I wouldn't know that. Even if I'm using the datadog tracer, I still might think I need to follow the instructions in this section.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this is a super big deal though, definitely not a blocker for merging this PR, we can always improve the structure later.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍 Updated!

Co-Authored-By: Stephen Firrincieli <[email protected]>
README.md Outdated
for `console.log()`, but you will have to implement this for other logging libraries.

The AWS Request ID is available in the context that is passed to your lambda handler,
as `context.awsRequestId`. It should be included in your log line as `@lambda.request_id`.
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this @ sign needed?

README.md Outdated
See instructions for [manual](https://docs.datadoghq.com/tracing/connect_logs_and_traces/?tab=nodejs#manual-trace-id-injection) trace id injection, if using other logging libraries.

Set the environment variable `DD_LOGS_INJECTION` to `false` to disable this feature.
Controlls whether or not the request ID is injected into log lines. See [DD_LOGS_INJECTION](#DD_LOGS_INJECTION-environment-variable) under
Copy link
Contributor

Choose a reason for hiding this comment

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

@DarcyRaynerDD correct me if i'm wrong. I thought DD_LOG_INJECTION only inject datadog trace id into logs?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay yep, I misread the other section.

README.md Outdated

### Using the Datadog Tracer

If you are using the [Datadog Tracer](#datadog-tracer-experimental), your log messages
Copy link
Contributor

Choose a reason for hiding this comment

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

This isn't quite true. You still need to follow the steps (here)[https://docs.datadoghq.com/tracing/connect_logs_and_traces/nodejs/]

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍 Thanks. Fixed.

@agocs agocs merged commit 4f40809 into master Mar 31, 2020
@agocs agocs deleted the chris.agocs/docs_for_adding_request_id_to_logs branch March 31, 2020 14:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants