ZeroTrust performance fixes #645
Conversation
…s rather than real exploitation
… performance info
# Conflicts: # envs/monkey_zoo/blackbox/test_blackbox.py # monkey/monkey_island/cc/ui/src/components/report-components/zerotrust/EventsModal.js
| assert (len(existing_findings) < 2), "More than one finding exists for {}:{}".format(test, status) | ||
|
|
||
| if len(existing_findings) == 0: | ||
| Finding.save_finding(test, status, events) | ||
| else: | ||
| # Now we know for sure this is the only one | ||
| orig_finding = existing_findings[0] | ||
| orig_finding.add_events(events) | ||
| orig_finding.update(push_all__events=events) |
There was a problem hiding this comment.
Please add a comment explaining what this is doing, why this is better, and link to the documentation
There was a problem hiding this comment.
Why do we need to reimplement mongoengine methods? It's error prone and adds lines of code that are not necessary. https://github.com/guardicore/pyinstaller/blob/develop/bootloader/src/old_machine_common_functions.c
There was a problem hiding this comment.
Not sure what the link should be telling me, but I agree - just adding a link to the method's documentation will help
There was a problem hiding this comment.
Ohh, you were talking about commenting the method in the code? Ok, I'll improve
| self.assertListEqual([], ZeroTrustService._ZeroTrustService__get_events_without_overlap(5, [1, 2, 3])) | ||
| self.assertListEqual([3], ZeroTrustService._ZeroTrustService__get_events_without_overlap(6, [1, 2, 3])) | ||
| self.assertListEqual([1, 2, 3, 4, 5], ZeroTrustService._ZeroTrustService__get_events_without_overlap(10, [1, 2, 3, 4, 5])) |
There was a problem hiding this comment.
If we're using something it's better to not name-mangle it... this test will break when we rename the class
There was a problem hiding this comment.
So you suggest to rename __get_events_without_overlap?
There was a problem hiding this comment.
I suggest making it not protected with __ but rather just mark it as "private" with a single _.
There was a problem hiding this comment.
So we are talking about the same thing :) I wanted to be consistent at least on the module scope. Why are methods mangled there anyway?
There was a problem hiding this comment.
I agree we should be consistent - you can rename all of them to not be mangled, I think that would be better
| PROFILER_LOG_DIR = "./profiler_logs/" | ||
|
|
||
|
|
||
| def profile(sort_args=['cumulative'], print_args=[100]): |
There was a problem hiding this comment.
Some usage documentation and examples would be useful
|
Also, not all unit tests are passing - please fix them :) |
ShayNehmad
added
the
Enhancement
monkey/monkey_island/cc/ui/src/components/report-components/zerotrust/SkippedEventsTimeline.js
Show resolved
Hide resolved
.gitignore
Outdated
| # Profiling logs | ||
| /monkey/profiler_logs/ | ||
|
|
There was a problem hiding this comment.
Won't that depend on the working directory from which you execute stuff which might change, let's say when you're running UTs?
Probs safer to ignore all directories called profiler_logs
| # Profiling logs | |
| /monkey/profiler_logs/ | |
| # Profiling logs | |
| profiler_logs/ |
|
UT's fail because of similar issue mongomock/mongomock#625 |
VakarisZ
force-pushed
the
feature/zt_performance_fixes
branch
from
5301402
to
44cb87a
Compare
What is this?
@profile()decorator that outputs profiling info of method decorated.Checklist
Proof that it works
If applicable, add screenshots or log transcripts of the feature working