feat: add publish_grafana_copy, Grafana examples, and optional OTel metrics (#89)
* feat: Grafana copy publishing, dashboard examples, and optional OTel metrics Implements three observability improvements: #82 — publish_grafana_copy(): Uses SQLite online backup API (WAL-safe) to atomically publish a consistent read-only copy beside the target. Adds --publish-copy option to grafana-schema and snapshot CLI commands. #83 — examples/grafana/: Minimal working Grafana setup with docker-compose, provisioning datasource/dashboard YAML, and three dashboard JSON files (mt5cli-overview, mt5cli-trades, mt5cli-market). All queries use grafana_* views; no credentials or private paths included. #84 — mt5cli/telemetry.py: Optional OTel metrics behind mt5cli[otel] extra. Base install is unaffected. Adds _Mt5Metrics singleton (no-op until configure_metrics() is called), wraps update_history() and update_observability() with record_history_update / record_snapshot_update context managers, and emits account/position gauges from snapshots. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: replace ambiguous multiplication sign in comment Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * chore: normalize markdown formatting in grafana README Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: preserve file mode on Grafana copy and fix unsupported time macro - publish_grafana_copy: chmod temp file to match the existing target's permissions (or 0o644 when no prior target exists) before atomic replace, so Grafana running as a different OS user (e.g. UID 472 in Docker) can read the published database - mt5cli-market.json: replace unsupported \$__timeFilter(time) with the epoch-based filter supported by frser-sqlite-datasource: "time" >= \$__from / 1000 AND "time" < \$__to / 1000 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: skip Windows-incompatible mode test, rename compose file to compose.yaml - Skip test_overwrite_preserves_existing_target_mode on win32 since Windows chmod does not preserve Unix group/other permission bits - Simplify test_fresh_target_has_readable_permissions to check owner read bit only (portable across platforms) - Rename docker-compose.yml -> compose.yaml (modern Compose convention) - Update README and test reference to match new filename Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * chore: rename *.yaml to *.yml in examples/grafana Renames compose.yaml, mt5cli-sqlite.yaml, and mt5cli.yaml to .yml; updates README and test references accordingly. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * chore: format Grafana dashboards and expand qa script to include JSON - Update qa.sh prettier pattern to format JSON files alongside markdown - Reformat Grafana dashboard JSONs with consistent spacing Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: address owner review comments before merge - qa.sh: fix Prettier glob from `{,d,json}` to `{md,json}` so Markdown files are actually formatted by local QA (P2) - compose.yml: add GF_INSTALL_PLUGINS env var so the frser-sqlite-datasource plugin is installed at container start (P1) - telemetry.py: replace no-op get_meter() call with a real SDK MeterProvider pipeline; add optional `readers` kwarg so callers can inject custom readers (e.g. InMemoryMetricReader in tests) without needing the OTLP package (P1) - sdk.py: aggregate profit and volume by symbol before emitting gauge values so hedging accounts with multiple same-symbol positions emit one point per symbol instead of overwriting with each row (P2) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: emit mt5_history_update_rows_total via conn.total_changes delta The counter was registered but never incremented, making the advertised history-update throughput metric permanently zero. Add add_history_rows() to _Mt5Metrics and call it in update_history() using the SQLite total_changes delta measured around write_incremental_datasets(). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: address three owner review comments - compose.yml: replace soft fallback with :? error expansion so Compose refuses to start when MT5CLI_DB_PATH is unset or empty (P1) - README.md: tell native Windows users to copy only the datasource provisioning file; the dashboards yml contains a Docker-specific path that is invalid on Windows (P2) - telemetry.py / sdk.py: emit mt5_terminal_connected, mt5_terminal_trade_allowed, and mt5_terminal_trade_expert gauges via a new record_terminal_state() method called from _snapshot_terminal(), completing the connection-status metric surface from issue #84 (P2) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * feat: add snapshot freshness panel and win-rate column to dashboards - mt5cli-overview.json: add a full-width "Last Snapshot" stat panel (dateTimeFromNow unit) below the account stats, querying MAX(time)*1000 from grafana_account_snapshots so users can tell whether Grafana is reading a current published copy (#83) - mt5cli-trades.json: add win_rate_pct computed column to the Trade Statistics by Symbol table via 100.0 * winning_deals / NULLIF( total_deals, 0), with a percent unit override and "Win Rate (%)" display label (#83) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: reject same source and target path in publish_grafana_copy Adds an early same-path guard to publish_grafana_copy: resolves both paths before any I/O and raises ValueError if they are identical, preventing the function from overwriting the live source database with its own backup copy. Also adds a unit test for the rejected case. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: address ruff EM102/TRY003/E501 in same-path guard Assigns the ValueError message to a variable before raising and shortens the test docstring to stay within the 88-char line limit. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: apply ruff format to publish_grafana_copy error message Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: remove grafana_ticks panel from default market dashboard The Tick Bid/Ask panel queried grafana_ticks which only exists when users collect tick data (opt-in). Users following the default OHLCV-only setup path hit "no such table: grafana_ticks" on dashboard load. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: close SQLite connections before atomic replace in publish_grafana_copy Wrap both src and dst connections with contextlib.closing() so they are explicitly closed before tmp_path.replace(target_path) runs. Without this, sqlite3.Connection's context manager only commits/rolls back but leaves the file handle open, which can cause PermissionError on Windows. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: rename history.grafana.db to history.mt5cli.db in Grafana examples frser-sqlite-datasource blocks paths containing "grafana.db" via its internal blocklist. Rename the recommended published filename in the README, compose comment, and datasource provisioning comment to avoid a blocked/denied datasource for native Windows users. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: update Docker Compose quick-start to pass MT5CLI_DB_PATH The compose.yml already required MT5CLI_DB_PATH via ${MT5CLI_DB_PATH:?...}, but the README still showed bare `docker compose up -d`. Update the section to show the env-var-prefixed invocation and document the .env file alternative. 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:
Daichi Narushima
co-authored by
Claude Sonnet 4.6
agent
Claude Sonnet 4.6
agent
parent
d27da02f3f
commit
1ffac45d57
+154
-1
@@ -4,8 +4,9 @@ from __future__ import annotations
|
||||
|
||||
import logging
|
||||
import sqlite3
|
||||
from pathlib import Path
|
||||
from typing import TYPE_CHECKING
|
||||
from unittest.mock import MagicMock
|
||||
from unittest.mock import MagicMock, patch
|
||||
|
||||
import pandas as pd
|
||||
import pytest
|
||||
@@ -24,6 +25,7 @@ from mt5cli.grafana import (
|
||||
insert_order_snapshots,
|
||||
insert_position_snapshots,
|
||||
insert_terminal_snapshot,
|
||||
publish_grafana_copy,
|
||||
record_snapshot_run,
|
||||
start_snapshot_run,
|
||||
)
|
||||
@@ -836,3 +838,154 @@ class TestSnapshotInserts:
|
||||
record_snapshot_run(conn, run_id, "ok")
|
||||
row = conn.execute("SELECT status, detail FROM snapshot_runs").fetchone()
|
||||
assert row == ("ok", None)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# TestPublishGrafanaCopy
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def _make_source_db(path: Path) -> None:
|
||||
"""Create a minimal source SQLite database with snapshot tables."""
|
||||
with sqlite3.connect(path) as conn:
|
||||
conn.execute("PRAGMA journal_mode=WAL")
|
||||
create_snapshot_tables(conn)
|
||||
conn.execute(
|
||||
"INSERT INTO snapshot_runs (observed_at, status) VALUES (?, 'ok')",
|
||||
(1700000000,),
|
||||
)
|
||||
|
||||
|
||||
class TestPublishGrafanaCopy:
|
||||
"""Tests for publish_grafana_copy."""
|
||||
|
||||
def test_publish_to_fresh_target(self, tmp_path: Path) -> None:
|
||||
"""publish_grafana_copy creates the target file."""
|
||||
source = tmp_path / "src.db"
|
||||
target = tmp_path / "out" / "grafana.db"
|
||||
_make_source_db(source)
|
||||
result = publish_grafana_copy(source, target)
|
||||
assert target.exists()
|
||||
assert result == target.resolve()
|
||||
|
||||
def test_overwrite_existing_target(self, tmp_path: Path) -> None:
|
||||
"""publish_grafana_copy replaces an existing target without error."""
|
||||
source = tmp_path / "src.db"
|
||||
target = tmp_path / "grafana.db"
|
||||
_make_source_db(source)
|
||||
target.write_bytes(b"stale")
|
||||
publish_grafana_copy(source, target)
|
||||
# Target must now be a valid SQLite file from source
|
||||
with sqlite3.connect(target) as conn:
|
||||
tables = {
|
||||
row[0]
|
||||
for row in conn.execute(
|
||||
"SELECT name FROM sqlite_master WHERE type='table'"
|
||||
).fetchall()
|
||||
}
|
||||
assert "snapshot_runs" in tables
|
||||
|
||||
def test_target_contains_source_tables(self, tmp_path: Path) -> None:
|
||||
"""Published target contains the same tables as the source."""
|
||||
source = tmp_path / "src.db"
|
||||
target = tmp_path / "grafana.db"
|
||||
_make_source_db(source)
|
||||
publish_grafana_copy(source, target)
|
||||
with sqlite3.connect(target) as conn:
|
||||
tables = {
|
||||
row[0]
|
||||
for row in conn.execute(
|
||||
"SELECT name FROM sqlite_master WHERE type='table'"
|
||||
).fetchall()
|
||||
}
|
||||
assert {"snapshot_runs", "account_snapshots"}.issubset(tables)
|
||||
|
||||
def test_target_can_be_opened_readonly(self, tmp_path: Path) -> None:
|
||||
"""Published target can be opened with uri=True in read-only mode."""
|
||||
source = tmp_path / "src.db"
|
||||
target = tmp_path / "grafana.db"
|
||||
_make_source_db(source)
|
||||
publish_grafana_copy(source, target)
|
||||
uri = f"file:{target}?mode=ro"
|
||||
with sqlite3.connect(uri, uri=True) as conn:
|
||||
row = conn.execute("SELECT status FROM snapshot_runs").fetchone()
|
||||
assert row == ("ok",)
|
||||
|
||||
def test_same_path_raises(self, tmp_path: Path) -> None:
|
||||
"""publish_grafana_copy raises ValueError when source equals target."""
|
||||
db = tmp_path / "history.db"
|
||||
_make_source_db(db)
|
||||
with pytest.raises(ValueError, match="must differ from the source"):
|
||||
publish_grafana_copy(db, db)
|
||||
|
||||
def test_source_not_found_raises(self, tmp_path: Path) -> None:
|
||||
"""publish_grafana_copy raises FileNotFoundError when source is absent."""
|
||||
with pytest.raises(FileNotFoundError):
|
||||
publish_grafana_copy(tmp_path / "missing.db", tmp_path / "out.db")
|
||||
|
||||
def test_preserve_old_target_on_backup_failure(self, tmp_path: Path) -> None:
|
||||
"""Old target is preserved when the backup fails."""
|
||||
source = tmp_path / "src.db"
|
||||
target = tmp_path / "grafana.db"
|
||||
_make_source_db(source)
|
||||
original_content = b"original_data"
|
||||
target.write_bytes(original_content)
|
||||
with patch("sqlite3.connect") as mock_connect:
|
||||
mock_src = MagicMock()
|
||||
mock_src.__enter__ = MagicMock(return_value=mock_src)
|
||||
mock_src.__exit__ = MagicMock(return_value=False)
|
||||
mock_src.backup.side_effect = sqlite3.OperationalError("backup failed")
|
||||
mock_connect.return_value = mock_src
|
||||
with pytest.raises(sqlite3.OperationalError, match="backup failed"):
|
||||
publish_grafana_copy(source, target)
|
||||
assert target.read_bytes() == original_content
|
||||
|
||||
def test_temp_file_cleaned_up_on_failure(self, tmp_path: Path) -> None:
|
||||
"""Temporary file is removed when backup raises an exception."""
|
||||
source = tmp_path / "src.db"
|
||||
target = tmp_path / "grafana.db"
|
||||
_make_source_db(source)
|
||||
with patch("sqlite3.connect") as mock_connect:
|
||||
mock_src = MagicMock()
|
||||
mock_src.__enter__ = MagicMock(return_value=mock_src)
|
||||
mock_src.__exit__ = MagicMock(return_value=False)
|
||||
mock_src.backup.side_effect = sqlite3.OperationalError("fail")
|
||||
mock_connect.return_value = mock_src
|
||||
with pytest.raises(sqlite3.OperationalError):
|
||||
publish_grafana_copy(source, target)
|
||||
tmp_files = list(tmp_path.glob("grafana.db.*.tmp"))
|
||||
assert not tmp_files, "Temp file should be cleaned up on failure"
|
||||
|
||||
def test_returns_path_object(self, tmp_path: Path) -> None:
|
||||
"""publish_grafana_copy returns a Path instance."""
|
||||
source = tmp_path / "src.db"
|
||||
target = tmp_path / "grafana.db"
|
||||
_make_source_db(source)
|
||||
result = publish_grafana_copy(source, target)
|
||||
assert isinstance(result, Path)
|
||||
|
||||
def test_fresh_target_has_readable_permissions(self, tmp_path: Path) -> None:
|
||||
"""Published copy is readable by the owner."""
|
||||
import stat as _stat # noqa: PLC0415
|
||||
|
||||
source = tmp_path / "src.db"
|
||||
target = tmp_path / "grafana.db"
|
||||
_make_source_db(source)
|
||||
publish_grafana_copy(source, target)
|
||||
mode = target.stat().st_mode & 0o777
|
||||
assert bool(mode & _stat.S_IRUSR), "owner must be able to read"
|
||||
|
||||
@pytest.mark.skipif(
|
||||
__import__("sys").platform == "win32",
|
||||
reason="Windows does not support Unix-style group/other permission bits",
|
||||
)
|
||||
def test_overwrite_preserves_existing_target_mode(self, tmp_path: Path) -> None:
|
||||
"""Overwriting an existing target preserves that target's file mode."""
|
||||
source = tmp_path / "src.db"
|
||||
target = tmp_path / "grafana.db"
|
||||
_make_source_db(source)
|
||||
target.write_bytes(b"old")
|
||||
target.chmod(0o640)
|
||||
publish_grafana_copy(source, target)
|
||||
mode = target.stat().st_mode & 0o777
|
||||
assert mode == 0o640
|
||||
|
||||
Reference in New Issue
Block a user