modifying the filtering tranformation to filter out unwanted info #175

weiweishi · 2018-11-29T18:04:53Z

Made changes to Matt's original scrubbing method, based on the json structure of the rollbar payload:
"exception": {
"message": "http://****:******@mycombe.library.ualberta.ca:8080/fedora/rest/prod/8c/ff/bb/0f/8cffbb0f-8cb8-4061-b6fe-3122a595d181/list_source: 404",
"class": "IOError"
}
Added level: ignore for IOError messages.

This should be considered as a WIP PR, as I don't have a good way to test the code changes, and I'd like to hear from others, which seems to be a better/more reliable solution, to avoid having our fedora password get into Rollbar again.

weiweishi · 2018-11-29T18:06:12Z

lib/pushmi_pullyu/cli.rb

      # add a filter after Rollbar has built the error payload but before it is delivered to the API,
      # in order to strip sensitive information out of certain error messages
      exception_message_transformer = proc do |payload|
-        clean_message = payload[:exception].message.sub(/http:\/\/.+:.+@(.+)\/fedora\/rest\/prod\/(.*)/,
+        clean_message = payload[:exception][:message].sub(/http:\/\/.+:.+@(.+)\/fedora\/rest\/prod\/(.*)/,


According to Rollbar, the payload json structured like this:
"exception": {
"message": "http://****:*****r@mycombe.library.ualberta.ca:8080/fedora/rest/prod/8c/ff/bb/0f/8cffbb0f-8cb8-4061-b6fe-3122a595d181/list_source: 404",
"class": "IOError"
}
So we will need to replace the value in payload[:exception][:message] with the scrubbed version.

houndci-bot · 2018-11-29T18:06:28Z

lib/pushmi_pullyu/cli.rb

-
+      config.exception_level_filters.merge!({
+       'IOError' => 'ignore'
+      })


Layout/IndentHash: Indent the right brace the same as the first position after the preceding left parenthesis.

houndci-bot · 2018-11-29T18:06:28Z

lib/pushmi_pullyu/cli.rb

@@ -59,13 +59,15 @@ def configure_rollbar
    Rollbar.configure do |config|
      config.enabled = false unless options[:rollbar][:token].present?
      config.access_token = options[:rollbar][:token]
-
+      config.exception_level_filters.merge!({
+       'IOError' => 'ignore'


Layout/IndentHash: Use 2 spaces for indentation in a hash, relative to the first position after the preceding left parenthesis.

houndci-bot · 2018-11-29T18:06:28Z

lib/pushmi_pullyu/cli.rb

@@ -59,13 +59,15 @@ def configure_rollbar
    Rollbar.configure do |config|
      config.enabled = false unless options[:rollbar][:token].present?
      config.access_token = options[:rollbar][:token]
-
+      config.exception_level_filters.merge!({


Performance/RedundantMerge: Use config.exception_level_filters['IOError'] = 'ignore' instead of config.exception_level_filters.merge!({
Style/BracesAroundHashParameters: Redundant curly braces around a hash parameter.

houndci-bot · 2018-11-29T18:06:29Z

lib/pushmi_pullyu/cli.rb

@@ -38,7 +38,7 @@ def run
      end
    # rubocop:disable Lint/RescueException
    rescue Exception => e
-      Rollbar.error(e)
+      Rollbar.error(e, :use_exception_level_filters => true)


Style/HashSyntax: Use the new Ruby 1.9 hash syntax. (https://github.com/bbatsov/ruby-style-guide#hash-literals)

weiweishi · 2018-11-29T18:07:12Z

lib/pushmi_pullyu/cli.rb

@@ -59,13 +59,15 @@ def configure_rollbar
    Rollbar.configure do |config|
      config.enabled = false unless options[:rollbar][:token].present?
      config.access_token = options[:rollbar][:token]
-
+      config.exception_level_filters.merge!({


Add IOError as to be ignored so this particular error message will not be sent to Rollbar.

murny · 2018-11-29T21:19:12Z

lib/pushmi_pullyu/cli.rb

@@ -38,7 +38,7 @@ def run
      end
    # rubocop:disable Lint/RescueException
    rescue Exception => e
-      Rollbar.error(e)
+      Rollbar.error(e, use_exception_level_filters: true)


We can probably make this global? Not sure how many Rollbar.error calls we doing in pushmi_pullyu (might just be this one, but if someone ever decides to come in and add another one, they don't have to forgot adding this use_exception_level_filters: true call).

Can add it right to the config:

Rollbar.configure do |config| config.use_exception_level_filters = true end

So that all Rollbar.error or Rollbar.warning calls receive this

(oddly this is not in the documentation, but heres a PR for the commit: rollbar/rollbar-gem#588)

Ah the PR example is wrong?

Correct usage is use_exception_level_filters_default = true

https://github.com/rollbar/rollbar-gem/blob/master/docs/configuration.md#use_exception_level_filters_default

murny · 2018-11-29T21:21:14Z

lib/pushmi_pullyu/cli.rb

@@ -59,13 +59,13 @@ def configure_rollbar
    Rollbar.configure do |config|
      config.enabled = false unless options[:rollbar][:token].present?
      config.access_token = options[:rollbar][:token]
-
+      config.exception_level_filters['IOError'] = 'ignore'


Maybe use their provided syntax? If we decide to filter another exception in the future, someone doesn't just come in and copy and paste this line, they can add it to the merge call:

config.exception_level_filters.merge!({ 'IOError' => 'ignore' })

Arrgh. I think Rubocop changed it to its current syntax with rubocop -a. I will revert it back.

Did we also want to ignore all IOErrors and not report them to rollbar? (as we throw IOErrors if things can't be found, for example if no file list exists:

pushmi_pullyu/lib/pushmi_pullyu/aip/file_list_creator.rb

Line 41 in 8d71774

# Note: raises IOError if can't find

)

murny

LGTM 👍

Left some minor comments to future proof the code a bit more.

…i_pullyu into rollbar_filter

lib/pushmi_pullyu/cli.rb

Co-Authored-By: weiweishi <weiwei.shi@ualberta.ca>

ualbertalib-bot · 2018-11-29T22:53:41Z

	1 Warning
⚠️	Please rebase to get rid of the merge commits in this PR

Generated by 🚫 Danger

…into rollbar_filter

…i_pullyu into rollbar_filter

modifying the filtering tranformation to filter out unwanted info

f7a70ca

weiweishi commented Nov 29, 2018

View reviewed changes

houndci-bot reviewed Nov 29, 2018

View reviewed changes

weiweishi commented Nov 29, 2018

View reviewed changes

fixed rubocop errors

54880aa

murny reviewed Nov 29, 2018

View reviewed changes

murny previously approved these changes Nov 29, 2018

View reviewed changes

make use_exception_level_filter a global setting

e00e163

weiweishi dismissed murny’s stale review via e00e163 November 29, 2018 21:48

murny previously approved these changes Nov 29, 2018

View reviewed changes

Fix rubocop hash alignment

58ebfb9

murny dismissed their stale review via 58ebfb9 November 29, 2018 21:55

murny previously approved these changes Nov 29, 2018

View reviewed changes

weiweishi added 2 commits November 29, 2018 15:14

Updated gemspec file to bump up rollbar gem version

309ba04

Merge branch 'rollbar_filter' of https://github.com/ualbertalib/pushm…

db1ab24

…i_pullyu into rollbar_filter

weiweishi dismissed murny’s stale review via db1ab24 November 29, 2018 22:14

murny reviewed Nov 29, 2018

View reviewed changes

lib/pushmi_pullyu/cli.rb Outdated Show resolved Hide resolved

add use_exception_level_filter_default to all rollbar payloads

00b30e2

Co-Authored-By: weiweishi <weiwei.shi@ualberta.ca>

murny approved these changes Nov 29, 2018

View reviewed changes

Merge branch 'master' into rollbar_filter

5bd8c14

weiweishi added 2 commits November 29, 2018 15:56

Merge branch 'master' of https://github.com/ualbertalib/pushmi_pullyu …

fcc0088

…into rollbar_filter

Merge branch 'rollbar_filter' of https://github.com/ualbertalib/pushm…

703c54d

…i_pullyu into rollbar_filter

weiweishi merged commit 3d15b35 into master Nov 29, 2018

murny deleted the rollbar_filter branch November 29, 2018 23:19

murny mentioned this pull request Dec 7, 2018

[PMPY] PushmiPullyu::AIP::Downloader::NoFileSets issue #178

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

modifying the filtering tranformation to filter out unwanted info #175

modifying the filtering tranformation to filter out unwanted info #175

weiweishi commented Nov 29, 2018 •

edited

Loading

weiweishi Nov 29, 2018

houndci-bot Nov 29, 2018

houndci-bot Nov 29, 2018

houndci-bot Nov 29, 2018

houndci-bot Nov 29, 2018

weiweishi Nov 29, 2018

murny Nov 29, 2018 •

edited

Loading

murny Nov 29, 2018

murny Nov 29, 2018

weiweishi Nov 29, 2018

murny Nov 29, 2018

murny left a comment

ualbertalib-bot commented Nov 29, 2018

modifying the filtering tranformation to filter out unwanted info #175

modifying the filtering tranformation to filter out unwanted info #175

Conversation

weiweishi commented Nov 29, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

murny Nov 29, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

murny left a comment

Choose a reason for hiding this comment

ualbertalib-bot commented Nov 29, 2018

weiweishi commented Nov 29, 2018 •

edited

Loading

murny Nov 29, 2018 •

edited

Loading