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

Benchmark script improvements #186

Merged
merged 8 commits into from
May 29, 2019
Merged

Conversation

domanchi
Copy link
Contributor

@domanchi domanchi commented May 28, 2019

Summary

Building off #181, this adds the following features:

  • machine consumable output, by default. (--pretty preserves the human readable table)
  • you can select which plugins you want to test
  • timeout functionality, so scripts won't run forever
  • customizing number of iterations, so you can get more accurate results

Example Output

$ python scripts/benchmark.py --plugin PrivateKeyDetector --plugin Base64HighEntropyString -n 5 | jq
Running performance tests on: PrivateKeyDetector, Base64HighEntropyString
for: ['~/pg/detect-secrets']
{
    "Base64HighEntropyString": 0.8232977999992727,
    "PrivateKeyDetector": 0.6896803999989061
}
$ python scripts/benchmark.py --plugin Base64HighEntropyString --harakiri 0.5 --pretty
Running performance tests on: Base64HighEntropyString
for: ['~/pg/detect-secrets']
------------------------------------------
plugin                              time
------------------------------------------
Base64HighEntropyString   Timeout exceeded!
------------------------------------------

@domanchi domanchi force-pushed the benchmark-script-improvements branch from 75dc36e to 61b6267 Compare May 28, 2019 17:05
@domanchi domanchi force-pushed the benchmark-script-improvements branch from 61b6267 to 1219eea Compare May 28, 2019 17:07
@KevinHock KevinHock self-requested a review May 28, 2019 19:42
Copy link
Collaborator

@KevinHock KevinHock left a comment

Choose a reason for hiding this comment

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

lgtm 🚢 ⛵️
(when Travis passes)

),
)
parser.add_argument(
'--harakiri',
Copy link
Collaborator

Choose a reason for hiding this comment

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

lol

Copy link
Contributor Author

Choose a reason for hiding this comment

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

FWIW, I copied uwsgi's syntax for this: https://uwsgi-docs.readthedocs.io/en/latest/Glossary.html

'-n',
'--num-iterations',
default=1,
# TODO: assert non-negative
Copy link
Collaborator

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Whoops. Forgot to remove this lol.

Yeah, we're not breaking new ground here. We've done this before: https://github.com/Yelp/detect-secrets/blob/master/detect_secrets/core/usage.py#L396

if not args.filenames:
args.filenames = [
os.path.realpath(
os.path.join(
Copy link
Collaborator

Choose a reason for hiding this comment

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

Super nit: variable names could maybe help readability here

@domanchi domanchi merged commit 5820fa7 into master May 29, 2019
@domanchi domanchi deleted the benchmark-script-improvements branch May 29, 2019 01:40
killuazhu pushed a commit to IBM/detect-secrets that referenced this pull request May 28, 2020
* Delete GHDetector V1

Follow up of Yelp#184

* Move verify functionality to GHv2

* Addressed @xianjun comment

* Address @xianjun comments
killuazhu pushed a commit to IBM/detect-secrets that referenced this pull request Jul 9, 2020
* Delete GHDetector V1

Follow up of Yelp#184

* Move verify functionality to GHv2

* Addressed @xianjun comment

* Address @xianjun comments
killuazhu pushed a commit to IBM/detect-secrets that referenced this pull request Sep 17, 2020
* Use GHDetectorV2

Supports git-defenders/detect-secrets-discuss#166

Cover additional token contexts in GitHub V2 detector (Yelp#183)

Turn on GHDetectorV2 (Yelp#184)

Supports git-defenders/detect-secrets-discuss#124

Delete GHDetector V1 (Yelp#186)
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.

None yet

2 participants