Skip to content

Commit dc8547e

Browse files
authored
Fix excessive disk logging (#287)
* add truncation logic to limit message in file logger * add unit test for filelogger * add unit test for flush() * add unit test for filelogger * revert comment * modify the write truncation logic and ut * remove truncation method in filelogger
1 parent e989b35 commit dc8547e

File tree

2 files changed

+156
-0
lines changed

2 files changed

+156
-0
lines changed

src/core/src/local_loggers/FileLogger.py

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ def __init__(self, env_layer, log_file):
2525
self.log_file = log_file
2626
self.log_failure_log_file = log_file + ".failure"
2727
self.log_file_handle = None
28+
self.max_msg_size = 32 * 1024 * 1024
2829
try:
2930
self.log_file_handle = self.env_layer.file_system.open(self.log_file, "a+")
3031
except Exception as error:
@@ -38,6 +39,8 @@ def __del__(self):
3839

3940
def write(self, message, fail_silently=True):
4041
try:
42+
if len(message) > self.max_msg_size:
43+
message = message[:self.max_msg_size]
4144
if self.log_file_handle is not None:
4245
self.log_file_handle.write(message)
4346
except Exception as error:
@@ -50,6 +53,8 @@ def write(self, message, fail_silently=True):
5053
def write_irrecoverable_exception(self, message):
5154
""" A best-effort attempt to write out errors where writing to the primary log file was interrupted"""
5255
try:
56+
if len(message) > self.max_msg_size:
57+
message = message[:self.max_msg_size]
5358
with self.env_layer.file_system.open(self.log_failure_log_file, 'a+') as fail_log:
5459
timestamp = self.env_layer.datetime.timestamp()
5560
fail_log.write("\n" + timestamp + "> " + message)
@@ -67,3 +72,4 @@ def close(self, message_at_close='<Log file was closed.>'):
6772
self.write(str(message_at_close))
6873
self.log_file_handle.close()
6974
self.log_file_handle = None # Not having this can cause 'I/O exception on closed file' exceptions
75+

src/core/tests/Test_FileLogger.py

Lines changed: 150 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,150 @@
1+
# Copyright 2025 Microsoft Corporation
2+
#
3+
# Licensed under the Apache License, Version 2.0 (the "License");
4+
# you may not use this file except in compliance with the License.
5+
# You may obtain a copy of the License at
6+
#
7+
# http://www.apache.org/licenses/LICENSE-2.0
8+
#
9+
# Unless required by applicable law or agreed to in writing, software
10+
# distributed under the License is distributed on an "AS IS" BASIS,
11+
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
# See the License for the specific language governing permissions and
13+
# limitations under the License.
14+
#
15+
# Requires Python 2.7+
16+
import unittest
17+
from core.src.local_loggers.FileLogger import FileLogger
18+
19+
20+
class MockFileHandle:
21+
def __init__(self, raise_on_write=False):
22+
self.raise_on_write = raise_on_write
23+
self.flushed = False
24+
self.fileno_called = False
25+
self.contents = ""
26+
self.closed = False
27+
28+
def write(self, message):
29+
if self.raise_on_write:
30+
raise Exception("Mock write exception")
31+
self.contents += message
32+
33+
def flush(self):
34+
self.flushed = True
35+
36+
def fileno(self):
37+
self.fileno_called = True
38+
return 1 # mock file
39+
40+
def close(self):
41+
self.closed = True
42+
43+
def __enter__(self):
44+
return self
45+
46+
def __exit__(self, exc_type, exc_value, traceback):
47+
self.close()
48+
49+
50+
class MockFileSystem:
51+
def __init__(self):
52+
self.files = {}
53+
54+
def open(self, file_path, mode):
55+
if "error" in file_path:
56+
raise Exception("Mock file open error")
57+
58+
if file_path not in self.files:
59+
self.files[file_path] = MockFileHandle()
60+
61+
return self.files[file_path]
62+
63+
64+
class MockEnvLayer:
65+
def __init__(self):
66+
self.file_system = MockFileSystem()
67+
self.datetime = self
68+
69+
def timestamp(self):
70+
return "2025-01-01T00:00:00Z"
71+
72+
73+
class TestFileLogger(unittest.TestCase):
74+
def setUp(self):
75+
self.mock_env_layer = MockEnvLayer()
76+
self.log_file = "test.log"
77+
self.file_logger = FileLogger(self.mock_env_layer, self.log_file)
78+
79+
def test_init_failure(self):
80+
""" Test when initiation object file open throws exception """
81+
self.mock_env_layer.file_system.open = lambda *args, **kwargs: (_ for _ in ()).throw(Exception("Mock file open error"))
82+
with self.assertRaises(Exception) as context:
83+
FileLogger(self.mock_env_layer, "error_log.log")
84+
85+
self.assertIn("Mock file open error", str(context.exception))
86+
87+
def test_write(self):
88+
""" Test FileLogger write() with no truncation """
89+
message = "Test message"
90+
self.file_logger.write(message)
91+
self.assertEqual(self.file_logger.log_file_handle.contents, message)
92+
93+
def test_write_truncation(self):
94+
""" Test FileLogger write() with truncation """
95+
max_msg_size = 32 * 1024 * 1024
96+
message = "A" * (32 * 1024 * 1024 + 10)
97+
self.file_logger.write(message)
98+
self.assertEqual(len(self.file_logger.log_file_handle.contents), max_msg_size)
99+
100+
def test_write_false_silent_failure(self):
101+
""" Test FileLogger write(), throws exception raise_on_write is true """
102+
self.file_logger.log_file_handle = MockFileHandle(raise_on_write=True)
103+
104+
with self.assertRaises(Exception) as context:
105+
self.file_logger.write("test message", fail_silently=False)
106+
107+
self.assertIn("Fatal exception trying to write to log file", str(context.exception))
108+
109+
def test_write_irrecoverable_exception(self):
110+
""" Test FileLogger write_irrecoverable_exception write failure log """
111+
message = "test message"
112+
self.file_logger.write_irrecoverable_exception(message)
113+
114+
self.assertIn(self.file_logger.log_failure_log_file, self.mock_env_layer.file_system.files)
115+
116+
failure_log = self.mock_env_layer.file_system.files[self.file_logger.log_failure_log_file]
117+
expected_output = "\n2025-01-01T00:00:00Z> test message"
118+
119+
self.assertIn(expected_output, failure_log.contents)
120+
121+
def test_write_irrecoverable_exception_failure(self):
122+
""" Test FileLogger write_irrecoverable_exception exception raised """
123+
self.file_logger.log_failure_log_file = "error_failure_log.log"
124+
message = "test message"
125+
126+
self.file_logger.write_irrecoverable_exception(message)
127+
128+
self.assertNotIn("error_failure_log.log", self.mock_env_layer.file_system.files)
129+
130+
def test_write_irrecoverable_exception_truncation(self):
131+
""" Test FileLogger write_irrecoverable_exception write failure log with truncation """
132+
timestamp_size = len("\n2025-01-01T00:00:00Z> ")
133+
max_msg_size = 32 * 1024 * 1024
134+
message = "A" * (32 * 1024 * 1024 + 10)
135+
self.file_logger.write_irrecoverable_exception(message)
136+
137+
self.assertIn(self.file_logger.log_failure_log_file, self.mock_env_layer.file_system.files)
138+
failure_log = self.mock_env_layer.file_system.files[self.file_logger.log_failure_log_file]
139+
self.assertEqual(len(failure_log.contents), max_msg_size + timestamp_size)
140+
141+
def test_flush_success(self):
142+
""" Test FileLogger flush() and fileno() are called"""
143+
self.file_logger.flush()
144+
145+
self.assertTrue(self.file_logger.log_file_handle.flushed)
146+
self.assertTrue(self.file_logger.log_file_handle.fileno_called)
147+
148+
149+
if __name__ == '__main__':
150+
unittest.main()

0 commit comments

Comments
 (0)