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

Suppress the unnecessary “unsupported options notice” #2349

Conversation

moznion
Copy link
Contributor

@moznion moznion commented Jul 22, 2024

A related change was introduced in #2303.

Description

In Scope#update_from_options(), the method strips key-value pairs from the options according to its parameters. When all keys in options are supported, it still logs an “unsupported options notice” for an empty unsupported_option_keys value with empty array literal in Sentry::Hub#capture_event, like: "Options [] are not supported and will not be applied to the event."

Example:

When calling subject.capture_event(event, level: 'DEBUG'), the capture_event method should handle the options like this:

# In this case, options == {:level=>'DEBUG'}
unsupported_option_keys = scope.update_from_options(**options)
# unsupported_option_keys should be [], but the following debug log will be shown
# like "Options [] are not supported and will not be applied to the event."
configuration.log_debug <<~MSG
  Options #{unsupported_option_keys} are not supported and will not be applied to the event.
  You may want to set them under the `extra` option.
MSG

This patch changes the logic to check whether unsupported_option_keys is empty before logging the notice, thus suppressing unnecessary logs.

@moznion moznion force-pushed the suppress_unnecessary_unsupported_options_notices branch 2 times, most recently from 4e60762 to f71019f Compare July 22, 2024 07:14
In `Scope#update_from_options()`, the method strips key-value pairs from
the `options` according to its parameters. When all keys in `options` are
supported, it still logs an “unsupported options notice” for an empty
`unsupported_option_keys` value with empty array literal in
`Sentry::Hub#capture_event`, like:
"Options [] are not supported and will not be applied to the event."

Example:

When calling `subject.capture_event(event, level: 'DEBUG')`, the
`capture_event` method should handle the options like this:

  # In this case, options == {:level=>'DEBUG'}
  unsupported_option_keys = scope.update_from_options(**options)
  # unsupported_option_keys should be [], but the following debug log will be shown
  # like "Options [] are not supported and will not be applied to the event."
  configuration.log_debug <<~MSG
    Options #{unsupported_option_keys} are not supported and will not be applied to the event.
    You may want to set them under the `extra` option.
  MSG

This patch changes the logic to check whether `unsupported_option_keys` is
empty before logging the notice, thus suppressing unnecessary logs.

Signed-off-by: moznion <moznion@mail.moznion.net>
@moznion moznion force-pushed the suppress_unnecessary_unsupported_options_notices branch from f71019f to e549293 Compare July 22, 2024 07:54
@st0012 st0012 self-requested a review July 23, 2024 09:19
Copy link

codecov bot commented Jul 23, 2024

Codecov Report

Attention: Patch coverage is 0% with 2 lines in your changes missing coverage. Please review.

Project coverage is 78.25%. Comparing base (36866c5) to head (5d55576).

❗ There is a different number of reports uploaded between BASE (36866c5) and HEAD (5d55576). Click for more details.

HEAD has 1 upload less than BASE
Flag BASE (36866c5) HEAD (5d55576)
6 5
Additional details and impacted files
@@             Coverage Diff             @@
##           master    #2349       +/-   ##
===========================================
- Coverage   98.66%   78.25%   -20.41%     
===========================================
  Files         205      146       -59     
  Lines       13483     6889     -6594     
===========================================
- Hits        13303     5391     -7912     
- Misses        180     1498     +1318     
Components Coverage Δ
sentry-ruby 54.91% <0.00%> (-44.12%) ⬇️
sentry-rails 97.41% <ø> (ø)
sentry-sidekiq 97.01% <ø> (ø)
sentry-resque 96.79% <ø> (ø)
sentry-delayed_job 98.92% <ø> (ø)
sentry-opentelemetry 100.00% <ø> (ø)
Files Coverage Δ
sentry-ruby/lib/sentry/hub.rb 40.71% <0.00%> (-58.68%) ⬇️

... and 124 files with indirect coverage changes

Copy link
Collaborator

@st0012 st0012 left a comment

Choose a reason for hiding this comment

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

Thanks for the fix 👍

@st0012 st0012 merged commit 89f371f into getsentry:master Jul 23, 2024
122 of 124 checks passed
@moznion moznion deleted the suppress_unnecessary_unsupported_options_notices branch July 23, 2024 16:14
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