Skip to content

Conversation

lcian
Copy link
Member

@lcian lcian commented Sep 30, 2025

⚠️ Stacked on top of #914 ⚠️

Description

Removes the double opt-in for the log crate integration, as we'll now map to both an exception/breadcrumb and a log when the logs feature flag is active and the level is at or above INFO.

If the user has set enable_logs: false in their client options, the log will be created but dropped when it reaches the client.
This creates the log uselessly to later discard it. Still, in 99% of cases, if I use the logs feature flag, it means I want to get logs, so most probably enable_logs == true and the log is actually used.

This change is analogous to the one we already did for the tracing integration.

Issues

Closes #909
Closes RUST-112

Copy link

linear bot commented Sep 30, 2025

Copy link

codecov bot commented Sep 30, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 73.52%. Comparing base (0da7c5e) to head (71749b9).

Additional details and impacted files
@@                         Coverage Diff                          @@
##           lcian/feat/log-multiple-mappings     #915      +/-   ##
====================================================================
+ Coverage                             73.51%   73.52%   +0.01%     
====================================================================
  Files                                    64       64              
  Lines                                  7539     7539              
====================================================================
+ Hits                                   5542     5543       +1     
+ Misses                                 1997     1996       -1     

@lcian lcian marked this pull request as ready for review October 1, 2025 12:35
@lcian lcian requested a review from szokeasaurusrex October 1, 2025 12:35
Copy link
Member

@szokeasaurusrex szokeasaurusrex left a comment

Choose a reason for hiding this comment

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

Looks good! Long term, I would consider completely removing the enable_logs flag, and have setting/unsetting the feature flag be the only way to enable/disable logs

@lcian
Copy link
Member Author

lcian commented Oct 2, 2025

@szokeasaurusrex Thanks. I also have this #910 which sets enable_logs to true if the feature flag is active, so practically you will only need the feature flag to get started.

@szokeasaurusrex
Copy link
Member

@lcian Ah I was more referring to that we maybe should consider completely removing the flag, so that you cannot disable logs if the logs feature flag is turned on

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.

2 participants