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

feat(sdk-logs): make dropping attribute print message #4614

Conversation

HyunnoH
Copy link
Contributor

@HyunnoH HyunnoH commented Apr 8, 2024

Which problem is this PR solving?

Make dropping attribute print message.

Fixes #3762

Short description of the changes

Type of change

Please delete options that are not relevant.

  • New feature (non-breaking change which adds functionality)

How Has This Been Tested?

  • Add below test cases to test message call:
    • Message should not be printed when there is no attribute drop
    • Message must be printed only once per LogRecord even if there are multiple dropped attributes

Checklist:

  • Followed the style guidelines of this project
  • Unit tests have been added
  • Documentation has been updated

@HyunnoH HyunnoH requested a review from a team April 8, 2024 13:42
@pichlermarc
Copy link
Member

@HyunnoH thanks for working on this. Please add a changelog so we can proceed 🙂

Copy link
Member

@hectorhdzg hectorhdzg left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link

codecov bot commented Apr 15, 2024

Codecov Report

Merging #4614 (f39a20f) into main (c046867) will increase coverage by 0.58%.
Report is 8 commits behind head on main.
The diff coverage is 100.00%.

❗ Current head f39a20f differs from pull request most recent head 6fc5d64. Consider uploading reports for the commit 6fc5d64 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4614      +/-   ##
==========================================
+ Coverage   92.83%   93.41%   +0.58%     
==========================================
  Files         329      198     -131     
  Lines        9528     6667    -2861     
  Branches     2053     1421     -632     
==========================================
- Hits         8845     6228    -2617     
+ Misses        683      439     -244     
Files Coverage Δ
experimental/packages/sdk-logs/src/LogRecord.ts 98.09% <100.00%> (+0.03%) ⬆️

... and 151 files with indirect coverage changes

@pichlermarc pichlermarc merged commit 3b5eb23 into open-telemetry:main Apr 15, 2024
17 checks passed
Zirak pushed a commit to Zirak/opentelemetry-js that referenced this pull request Sep 14, 2024
…#4614)

* feat(sdk-logs): make dropping attribute print message

* chore: update changelog

* chore(sdk-logs): add comment to explain message logic

* Update experimental/CHANGELOG.md

---------

Co-authored-by: Marc Pichler <[email protected]>
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.

Print message when LogRecord attribute is discarded
3 participants