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

Allow customization breadcrumb data of ActiveSupportLogger #2139

Merged
merged 5 commits into from
Oct 24, 2023

Conversation

igordepolli
Copy link
Contributor

@igordepolli igordepolli commented Oct 13, 2023

Ref: #2135

Description

Add new config.rails.active_support_logger_subscription_items to allow customization breadcrumb data of active support logger

config.rails.active_support_logger_subscription_items["sql.active_record"] << :type_casted_binds
config.rails.active_support_logger_subscription_items.delete("sql.active_record")
config.rails.active_support_logger_subscription_items["foo"] = :bar

Copy link
Member

@sl0thentr0py sl0thentr0py left a comment

Choose a reason for hiding this comment

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

thx @igordepolli!
one small change then good to go

@codecov
Copy link

codecov bot commented Oct 23, 2023

Codecov Report

Merging #2139 (c51f9e0) into master (1c0550f) will decrease coverage by 37.58%.
Report is 3 commits behind head on master.
The diff coverage is 75.00%.

@@             Coverage Diff             @@
##           master    #2139       +/-   ##
===========================================
- Coverage   97.27%   59.69%   -37.58%     
===========================================
  Files          97       86       -11     
  Lines        3630     3389      -241     
===========================================
- Hits         3531     2023     -1508     
- Misses         99     1366     +1267     
Components Coverage Δ
sentry-ruby 55.88% <ø> (-42.10%) ⬇️
sentry-rails 45.75% <75.00%> (-49.21%) ⬇️
sentry-sidekiq 93.70% <ø> (ø)
sentry-resque 92.06% <ø> (ø)
sentry-delayed_job 94.36% <ø> (ø)
sentry-opentelemetry 100.00% <ø> (ø)
Files Coverage Δ
sentry-rails/lib/sentry/rails/configuration.rb 85.71% <100.00%> (-11.17%) ⬇️
sentry-rails/lib/sentry/rails/railtie.rb 25.00% <0.00%> (-73.69%) ⬇️

... and 73 files with indirect coverage changes

CHANGELOG.md Outdated Show resolved Hide resolved
@igordepolli
Copy link
Contributor Author

thx @igordepolli! one small change then good to go

@sl0thentr0py Done. Ty for review :)

@igordepolli
Copy link
Contributor Author

@st0012 Done. Good catch! Ty! :)

@igordepolli igordepolli requested a review from st0012 October 23, 2023 23:31
@sl0thentr0py
Copy link
Member

@igordepolli the ensure fails on ruby 2.4 because this feature was added in 2.5, you can either add a begin/end or just wrap the spec in a context with a before/after which is probably cleaner.

https://github.com/getsentry/sentry-ruby/actions/runs/6620176438/job/17999733560?pr=2139#step:5:156

You can also just give me push rights to the PR and I'll take care of the rest and merge this in.

@igordepolli
Copy link
Contributor Author

@igordepolli the ensure fails on ruby 2.4 because this feature was added in 2.5, you can either add a begin/end or just wrap the spec in a context with a before/after which is probably cleaner.

https://github.com/getsentry/sentry-ruby/actions/runs/6620176438/job/17999733560?pr=2139#step:5:156

You can also just give me push rights to the PR and I'll take care of the rest and merge this in.

@sl0thentr0py Oh, sorry for the inconvenience. You already have the right to manipulate PR however you want, feel free.

@sl0thentr0py sl0thentr0py merged commit d315a15 into getsentry:master Oct 24, 2023
95 of 97 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants