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

Release 6.14.0 #607

Merged
merged 54 commits into from
Jul 20, 2020
Merged

Release 6.14.0 #607

merged 54 commits into from
Jul 20, 2020

Conversation

imjoehaines
Copy link
Contributor

@imjoehaines imjoehaines commented Jul 20, 2020

This release looks huge, but most of changes are the new Rails 6 example

6.14.0

Enhancements

  • Add configurable discard_classes option to allow filtering errors using either a String or Regexp matched against the error's class name
    | #597

  • The Breadcrumb name limit of 30 characters has been removed
    | #600

  • Improve performance of payload cleaning
    | #601

  • Improve performance when processing stacktraces
    | #602
    | #603

  • If a custom object responds to id method, show the id and class in error reports
    | #531
    | manojmj92

We only need this for development and run it on the latest version
of Ruby on CI, so there's no reason not to use the latest Rubocop
version

The TODO file has grown, we should look at fixing these (a lot of
them can be autofixed)
Update Rubocop version and todo file
This accepts Strings and Regexps rather than Classes and Procs,
which is more inline with other Bugsnag notifiers

It also doesn't walk the ancestor chain, which should lead to less
false positives — with regexps especailly, this could be very
dangerous!
Having defaults here is problematic for a few reasons, mainly that
if someone is emptying `ignore_classes` then we'll break their app
by re-adding those defaults to `discard_classes`

We can move the values over when `ignore_classes` is removed
…ore-classes

Add discard_classes option to support strings & regular expressions and deprecate ignore_classes
TODO:
    1. Sidekiq
    2. Que
    3. Resque
    4. README
Remove the length limit on breadcrumb names
This is a first iteration of improving how we clean objects when
preparing to JSON encode them

Currently we iterate over the payload multiple times; sometimes to clean
up encoding errors/recursion and other times to filter sensitive data

Ideally we should be iterating over the payload once, which is slightly
complicated because we should only be filtering parts of the payload
(the metadata and breadcrumb metadata)
Helper no longer breaks recursion in 'trim_if_needed', so these tests
no longer apply there. However we are still breaking recursion and so
can test the same thing elsewhere
For example, in this hash:

{ a: { b: 'c' } }

'c' lives in scope 'a.b' and so should only be filtered if Cleaner
is given 'a.b' in its 'scopes_to_filter'
imjoehaines and others added 24 commits June 24, 2020 14:48
This exposed a pretty big regression where filters wouldn't match when
they should have. This was caused by us filtering the entire report
object in one go, which means the scopes were nested deeper than they
were before

Previously we filtered the events.metaData directly, so scopes would not
include 'events.metaData' and therefore a filter of 'foo' would match
'events.metaData.foo'. Now that we filter the entire report, if a filter
relied on 'deep_filters', it would apply and so things that should be
redacted wouldn't have been

To solve this, we strip each 'scope_to_filter' from the scope before
matching it, if deep_filters are enabled

The tests passed before this change because we set 'scopes_to_filter'
in each test. Now that the instance is shared, the scopes are fetched
from the Configuration so this isn't possible and it exposed the bug

The tests now cover this case, because they can't set 'scopes_to_filter'
directly anymore, so they are testing that the filters they're using
match with the Configuration's 'scopes_to_filter'
…ance

Improve payload cleaning performance
This is a significant performance boost when sending a large number of
errors as it avoids the overhead of creating another Pathname object
Improve stacktrace file path resolving performance by avoiding Pathname
This isn't a _huge_ perf boost, but every little helps :^)

In the old Pathname method, this if was useful because it was quicker
to skip files that were already resolved, assuming there was a mix of
relative and absolute paths

With File.realpath, however, it's quicker to just run the function as
it does nothing if the path is already resolved. This is roughtly
equal in terms of performance to using the new File.absolute_path?
method (they are so close in benchmarks that it's essentially noise)
Speed up file path resolution when processing stacktraces
If a custom object responds to `id` method, show the id and class value, instead of showing "[OBJECT]" in error reports
@imjoehaines imjoehaines merged commit 1d43234 into master Jul 20, 2020
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.

4 participants