-
Notifications
You must be signed in to change notification settings - Fork 136
Fix package reporting implementation #1670
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
base: main
Are you sure you want to change the base?
Changes from 13 commits
89203b0
11aaece
2454063
b93898c
c99e785
dd0cca6
bcbfad4
2a32b43
8cfbb0d
28993c2
c2031f1
515b5b7
c6be361
8c476a0
8d38018
d4d0262
ed8ad69
53af784
89594b4
7265f2f
4968e64
3808b05
8cc9f6c
d06ae42
c4f906c
133c954
7628eb2
ad8bccc
78f0512
63f579c
2c2a62e
8961d8c
467cc52
e6f1559
d6aecbe
7b7cf99
935f0c5
1b90863
8f96bdd
86dca9b
d70fbcd
8e73e0b
8e6668b
d83c9f3
786ba5f
6edebee
64273a1
406ba4e
66f5ccc
8fdd467
2e250e5
81de885
aa8ee8a
23415ab
bf214f6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -49,7 +49,7 @@ | |||||
|
|
||||||
| _logger = logging.getLogger(__name__) | ||||||
|
|
||||||
| MAX_PACKAGE_CAPTURE_TIME_PER_SLOW_HARVEST = 2.0 | ||||||
| MAX_PACKAGE_CAPTURE_TIME_PER_SLOW_HARVEST = 5.0 | ||||||
|
|
||||||
|
|
||||||
| class Application: | ||||||
|
|
@@ -1284,27 +1284,23 @@ def harvest(self, shutdown=False, flexible=False): | |||||
| data_sampler.name, | ||||||
| ) | ||||||
|
|
||||||
| # Send environment plugin list | ||||||
|
|
||||||
| stopwatch_start = time.time() | ||||||
| while ( | ||||||
| configuration | ||||||
| and configuration.package_reporting.enabled | ||||||
| and self._remaining_plugins | ||||||
| and ((time.time() - stopwatch_start) < MAX_PACKAGE_CAPTURE_TIME_PER_SLOW_HARVEST) | ||||||
| ): | ||||||
| try: | ||||||
| module_info = next(self.plugins) | ||||||
| self.modules.append(module_info) | ||||||
| except StopIteration: | ||||||
| self._remaining_plugins = False | ||||||
|
|
||||||
| # Send the accumulated environment plugin list if not empty | ||||||
| if self.modules: | ||||||
| self._active_session.send_loaded_modules(self.modules) | ||||||
|
|
||||||
| # Reset the modules list every harvest cycle | ||||||
| self.modules = [] | ||||||
| # Send environment plugin. | ||||||
| # If a module takes less than 0.5 seconds to load, | ||||||
| # continue to upload. Do this until we reach 5.0 | ||||||
| # seconds or until a module has taken more than | ||||||
| # 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() | ||||||
| while ((time.time() - stopwatch_start) < 0.5) and ( | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wonder-can you just simplify this to be the following and leave MAX_PACKAGE_CAPTURE_TIME_PER_SLOW_HARVEST=2:
Suggested change
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. |
||||||
| (time.time() - start) < MAX_PACKAGE_CAPTURE_TIME_PER_SLOW_HARVEST | ||||||
| ): | ||||||
| try: | ||||||
| self._active_session.send_loaded_modules([next(self.plugins)]) | ||||||
| stopwatch_start = time.time() | ||||||
| except StopIteration: | ||||||
| self._remaining_plugins = False | ||||||
| break | ||||||
|
|
||||||
| # Add a metric we can use to track how many harvest | ||||||
| # periods have occurred. | ||||||
|
|
@@ -1745,15 +1741,12 @@ def internal_agent_shutdown(self, restart=False): | |||||
| # if this has not been completed during harvest | ||||||
| # lifetime of the application | ||||||
|
|
||||||
| while self.configuration and self.configuration.package_reporting.enabled and self._remaining_plugins: | ||||||
| try: | ||||||
| module_info = next(self.plugins) | ||||||
| self.modules.append(module_info) | ||||||
| except StopIteration: | ||||||
| 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: | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm wondering if we really need the self._remaining_plugins or can we just set self.plugins = False?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh, like override the generator object to be a boolean when it runs out and use that as the conditional? |
||||||
| # Anything that was left in the plugins generator | ||||||
| # will be resolved here. | ||||||
| plugins_list = list(self.plugins) | ||||||
| if plugins_list: | ||||||
| self._active_session.send_loaded_modules(plugins_list) | ||||||
|
|
||||||
| # Now shutdown the actual agent session. | ||||||
|
|
||||||
|
|
@@ -1776,6 +1769,10 @@ def internal_agent_shutdown(self, restart=False): | |||||
| # as shutdown. | ||||||
|
|
||||||
| if restart: | ||||||
| # Reset package/module generator | ||||||
| self.plugins = plugins() | ||||||
| self._remaining_plugins = True | ||||||
|
|
||||||
| self._agent_restart += 1 | ||||||
| self.activate_session() | ||||||
|
|
||||||
|
|
||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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: