Skip to content

Conversation

heitorlessa
Copy link
Contributor

Issue number: #2835

Summary

This PR addresses the issue of X-Ray Trace ID key (xray_trace_id) not being removed despite being explicitly disabled.

Changes

Please provide a summary of what's being changed

User experience

Please share what the user experience looks like before and after this change

bla.py

logger = Logger(xray_trace_id=None)
logger.info("blah")

Setting X-Ray Trace ID env var produces no effect when explicitly disabling at Logger constructor (as expected now)

_X_AMZN_TRACE_ID="Root=1-5759e988-bd862e3fe1be46a994272793;Parent=53995c3f42cd8ad8;Sampled=1" python bla.py

Output

{
    "level": "INFO",
    "location": "<module>:4",
    "message": "blah",
    "timestamp": "2023-07-26 14:17:53,765+0200",
    "service": "service_undefined"
}

Checklist

If your change doesn't seem to apply, please leave them unchecked.

Is this a breaking change?

RFC issue number:

Checklist:

  • Migration process documented
  • Implement warnings (if it can live side by side)

Acknowledgment

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

Disclaimer: We value your time and bandwidth. As such, any pull requests created on non-triaged issues might not be successful.

@heitorlessa heitorlessa requested a review from a team July 26, 2023 12:20
@pull-request-size pull-request-size bot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Jul 26, 2023
@github-actions github-actions bot added the bug Something isn't working label Jul 26, 2023
@codecov-commenter
Copy link

codecov-commenter commented Jul 26, 2023

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (37c3c62) 97.57% compared to head (b123adb) 97.57%.
Report is 1 commits behind head on develop.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

Additional details and impacted files
@@           Coverage Diff            @@
##           develop    #2852   +/-   ##
========================================
  Coverage    97.57%   97.57%           
========================================
  Files          162      162           
  Lines         7473     7475    +2     
  Branches      1416     1416           
========================================
+ Hits          7292     7294    +2     
  Misses         133      133           
  Partials        48       48           
Files Changed Coverage Δ
aws_lambda_powertools/logging/formatter.py 100.00% <100.00%> (ø)

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@leandrodamascena leandrodamascena left a comment

Choose a reason for hiding this comment

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

Hei @heitorlessa! I tried to find information about this parameter xray_trace_id in our documentation and I couldn't find. Should we add a section or banner in the Advanced section to explain this?

@heitorlessa
Copy link
Contributor Author

It's documented under Overriding Log Record: https://docs.powertools.aws.dev/lambda/python/latest/core/logger/#overriding-log-records

image

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.5% 0.5% Duplication

@heitorlessa heitorlessa merged commit 7dbdd81 into aws-powertools:develop Jul 26, 2023
@heitorlessa heitorlessa deleted the fix/logger-xray-trace-id branch July 26, 2023 13:15
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 size/S Denotes a PR that changes 10-29 lines, ignoring generated files. tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants