feat: add Grafana-ready SQLite observability (#86)
* feat: add Grafana-ready SQLite observability (#79, #80, #81) New `mt5cli/grafana.py` module with idempotent DDL helpers: - `create_snapshot_tables` — five SQLite tables for time-series account, position, order, terminal, and run-status snapshots - `create_grafana_views` — 13 `grafana_*` views with integer epoch-second `time` columns; missing source tables emit warnings and are skipped - `create_grafana_indexes` — 9 performance indexes guarded by column checks - `ensure_grafana_schema` — convenience wrapper calling all three above - Insert helpers: `insert_account_snapshot`, `insert_position_snapshots`, `insert_order_snapshots`, `insert_terminal_snapshot`, `record_snapshot_run` New stable SDK exports in `mt5cli.__init__` and `mt5cli.contract`: - `update_observability` — appends a timestamped snapshot to a SQLite db using an already-connected `Mt5DataClient`; never places orders - `update_observability_with_config` — standalone wrapper that opens and closes the MT5 connection automatically New CLI commands (Collection panel): - `grafana-schema` — idempotent schema setup, no MT5 connection required - `snapshot` — append account/position/order/terminal rows; supports `--symbol`, `--with-account/--no-account`, and equivalent flags All public modules maintain 100 % branch coverage; 968 tests pass. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * chore: apply mdformat to docs after grafana observability additions Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * chore: bump version to 1.1.0 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: address PR #86 review feedback - Fix unaggregated time in grafana_realized_pnl GROUP BY query (MAX) - Replace O(N) per-symbol API calls with single call + client-side filter - Eliminate double create_snapshot_tables when with_grafana_schema=True - Move grafana imports to module level in sdk.py (remove PLC0415 noqa) - Default with_grafana_schema to False (run grafana-schema once for setup) - Fix README position example to use snapshot_runs for latest snapshot - Update tests to reflect new behavior and correct patch targets Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * chore: apply ruff formatting and sync lock file Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: address owner review feedback on PR #86 - Filter grafana_*_snapshots views to only expose rows from successful runs (JOIN snapshot_runs WHERE status='ok'), closing the partial-snapshot visibility gap raised in PRRT_kwDORzI_286MvDJ6 - Add issubset column guards for snapshot table index creation, consistent with the rest of create_grafana_indexes (PRRT_kwDORzI_286MvUx1) - Fix README example queries: views expose 'time' not 'observed_at' (PRRT_kwDORzI_286MvUxw) - Correct public-contract.md default for with_grafana_schema (False, not True) and point to grafana-schema command (PRRT_kwDORzI_286MvUxy) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: add run_id to snapshot schema and fix stale-positions README query Replace second-level observed_at as the join key between snapshot_runs and snapshot tables with a stable run_id INTEGER PRIMARY KEY. Two runs in the same second now get distinct run_ids, preventing view duplication and cross-contamination from a failed run. Update README example to use snapshot_runs for latest-snapshot lookup so zero-position runs return an empty result instead of stale rows. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * chore: format markdown tables Align table column widths in README and public-contract documentation. Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com> * fix: close sqlite connections deterministically * fix: require entry filter in grafana_realized_pnl and add time to grafana_trade_stats grafana_realized_pnl now requires the entry column and filters to close-side deals (entry IN (1, 2, 3)), consistent with grafana_symbol_pnl and grafana_trade_stats. grafana_trade_stats now requires the time column and emits MAX(time_expr) AS "time" so it satisfies the documented Grafana view contract (integer epoch-second time column throughout). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: normalize pd.Timestamp time_setup to epoch int in insert_order_snapshots orders_get_as_df() returns datetime-converted columns by default, so time_setup is a pd.Timestamp in normal use. Passing it directly to sqlite3.executemany raises ProgrammingError. Added _to_epoch_int helper that converts datetime.datetime subclasses (including pd.Timestamp) and raw int/float values to integer epoch seconds, returning None for other types. Regression tests cover all four input paths. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: pre-drop all grafana_* views at start of create_grafana_views Previously, a builder that skipped due to a missing source table or column would not drop the view it owned, leaving stale views referencing gone tables. Now create_grafana_views drops all 13 known grafana_* views before calling any builder, so a schema refresh always removes views whose source has disappeared. Regression test covers the create → drop-source → refresh cycle. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: expose run_id in snapshot views and drop time from summary views Grafana snapshot views now expose run_id so latest-state queries can use MAX(run_id) instead of the ambiguous second-level MAX(observed_at). grafana_realized_pnl and grafana_trade_stats lose their MAX(time) column and are reclassified as static summary views; their all-time aggregates are not filterable by Grafana time-range selectors. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: guard snapshot views against missing run_id and fix README view docs _build_snapshot_view now skips with a warning when the underlying snapshot table exists but lacks a run_id column, preventing a broken view that fails at query time. Adds a regression test for that path. README Grafana section now qualifies that grafana_realized_pnl and grafana_trade_stats are static summary views (no time column) and splits the view table to match docs/api/public-contract.md. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: agent <agent@localhost> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -1855,6 +1855,174 @@ class TestCollectHistory:
|
||||
)
|
||||
|
||||
|
||||
class TestGrafanaSchemaCommand:
|
||||
"""Tests for the grafana-schema CLI command."""
|
||||
|
||||
def test_grafana_schema_creates_snapshot_tables_in_sqlite(
|
||||
self,
|
||||
tmp_path: Path,
|
||||
) -> None:
|
||||
"""grafana-schema applies Grafana schema to a SQLite database."""
|
||||
output = tmp_path / "out.db"
|
||||
result = runner.invoke(app, ["-o", str(output), "grafana-schema"])
|
||||
assert result.exit_code == 0, result.output
|
||||
with sqlite3.connect(output) as conn:
|
||||
tables = {
|
||||
row[0]
|
||||
for row in conn.execute(
|
||||
"SELECT name FROM sqlite_master WHERE type='table'"
|
||||
).fetchall()
|
||||
}
|
||||
assert "snapshot_runs" in tables
|
||||
assert "account_snapshots" in tables
|
||||
|
||||
def test_grafana_schema_is_idempotent(self, tmp_path: Path) -> None:
|
||||
"""grafana-schema can be invoked multiple times without error."""
|
||||
output = tmp_path / "out.db"
|
||||
result1 = runner.invoke(app, ["-o", str(output), "grafana-schema"])
|
||||
result2 = runner.invoke(app, ["-o", str(output), "grafana-schema"])
|
||||
assert result1.exit_code == 0, result1.output
|
||||
assert result2.exit_code == 0, result2.output
|
||||
|
||||
def test_grafana_schema_rejects_non_sqlite_output(
|
||||
self,
|
||||
tmp_path: Path,
|
||||
) -> None:
|
||||
"""grafana-schema fails when output is not a SQLite3 format."""
|
||||
result = runner.invoke(
|
||||
app,
|
||||
["-o", str(tmp_path / "out.csv"), "grafana-schema"],
|
||||
)
|
||||
assert result.exit_code != 0
|
||||
assert "grafana-schema requires SQLite3 output" in result.output
|
||||
|
||||
|
||||
class TestSnapshotCommand:
|
||||
"""Tests for the snapshot CLI command."""
|
||||
|
||||
def test_snapshot_rejects_non_sqlite_output(self, tmp_path: Path) -> None:
|
||||
"""Snapshot fails when output is not a SQLite3 format."""
|
||||
result = runner.invoke(
|
||||
app,
|
||||
["-o", str(tmp_path / "out.csv"), "snapshot"],
|
||||
)
|
||||
assert result.exit_code != 0
|
||||
assert "snapshot requires SQLite3 output" in result.output
|
||||
|
||||
def test_snapshot_delegates_to_update_observability_with_config(
|
||||
self,
|
||||
tmp_path: Path,
|
||||
mocker: MockerFixture,
|
||||
) -> None:
|
||||
"""Snapshot calls sdk.update_observability_with_config."""
|
||||
updater = mocker.patch("mt5cli.cli.sdk.update_observability_with_config")
|
||||
output = tmp_path / "out.db"
|
||||
result = runner.invoke(app, ["-o", str(output), "snapshot"])
|
||||
assert result.exit_code == 0, result.output
|
||||
updater.assert_called_once()
|
||||
kwargs = updater.call_args.kwargs
|
||||
assert kwargs["output"] == output
|
||||
assert kwargs["symbols"] is None
|
||||
assert kwargs["include_account"] is True
|
||||
assert kwargs["include_positions"] is True
|
||||
assert kwargs["include_orders"] is True
|
||||
assert kwargs["include_terminal"] is True
|
||||
assert kwargs["with_grafana_schema"] is False
|
||||
|
||||
def test_snapshot_with_symbol_filter(
|
||||
self,
|
||||
tmp_path: Path,
|
||||
mocker: MockerFixture,
|
||||
) -> None:
|
||||
"""Snapshot passes symbol list to update_observability_with_config."""
|
||||
updater = mocker.patch("mt5cli.cli.sdk.update_observability_with_config")
|
||||
result = runner.invoke(
|
||||
app,
|
||||
[
|
||||
"-o",
|
||||
str(tmp_path / "out.db"),
|
||||
"snapshot",
|
||||
"--symbol",
|
||||
"EURUSD",
|
||||
"--symbol",
|
||||
"GBPUSD",
|
||||
],
|
||||
)
|
||||
assert result.exit_code == 0, result.output
|
||||
kwargs = updater.call_args.kwargs
|
||||
assert kwargs["symbols"] == ["EURUSD", "GBPUSD"]
|
||||
|
||||
def test_snapshot_with_no_account_flag(
|
||||
self,
|
||||
tmp_path: Path,
|
||||
mocker: MockerFixture,
|
||||
) -> None:
|
||||
"""--no-account disables account snapshotting."""
|
||||
updater = mocker.patch("mt5cli.cli.sdk.update_observability_with_config")
|
||||
result = runner.invoke(
|
||||
app,
|
||||
["-o", str(tmp_path / "out.db"), "snapshot", "--no-account"],
|
||||
)
|
||||
assert result.exit_code == 0, result.output
|
||||
assert updater.call_args.kwargs["include_account"] is False
|
||||
|
||||
def test_snapshot_with_no_positions_flag(
|
||||
self,
|
||||
tmp_path: Path,
|
||||
mocker: MockerFixture,
|
||||
) -> None:
|
||||
"""--no-positions disables position snapshotting."""
|
||||
updater = mocker.patch("mt5cli.cli.sdk.update_observability_with_config")
|
||||
result = runner.invoke(
|
||||
app,
|
||||
["-o", str(tmp_path / "out.db"), "snapshot", "--no-positions"],
|
||||
)
|
||||
assert result.exit_code == 0, result.output
|
||||
assert updater.call_args.kwargs["include_positions"] is False
|
||||
|
||||
def test_snapshot_with_no_orders_flag(
|
||||
self,
|
||||
tmp_path: Path,
|
||||
mocker: MockerFixture,
|
||||
) -> None:
|
||||
"""--no-orders disables order snapshotting."""
|
||||
updater = mocker.patch("mt5cli.cli.sdk.update_observability_with_config")
|
||||
result = runner.invoke(
|
||||
app,
|
||||
["-o", str(tmp_path / "out.db"), "snapshot", "--no-orders"],
|
||||
)
|
||||
assert result.exit_code == 0, result.output
|
||||
assert updater.call_args.kwargs["include_orders"] is False
|
||||
|
||||
def test_snapshot_with_no_terminal_flag(
|
||||
self,
|
||||
tmp_path: Path,
|
||||
mocker: MockerFixture,
|
||||
) -> None:
|
||||
"""--no-terminal disables terminal snapshotting."""
|
||||
updater = mocker.patch("mt5cli.cli.sdk.update_observability_with_config")
|
||||
result = runner.invoke(
|
||||
app,
|
||||
["-o", str(tmp_path / "out.db"), "snapshot", "--no-terminal"],
|
||||
)
|
||||
assert result.exit_code == 0, result.output
|
||||
assert updater.call_args.kwargs["include_terminal"] is False
|
||||
|
||||
def test_snapshot_with_no_grafana_schema_flag(
|
||||
self,
|
||||
tmp_path: Path,
|
||||
mocker: MockerFixture,
|
||||
) -> None:
|
||||
"""--no-grafana-schema disables Grafana schema creation."""
|
||||
updater = mocker.patch("mt5cli.cli.sdk.update_observability_with_config")
|
||||
result = runner.invoke(
|
||||
app,
|
||||
["-o", str(tmp_path / "out.db"), "snapshot", "--no-grafana-schema"],
|
||||
)
|
||||
assert result.exit_code == 0, result.output
|
||||
assert updater.call_args.kwargs["with_grafana_schema"] is False
|
||||
|
||||
|
||||
class TestMain:
|
||||
"""Tests for the main entry point."""
|
||||
|
||||
|
||||
Reference in New Issue
Block a user