Skip to content
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

fix(logger): add xray_trace_id to every log #776

Merged

Conversation

dreamorosi
Copy link
Contributor

@dreamorosi dreamorosi commented Apr 15, 2022

Description of your changes

As reported by user @humanzz in #773 the Logger utility was not including the xray_trace_id key in logs. This field is listed in the documentation as a standard key that should appear in each log.

The user reporting the issue was helpful in detailing the fact that the _X_AMZN_TRACE_ID environment variable used to retrieve the value changes at every invocation while Logger was instead attempting to read this value when instantiated (see constructor -> setOptions -> setPowertoolLogData).

This PR changes this behaviour and appends the xray_trace_id key when the log entry is created and populated instead (createAndPopulateLogItem function). This behaviour is consistent with the implementation also used in the Python version of Powertools.

As additional notes:

  • This was not caught in unit tests because the environment variable is set by packages/logger/tests/helpers/populateEnvironmentVariables.ts which is imported before tests are run. So when running the tests the xray_trace_id would always be present.
  • This was not caught in the e2e/integration tests either because no assertion was made on the presence of this key in the logs (this PR adds a test case to check this).
  • The documentation erroneously stated that this key should be set only when Active Tracing is enabled. This is not true as the _X_AMZN_TRACE_ID environment variable is always set in the Lambda execution environment, regardless of whether tracing is enabled or not. This means the key will always be present in each log. AWS Lambda Powertools for Python makes the same statement and I have tested the latest version available of the library on a Lambda function with Active Tracing disabled and the key is also always present in each log.

How to verify this change

  • See unit test results in this PR
  • See result of e2e tests here
  • See example of CloudWatch logs generated after this change (below)

image

Related issues, RFCs

#773

PR status

Is this ready for review?: YES
Is it a breaking change?: NO

Checklist

  • My changes meet the tenets criteria
  • I have performed a self-review of my own code
  • I have commented my code where necessary, particularly in areas that should be flagged with a TODO, or hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • The code coverage hasn't decreased
  • I have added tests that prove my change is effective and works
  • New and existing unit tests pass locally and in Github Actions
  • Any dependent changes have been merged and published in downstream module
  • The PR title follows the conventional commit semantics

Breaking change checklist

N/A


By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@dreamorosi dreamorosi added logger This item relates to the Logger Utility fix labels Apr 15, 2022
@dreamorosi dreamorosi added this to the production-ready-release milestone Apr 15, 2022
@dreamorosi dreamorosi self-assigned this Apr 15, 2022
@dreamorosi dreamorosi linked an issue Apr 15, 2022 that may be closed by this pull request
@github-actions github-actions bot added the bug Something isn't working label Apr 15, 2022
@dreamorosi
Copy link
Contributor Author

docs/core/logger.md Outdated Show resolved Hide resolved
Co-authored-by: ijemmy <ijemmy@users.noreply.github.com>
@dreamorosi dreamorosi requested a review from ijemmy April 20, 2022 13:26
@dreamorosi dreamorosi merged commit 11af21a into main Apr 29, 2022
@dreamorosi dreamorosi deleted the 773-bug-logger-xray_trace_id-is-not-populated-in-log-lines branch April 29, 2022 08:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working logger This item relates to the Logger Utility
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bug: xray_trace_id is not populated in log lines
3 participants