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

Add --kind and --level arguments for granular control of results reporting #2228

Closed
michaelcfanning opened this issue Jan 9, 2021 · 1 comment · Fixed by #2241
Closed

Comments

@michaelcfanning
Copy link
Member

michaelcfanning commented Jan 9, 2021

Implementation notes:

  • Follow the same pattern for enum arguments set by OptionallyEmittedData
  • Defaults follow current reporting behavior --kind Fail and --level Error;Warning
  • Currently, the SDK provides all reporting not matching the criteria in point 2 in the case when the --verbose flag is passed. We should consider deprecating --verbose entirely, as we also have the new --traces mechanism to control arbitrary verbose reporting. Something to keep in mind as we review this functionality globally.
  • We need to locate the best place to insert the filtering. The open question is whether we should disable skimmers based on these settings (because we determine they can never fire) or if we allow them to function then filter immediately before logging. Let's start with the latter approach as a default (I'm open to a contrary view though if you build one)
  • Let's start a new 1.8 versioning line for this significant change in tool behavior.

@eddynaka
@jameswinkler, FYI

@michaelcfanning
Copy link
Member Author

For completeness, I will mention that when building this, we should at least consider the possibility we want an even more granular, per-rule mechanism to control reporting behavior.

For example:

-level Error;BadRuleId.Note

might do something like remap BadRuleId's output into a Note (which could be disabled). we should also consider whether this argument could contain a value like 'Disabled' which doesn't literally existing on the SARIF Level type. we could do this because we'll have a string [] representation we can process because constructing the SARIF types. On encountering a Disabled value, we would proactively add this id to the disabled skimmers set.

This would provide a super easy way for people to opt out of a poorly behaving check (one that's crashing in general code paths for example).

jameswinkler added a commit that referenced this issue Jan 18, 2021
…ogger

#2228 Remove verbose and largely replace it with "quiet"
jameswinkler added a commit that referenced this issue Jan 18, 2021
…ogger

#2228 Renaming, remove quiet from Logging Options
jameswinkler added a commit that referenced this issue Jan 20, 2021
eddynaka added a commit that referenced this issue Jan 25, 2021
…2241)

* Creating BaseLogger to share methods

* removing logmessage from interface

* updating more loggers

* Fix parameters

* Replace verbose in ConsoleLogger.  It should know about quiet instead

* Remove verbose

* Remove unused method

* Remove unnecessary nullcheck

* fixing functional tests

* back to quiet

* fixing tests

* Use ShouldLog in more locations

* Add to release history for breaking change

* Fix formatting

* Make more loggers children of BaseLogger.  Cleanup some functionality with --quiet.  Correct some tests

* fixing spacing

* Rename some parameters, classes, and files.  Remove "quiet" from LoggingOptions, and correct unit tests affected

* More complete renaming

* Rewording, minor cleanup

* fixing sample build

* Correct validate logic.  Remove redundant "none"s

* Rename parameters, reword helper text

* Add unit tests for DriverExtensionMethod and BaseLogger

* Fix casting error

* fixing samples compilation issue

* Small changes to address comments

* Formatting fixes

* Address renaming and refactoring comments

* More renaming

* Change validation in BaseLogger and correct unit tests and code  (#2262)

* Change validation in BaseLogger and correct unit tests and code broken as a result

* Correct formatting

* Test SarifLogger honors Kind/Level

* Refactor, remove redundant code

Co-authored-by: Eddy Nakamura <ednakamu@microsoft.com>
Co-authored-by: Eddy Nakamura <eddynaka@gmail.com>
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 a pull request may close this issue.

1 participant