Fix package reporting implementation#1670
Conversation
✅MegaLinter analysis: Success
See detailed reports in MegaLinter artifacts
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1670 +/- ##
==========================================
- Coverage 82.00% 82.00% -0.01%
==========================================
Files 215 215
Lines 26309 26300 -9
Branches 4150 4151 +1
==========================================
- Hits 21575 21567 -8
+ Misses 3319 3317 -2
- Partials 1415 1416 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
lrafeei
changed the title
hmstepanek
left a comment
hmstepanek
left a comment
There was a problem hiding this comment.
A couple thoughts here-I'm not sure exactly why it would not be reporting the module(s) again but I had one thought. Seems unlikely to be the reason but I didn't see anything else that would cause that. You could add some log statements or a metric in or something to maybe help you debug what's going on.
| # harvest cycle before resuming. | ||
| if self._remaining_plugins and self.configuration and self.configuration.package_reporting.enabled: | ||
| start = stopwatch_start = time.time() | ||
| while ((time.time() - stopwatch_start) < 0.5) and ( |
There was a problem hiding this comment.
I wonder-can you just simplify this to be the following and leave MAX_PACKAGE_CAPTURE_TIME_PER_SLOW_HARVEST=2:
| while ((time.time() - stopwatch_start) < 0.5) and ( | |
| while (time.time() - start) < MAX_PACKAGE_CAPTURE_TIME_PER_SLOW_HARVEST: |
This way it will at least load 1 module before exiting. I realize this is slow harvest but I wonder if 5s is a bit long.
| # 0.5 seconds to upload. Then, wait for the next | ||
| # harvest cycle before resuming. | ||
| if self._remaining_plugins and self.configuration and self.configuration.package_reporting.enabled: | ||
| start = stopwatch_start = time.time() |
There was a problem hiding this comment.
This seems unlikely but I wonder if what's happening for the AI team who is testing this is they are never entering the while loop because of that less than .5 check? Are they seeing any modules reporting at all? Another way you could implement this that might be simpler and avoid this potential issue is:
start = time.time()
for module in self.plugins:
self._active_session.send_loaded_modules([module])
if time.time() - start > MAX_PACKAGE_CAPTURE_TIME_PER_SLOW_HARVEST:
break
else:
self._remaining_plugins = False
| if self.modules: | ||
| self._active_session.send_loaded_modules(self.modules) | ||
| self.modules = [] | ||
| if self._remaining_plugins and self.configuration and self.configuration.package_reporting.enabled: |
There was a problem hiding this comment.
I'm wondering if we really need the self._remaining_plugins or can we just set self.plugins = False?
There was a problem hiding this comment.
Oh, like override the generator object to be a boolean when it runs out and use that as the conditional?
4 participants
This PR attempts to fix an issue where some modules are not loaded in the environment section of the UI (the modules do not get passed into the
update_loaded_modulesendpoint). It also fixes a bug where it does not reload the modules upon agent restart.Previously the logic attempted to load as many modules as possible during a 2 second window by iterating through a plugins generator. However, it seems that some modules that have a longer loading time get missed.
The logic here has been changed to continue to load the next module if the previous module took less than 0.5 seconds and the amount of time uploading modules is less than 5.0 seconds. If the previous module takes longer than 0.5 seconds to load, the agent will wait until the next harvest cycle to resume.