-
Notifications
You must be signed in to change notification settings - Fork 8
feat(export): include accumulated warnings in output json (#113) #116
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 all commits
5706fa6
46c8765
bf1d7e5
6f4b8a7
107e62e
69bb73b
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 |
|---|---|---|
|
|
@@ -11,6 +11,15 @@ class TraceEvent(dict): | |
| pass | ||
|
|
||
|
|
||
| class DiagnosticEvent(TraceEvent): | ||
| pass | ||
|
|
||
|
|
||
| # name of the meta-event used to carry an accumulated warning through the pipeline | ||
| # so the exporter can fold it into the output json instead of losing it in the console | ||
| TRACE_ISSUE_EVENT_NAME = "trace_issue" | ||
|
|
||
|
|
||
| class InputDialect: | ||
| categories = set() | ||
| dialect_map = {} | ||
|
|
@@ -292,13 +301,19 @@ def update(self, | |
| def has_warning(self) -> bool: | ||
| return self.occurred | ||
|
|
||
| def is_error(self) -> bool: | ||
| return self.warn_level == aiulog.ERROR | ||
|
|
||
| def severity(self) -> str: | ||
| return "error" if self.is_error() else "warning" | ||
|
|
||
| def add_instance(self, data: dict) -> None: | ||
| self._instances.append(data) | ||
|
|
||
| def to_verification_event_args(self) -> dict: | ||
| return { | ||
| "finding": self.name, | ||
| "is_error": self.warn_level == aiulog.ERROR, | ||
| "is_error": self.is_error(), | ||
| "count": self.args_list.get("count", len(self._instances)), | ||
|
Member
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. Just noticed this (and it existed this way before). I think, this becomes more complex than it should be. We're setting the loglevel based in If you don't mind, something like |
||
| "instances": list(self._instances), | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,78 @@ | ||
| # Copyright 2024-2026 IBM Corporation | ||
|
|
||
| import json | ||
|
|
||
| import pytest | ||
|
|
||
| from aiu_trace_analyzer.core.processing import EventProcessor | ||
| from aiu_trace_analyzer.pipeline.context import AbstractContext | ||
| from aiu_trace_analyzer.types import TraceWarning, TRACE_ISSUE_EVENT_NAME | ||
| from aiu_trace_analyzer.export.exporter import JsonFileTraceExporter | ||
|
|
||
|
|
||
| @pytest.fixture | ||
| def warned_context() -> AbstractContext: | ||
| warning = TraceWarning( | ||
| name="long_dur", | ||
| text="OVC: Detected {d[count]} long event(s).", | ||
| data={"count": 0}, | ||
| update_fn={"count": int.__add__}, | ||
| auto_log=False, | ||
| ) | ||
| ctx = AbstractContext(warnings=[warning]) | ||
| ctx.enable() | ||
| return ctx | ||
|
|
||
|
|
||
| def _processor_with(context: AbstractContext) -> EventProcessor: | ||
| proc = EventProcessor() | ||
| # register the stage directly to avoid pulling in a full StageProfile for the test | ||
| proc.stages.append((lambda event, ctx: [event], context, {})) | ||
| return proc | ||
|
|
||
|
|
||
| def test_drain_emits_active_warning_as_meta_event(warned_context): | ||
| warned_context.issue_warning("long_dur", {"count": 3}) | ||
|
|
||
| drained = _processor_with(warned_context).drain() | ||
|
|
||
| issue_events = [e for e in drained if e.name == TRACE_ISSUE_EVENT_NAME] | ||
| assert len(issue_events) == 1 | ||
| assert issue_events[0].args == {"finding": "long_dur", | ||
| "text": "OVC: Detected 3 long event(s).", | ||
| "is_error": False} | ||
|
|
||
|
|
||
| def test_drain_emits_nothing_when_no_warning(warned_context): | ||
| drained = _processor_with(warned_context).drain() | ||
|
|
||
| assert [e for e in drained if e.name == TRACE_ISSUE_EVENT_NAME] == [] | ||
|
|
||
|
|
||
| def test_warning_reaches_exporter_other_data(warned_context): | ||
| warned_context.issue_warning("long_dur", {"count": 3}) | ||
|
|
||
| drained = _processor_with(warned_context).drain() | ||
| exporter = JsonFileTraceExporter(target_uri="unused.json") | ||
| exporter.export(drained) | ||
|
|
||
| output = json.loads(exporter.get_data()) | ||
| assert output["otherData"]["warnings"] == [{ | ||
| "finding": "long_dur", | ||
| "text": "OVC: Detected 3 long event(s).", | ||
| }] | ||
| assert output["traceEvents"] == [] | ||
|
|
||
|
|
||
| def test_drain_warning_bypasses_remaining_pipeline_stages(warned_context): | ||
| warned_context.issue_warning("long_dur", {"count": 3}) | ||
| proc = _processor_with(warned_context) | ||
|
|
||
| def drop_everything(event, ctx): | ||
| return [] | ||
|
|
||
| proc.stages.append((drop_everything, None, {})) | ||
|
|
||
| drained = proc.drain() | ||
|
|
||
| assert [e for e in drained if e.name == TRACE_ISSUE_EVENT_NAME] |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,63 @@ | ||
| # Copyright 2024-2026 IBM Corporation | ||
|
|
||
| import json | ||
|
|
||
| import pytest | ||
|
|
||
| from aiu_trace_analyzer.types import TRACE_ISSUE_EVENT_NAME | ||
| from aiu_trace_analyzer.trace_view import AbstractEventType | ||
| from aiu_trace_analyzer.export.exporter import JsonFileTraceExporter | ||
|
|
||
|
|
||
| @pytest.fixture | ||
| def json_exporter() -> JsonFileTraceExporter: | ||
| return JsonFileTraceExporter(target_uri="unused.json") | ||
|
|
||
|
|
||
| def _issue_event(name: str, text: str, is_error: bool = False) -> AbstractEventType: | ||
| return AbstractEventType.from_dict({ | ||
| "ph": "M", "ts": 0, "pid": 0, | ||
| "name": TRACE_ISSUE_EVENT_NAME, | ||
| "args": {"finding": name, "text": text, "is_error": is_error}, | ||
| }) | ||
|
|
||
|
|
||
| def _instant_event() -> AbstractEventType: | ||
| return AbstractEventType.from_dict({ | ||
| "ph": "i", "ts": 1, "pid": 0, "tid": 0, "s": "g", | ||
| "name": "regular_event", "args": {}, | ||
| }) | ||
|
|
||
|
|
||
| def test_export_captures_issue_events(json_exporter): | ||
| text = "OVC: Detected 3 event(s) with long duration." | ||
| json_exporter.export([_issue_event("long_dur", text)]) | ||
|
|
||
| other_data = json.loads(json_exporter.get_data())["otherData"] | ||
| assert other_data["warnings"] == [{"finding": "long_dur", "text": text}] | ||
|
|
||
|
|
||
| def test_export_separates_errors_from_warnings(json_exporter): | ||
| json_exporter.export([_issue_event("long_dur", "warn text"), | ||
| _issue_event("bad_ts", "error text", is_error=True)]) | ||
|
|
||
| other_data = json.loads(json_exporter.get_data())["otherData"] | ||
| assert other_data["warnings"] == [{"finding": "long_dur", "text": "warn text"}] | ||
| assert other_data["errors"] == [{"finding": "bad_ts", "text": "error text"}] | ||
|
|
||
|
|
||
| def test_export_issue_events_do_not_leak_into_trace(json_exporter): | ||
| json_exporter.export([_issue_event("long_dur", "text"), _instant_event()]) | ||
|
|
||
| dumped = json.loads(json_exporter.get_data()) | ||
| names = [e["name"] for e in dumped["traceEvents"]] | ||
| assert TRACE_ISSUE_EVENT_NAME not in names | ||
| assert "regular_event" in names | ||
|
|
||
|
|
||
| def test_export_no_issue_section_when_absent(json_exporter): | ||
| json_exporter.export([_instant_event()]) | ||
|
|
||
| other_data = json.loads(json_exporter.get_data())["otherData"] | ||
| assert "warnings" not in other_data | ||
| assert "errors" not in other_data |
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.
unnecessary comment