Update logcollector telemetry with common properties #3242
Conversation
maddieford
requested review from
narrieta,
ZhidongPeng and
nagworld9
as code owners
| @@ -208,28 +209,30 @@ def collect_logs(self, is_full_mode): | |||
| else: | |||
| logger.info("Running log collector mode normal") | |||
|
|
|||
| LogCollector.initialize_telemetry() | |||
There was a problem hiding this comment.
Previously this was happening in LogCollector init.
We don't initialize the LogCollector until after cgroup checks, which resulted in all cgroup check events having initialized common properties (see PR description)
azurelinuxagent/agent.py
Outdated
| # Check the cgroups unit | ||
| log_collector_monitor = None | ||
| tracked_controllers = [] | ||
| if CollectLogsHandler.is_enabled_monitor_cgroups_check(): | ||
| try: | ||
| cgroup_api = get_cgroup_api() | ||
| except InvalidCgroupMountpointException as e: | ||
| log_cgroup_warning("The agent does not support cgroups if the default systemd mountpoint is not being used: {0}".format(ustr(e)), send_event=True) | ||
| log_cgroup_warning("The agent does not support cgroups if the default systemd mountpoint is not being used: {0}".format(ustr(e)), op=WALAEventOperation.LogCollection, send_event=True) |
There was a problem hiding this comment.
Updating these events to be marked as LogCollection operations
There was a problem hiding this comment.
I understand the motivation, but odd to send logcollection events with a function named as "cgroup". Plus the local log will still have the cgroups mark ("[CGW"]).
Maybe just create a similar log_log_collection_warn function? or if you don't need the special mark on the local log just use event.warn?
| @@ -76,7 +76,6 @@ def __init__(self, is_full_mode=False): | |||
| self._must_collect_files = self._expand_must_collect_files() | |||
| self._create_base_dirs() | |||
| self._set_logger() | |||
| self._initialize_telemetry() | |||
There was a problem hiding this comment.
This is now done in the azurelinuxagent.agent.Agent.collect_logs
| _LOGGER.info("Uncompressed archive size is %s b", total_uncompressed_size) | ||
| msg = "Uncompressed archive size is {0} b".format(total_uncompressed_size) | ||
| _LOGGER.info(msg) | ||
| add_event(op=WALAEventOperation.LogCollection, message=msg) |
There was a problem hiding this comment.
Adding this event before the process enters the main loop in an effort to collect this info for runs that fail due to mem limit exceeded
| @@ -211,9 +211,8 @@ def test_log_collector_parses_commands_in_manifest(self): | |||
| diskinfo,""".format(folder_to_list, file_to_collect) | |||
|
|
|||
| with patch("azurelinuxagent.ga.logcollector.MANIFEST_NORMAL", manifest): | |||
| with patch('azurelinuxagent.ga.logcollector.LogCollector._initialize_telemetry'): | |||
There was a problem hiding this comment.
These unit tests mocked _initialize_telemetry because it was in LogCollector init. Now that it has been removed from init, the mock is not necessary
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #3242 +/- ##
===========================================
+ Coverage 71.97% 72.54% +0.56%
===========================================
Files 103 114 +11
Lines 15692 16900 +1208
Branches 2486 2247 -239
===========================================
+ Hits 11295 12260 +965
- Misses 3881 4105 +224
- Partials 516 535 +19 ☔ View full report in Codecov by Sentry. |
Description
The log collector process first checks that cgroups are setup correctly, adding any telemetry events if there are unexpected results.
Previously, the common properties for the telemetry events were initialized in LogCollector init, but LogCollector was not initialized until after cgroup checks. As a result, if the process added any events during cgroup checks, the common properties were not initialized:
This PR updates the log collector process to initialize the common properties in telemetry events before any cgroup checks. It also adds a telemetry event for uncompressed file size before we expect the process to exit due to mem limit exceeded.
Issue #
PR information
Quality of Code and Contribution Guidelines