From e49a13750337ad1e082fcfd6e83d2ae80623e7fe Mon Sep 17 00:00:00 2001 From: bingmokaka <103943264+bingmokaka@users.noreply.github.com> Date: Fri, 3 Jul 2026 21:26:27 +0800 Subject: [PATCH] Add independent log parser fixtures --- tests/fixtures/log_aggregator/json.log | 1 + tests/fixtures/log_aggregator/malformed.log | 1 + tests/fixtures/log_aggregator/nginx.log | 1 + tests/fixtures/log_aggregator/text.log | 1 + tests/test_log_aggregator_fixtures.py | 71 +++++++++++++++++++++ tools/log_aggregator.py | 4 +- 6 files changed, 77 insertions(+), 2 deletions(-) create mode 100644 tests/fixtures/log_aggregator/json.log create mode 100644 tests/fixtures/log_aggregator/malformed.log create mode 100644 tests/fixtures/log_aggregator/nginx.log create mode 100644 tests/fixtures/log_aggregator/text.log create mode 100644 tests/test_log_aggregator_fixtures.py diff --git a/tests/fixtures/log_aggregator/json.log b/tests/fixtures/log_aggregator/json.log new file mode 100644 index 00000000..210c522d --- /dev/null +++ b/tests/fixtures/log_aggregator/json.log @@ -0,0 +1 @@ +{"timestamp":"2024-05-17T12:34:56Z","level":"ERROR","service":"payments","message":"charge settlement failed","request_id":"req-7f2"} diff --git a/tests/fixtures/log_aggregator/malformed.log b/tests/fixtures/log_aggregator/malformed.log new file mode 100644 index 00000000..14e6a7b7 --- /dev/null +++ b/tests/fixtures/log_aggregator/malformed.log @@ -0,0 +1 @@ +this is not json and not nginx, but it should still be handled as text diff --git a/tests/fixtures/log_aggregator/nginx.log b/tests/fixtures/log_aggregator/nginx.log new file mode 100644 index 00000000..c0ea4f4c --- /dev/null +++ b/tests/fixtures/log_aggregator/nginx.log @@ -0,0 +1 @@ +203.0.113.24 - frank [17/May/2024:12:36:02 +0000] "GET /api/trials/42 HTTP/1.1" 502 512 "https://tent.example/dashboard" "Mozilla/5.0 fixture" diff --git a/tests/fixtures/log_aggregator/text.log b/tests/fixtures/log_aggregator/text.log new file mode 100644 index 00000000..c972401f --- /dev/null +++ b/tests/fixtures/log_aggregator/text.log @@ -0,0 +1 @@ +2024-05-17 12:35:01 WARN [scheduler] retrying delayed payout job diff --git a/tests/test_log_aggregator_fixtures.py b/tests/test_log_aggregator_fixtures.py new file mode 100644 index 00000000..09b97121 --- /dev/null +++ b/tests/test_log_aggregator_fixtures.py @@ -0,0 +1,71 @@ +import sys +import unittest +from pathlib import Path + +ROOT = Path(__file__).resolve().parents[1] +sys.path.insert(0, str(ROOT / "tools")) + +from log_aggregator import JSONLogParser, LogAggregator, NginxLogParser, TextLogParser + + +FIXTURE_DIR = ROOT / "tests" / "fixtures" / "log_aggregator" + + +def read_fixture(name: str) -> str: + return (FIXTURE_DIR / name).read_text(encoding="utf-8").strip() + + +class LogAggregatorFixtureTests(unittest.TestCase): + def test_json_fixture_parses_core_fields(self): + entry = JSONLogParser().parse(read_fixture("json.log")) + + self.assertIsNotNone(entry) + self.assertEqual(entry["format"], "json") + self.assertEqual(entry["timestamp"], "2024-05-17T12:34:56Z") + self.assertEqual(entry["level"], "ERROR") + self.assertEqual(entry["service"], "payments") + self.assertEqual(entry["message"], "charge settlement failed") + self.assertEqual(entry["fields"]["request_id"], "req-7f2") + + def test_text_fixture_parses_timestamp_level_and_service(self): + entry = TextLogParser().parse(read_fixture("text.log")) + + self.assertIsNotNone(entry) + self.assertEqual(entry["format"], "text") + self.assertEqual(entry["timestamp"], 1715949301) + self.assertEqual(entry["level"], "warn") + self.assertEqual(entry["service"], "scheduler") + self.assertIn("retrying delayed payout job", entry["message"]) + + def test_nginx_fixture_parses_key_fields(self): + entry = NginxLogParser().parse(read_fixture("nginx.log")) + + self.assertIsNotNone(entry) + self.assertEqual(entry["format"], "nginx") + self.assertEqual(entry["timestamp"], 1715949362) + self.assertEqual(entry["level"], "error") + self.assertEqual(entry["service"], "nginx") + self.assertEqual(entry["message"], "GET /api/trials/42 HTTP/1.1") + self.assertEqual(entry["fields"]["remote_addr"], "203.0.113.24") + self.assertEqual(entry["fields"]["remote_user"], "frank") + self.assertEqual(entry["fields"]["status"], 502) + self.assertEqual(entry["fields"]["body_bytes"], "512") + + def test_malformed_fixture_does_not_crash_aggregator(self): + aggregator = LogAggregator() + + self.assertTrue(aggregator._parse_line(read_fixture("malformed.log"))) + self.assertEqual(aggregator.entries[0]["format"], "text") + self.assertEqual(aggregator.entries[0]["level"], "unknown") + + def test_aggregator_routes_nginx_before_plain_text_fallback(self): + aggregator = LogAggregator() + + self.assertTrue(aggregator._parse_line(read_fixture("nginx.log"))) + self.assertEqual(aggregator.entries[0]["format"], "nginx") + self.assertEqual(aggregator.level_counts["error"], 1) + self.assertEqual(aggregator.service_counts["nginx"], 1) + + +if __name__ == "__main__": + unittest.main() diff --git a/tools/log_aggregator.py b/tools/log_aggregator.py index c9527d30..39e3ae83 100644 --- a/tools/log_aggregator.py +++ b/tools/log_aggregator.py @@ -187,7 +187,7 @@ def parse(self, line: str) -> Optional[Dict[str, Any]]: 'message': match.group(5), 'fields': { 'remote_addr': match.group(1), - 'remote_user': match.group(2), + 'remote_user': match.group(3), 'request': match.group(5), 'status': status_code, 'body_bytes': match.group(7), @@ -204,7 +204,7 @@ def parse(self, line: str) -> Optional[Dict[str, Any]]: class LogAggregator: def __init__(self): - self.parsers = [JSONLogParser(), TextLogParser(), NginxLogParser()] + self.parsers = [JSONLogParser(), NginxLogParser(), TextLogParser()] self.entries: List[Dict[str, Any]] = [] self.level_counts: Counter = Counter() self.service_counts: Counter = Counter()