diff --git a/README.md b/README.md index 0352701..a624524 100644 --- a/README.md +++ b/README.md @@ -162,9 +162,9 @@ When you play in your own browser (the Pyodide front-end above), those same slot ## Usage reporting -Usage reporting is on by default: FishE sends a `startup` event once per launch and a `save-loaded` event each time a save slot is created or opened to [trace](https://github.com/Stephenson-Software/trace) at `https://trace.danielstephenson.dev`, each carrying only the program name (`FishE`) and the version from `version.txt`. Nothing about you or your run is sent — no username, hostname, IP address, path, slot number or save contents. The in-browser front-end (`UIType.PYODIDE`) is silent: the game runs in your tab, where the client's background thread cannot exist, and nothing is sent from there. +Usage reporting is on by default: FishE sends a `startup` event once per launch and a `save-loaded` event each time a save slot is created or opened to [trace](https://github.com/Stephenson-Software/trace) at `https://trace.danielstephenson.dev`, each carrying only the program name (`FishE`), the version from `version.txt` and a random installation ID (the tag `install`, so installations can be counted rather than launches). Nothing about you or your run is sent — no username, hostname, IP address, path, slot number or save contents. The in-browser front-end (`UIType.PYODIDE`) is silent: the game runs in your tab, where the client's background thread cannot exist, and nothing is sent from there. -The first time an install reports, one line saying so is printed on the console (above the save-file menu) and a `usage-reporting-notice-shown` marker is left in the save directory so it is not printed again. Reporting never gets in the game's way: it happens on a background thread, never raises into the game, and a server that is down or slow costs a dropped event, not a wait. +The first time an install reports, one line saying so is printed on the console (above the save-file menu) and a `usage-reporting-notice-shown` marker is left in the save directory so it is not printed again. The installation ID is a random UUID kept next to it, in `trace-install-id` in the save directory (`data/`, or `FISHE_SAVE_DIR`); it identifies no person, account or address, and deleting the file gives a new one. It is only created while reporting is on, so every opt-out below also stops it, and the in-browser front-end makes none. Reporting never gets in the game's way: it happens on a background thread, never raises into the game, and a server that is down or slow costs a dropped event, not a wait. To turn it off, set any of these in the environment the game runs in: @@ -183,8 +183,9 @@ FISHE_USAGE_REPORTING_ENABLED=false python3 src/fishE.py | `FISHE_USAGE_REPORTING_KEY` | the key issued to FishE | The program key sent with each event. | | `TRACE_USAGE_REPORTING` | unset | `off`, `false`, `0` or `no` turns reporting off, for FishE and every other trace client. | | `DO_NOT_TRACK` | unset | `1`, `true` or `yes` turns reporting off the same way. | +| `TRACE_INSTALL_ID` | unset | Sent as the installation ID instead of the one in `trace-install-id`, which is then left alone — to pin one for a container or service. | -The test suite switches reporting off for every test (`tests/conftest.py`), and the tests that exercise it point at a loopback stub server, so running the tests never reports anything either. The client is `src/trace_client.py`, vendored as one standard-library file from [trace-client-python](https://github.com/Stephenson-Software/trace-client-python) (0.3.0); the wiring is `src/usageReporting.py`. +The test suite switches reporting off for every test (`tests/conftest.py`), and the tests that exercise it point at a loopback stub server, so running the tests never reports anything either. The client is `src/trace_client.py`, vendored as one standard-library file from [trace-client-python](https://github.com/Stephenson-Software/trace-client-python) (0.4.0); the wiring is `src/usageReporting.py`. Details: https://github.com/Stephenson-Software/trace#usage-reporting diff --git a/src/trace_client.py b/src/trace_client.py index 9997993..a8a3803 100644 --- a/src/trace_client.py +++ b/src/trace_client.py @@ -1,4 +1,4 @@ -"""trace-client 0.3.0 -- https://github.com/Stephenson-Software/trace-client-python +"""trace-client 0.4.0 -- https://github.com/Stephenson-Software/trace-client-python One call to report that a program was used. Copy this file into a project as is, or vendor the package; either way there is nothing else to add. Standard @@ -14,12 +14,14 @@ import logging import os import queue +import re import threading import urllib.error import urllib.request -from typing import Dict, Mapping, Optional +import uuid +from typing import Dict, Mapping, Optional, Union -__version__ = "0.3.0" +__version__ = "0.4.0" _LOG = logging.getLogger("trace") @@ -42,6 +44,16 @@ #: limit on a tag value. MAX_TAG_LENGTH = 255 +#: The most tags one event carries: the trace server's limit. The ``install`` +#: tag is only added while an event has fewer than this. +MAX_TAGS = 32 + +#: The tag every event carries the installation's ID as. +INSTALL_TAG = "install" + +# What install_id_from_file accepts as an ID on a line of its file. +_INSTALL_ID_LINE = re.compile(r"^[A-Za-z0-9_.-]{1,%d}$" % MAX_TAG_LENGTH) + def environment_opts_out(environ: Optional[Mapping[str, str]] = None) -> bool: """Whether the environment asks for usage reporting to be off, via @@ -89,6 +101,16 @@ class TraceClient: release as well as a ``startup`` one. An event's own ``version`` tag wins over it. + Every event also carries a random per-installation ID as the tag + ``install``, so the trace server can count distinct installations rather + than raw events -- when the program supplies one: ``install_id=`` (an ID + it stores itself) or ``install_id_file=`` (a path the client loads the ID + from, or writes a new random one to; see :meth:`install_id_from_file`). + Without either, no ``install`` tag is sent and nothing is written + anywhere. Both are resolved only after the opt-outs, so a disabled client + never makes up an ID and never writes one. An event's own ``install`` tag + wins over it. + :: trace = TraceClient("https://trace.example.org", "roam", __version__, @@ -102,11 +124,21 @@ class TraceClient: TIMEOUT_SECONDS = 5.0 def __init__(self, base_url: str, application: str, version: str, *, key: Optional[str] = None, - enabled: bool = True) -> None: + enabled: bool = True, install_id: Optional[str] = None, + install_id_file: Optional[Union[str, "os.PathLike[str]"]] = None) -> None: """A client for the program named ``application``, at ``version``, reporting to the trace server at ``base_url``. The version is sent as the tag ``version`` on every event; a blank one, or one longer than - :data:`MAX_TAG_LENGTH` characters, is a :class:`ValueError`.""" + :data:`MAX_TAG_LENGTH` characters, is a :class:`ValueError`. + + ``install_id`` is the installation's ID, sent as the tag ``install`` + on every event. It should be random -- e.g. a :func:`uuid.uuid4` the + program stores in its own settings -- and never derived from a + person, account or address. Trimmed; ``None`` or blank means none; + longer than :data:`MAX_TAG_LENGTH` characters is a + :class:`ValueError`. ``install_id_file`` is a path to load it from + (or create it in) with :meth:`install_id_from_file`, only when the + client is enabled; an explicit ``install_id`` wins over it.""" if not base_url or not base_url.strip(): raise ValueError("base_url is required") if not application or not application.strip(): @@ -115,6 +147,9 @@ def __init__(self, base_url: str, application: str, version: str, *, key: Option raise ValueError("version is required") if len(version.strip()) > MAX_TAG_LENGTH: raise ValueError("version is longer than %d characters" % MAX_TAG_LENGTH) + explicit_install_id = (install_id or "").strip() or None + if explicit_install_id is not None and len(explicit_install_id) > MAX_TAG_LENGTH: + raise ValueError("install_id is longer than %d characters" % MAX_TAG_LENGTH) self._endpoint = base_url.strip().rstrip("/") + "/api/metrics" self._application = application.strip() self._version = version.strip() @@ -132,7 +167,14 @@ def __init__(self, base_url: str, application: str, version: str, *, key: Option self.disabled_reason = REASON_CONFIG elif not self._key: self.disabled_reason = REASON_NO_KEY + self._install_id: Optional[str] = None if self.disabled_reason is None: + # After the opt-outs, never before: a disabled client neither + # makes up an ID nor writes one to disk. + if explicit_install_id is not None: + self._install_id = explicit_install_id + elif install_id_file is not None: + self._install_id = TraceClient.install_id_from_file(install_id_file) self._queue = queue.Queue(maxsize=self.QUEUE_CAPACITY) self._thread = threading.Thread(target=self._drain, name="trace-client/" + self._application, daemon=True) @@ -143,6 +185,57 @@ def disabled(cls) -> "TraceClient": """A client that reports nothing. Useful as a default before settings are read.""" return cls("http://disabled.invalid", "disabled", "disabled", enabled=False) + @property + def install_id(self) -> Optional[str]: + """The per-installation ID every event carries as the tag ``install``, + or ``None`` when the client is disabled or was given none (no + ``install_id`` and no ``install_id_file``).""" + return self._install_id + + @staticmethod + def install_id_from_file(path: Union[str, "os.PathLike[str]"]) -> str: + """The installation's ID kept in the file at ``path``, which the + program chooses -- there is no default location. The first line that + is an ID (``[A-Za-z0-9_.-]``, at most :data:`MAX_TAG_LENGTH` + characters, surrounding whitespace ignored) is returned. If the file + does not exist or holds no such line, a new random :func:`uuid.uuid4` + is written to it (parent directories created) and returned. Delete + the file to get a new one. + + Never raises: if the file exists but cannot be read, or cannot be + written, a fresh random ID is returned for this process only, and an + unreadable file is left as it is. + + Called directly, this writes whatever the opt-outs say. Pass the path + as ``install_id_file=`` instead to keep the guarantee that a disabled + client writes nothing.""" + fresh = str(uuid.uuid4()) + try: + target = os.fspath(path) + if not target or not str(target).strip(): + return fresh + try: + with open(target, "r", encoding="utf-8") as existing: + for line in existing: + candidate = line.strip() + if _INSTALL_ID_LINE.match(candidate): + return candidate + except FileNotFoundError: + pass + except Exception as failure: # noqa: BLE001 - unreadable: never overwrite it + _LOG.debug("[trace] could not read install ID file %s, using an in-memory one: %s", + target, failure) + return fresh + parent = os.path.dirname(os.path.abspath(target)) + os.makedirs(parent, exist_ok=True) + with open(target, "w", encoding="utf-8") as written: + written.write(fresh + "\n") + return fresh + except Exception as failure: # noqa: BLE001 - an ID must never be the reason a program stops + _LOG.debug("[trace] could not write install ID file %s, using an in-memory one: %s", + path, failure) + return fresh + @property def enabled(self) -> bool: """Whether :meth:`report` will actually send anything. ``False`` after @@ -159,7 +252,8 @@ def report(self, name: str, value: Optional[float] = None, try: if not name or not name.strip(): return - body = _json(self._application, name, value, _with_version(tags, self._version)) + tags = _with_install(_with_version(tags, self._version), self._install_id) + body = _json(self._application, name, value, tags) self._queue.put_nowait(body) except queue.Full: _LOG.debug("[trace] queue full, dropped %s", name) @@ -200,7 +294,10 @@ def _drain(self) -> None: body = q.get() if body is None: return - self._send(body) + try: + self._send(body) + except Exception as failure: # noqa: BLE001 - a dead sender would leave the queue filling forever + _LOG.debug("[trace] sender failed on %s: %s", body, failure) def _send(self, body: bytes) -> None: try: @@ -242,6 +339,16 @@ def _with_version(tags: Optional[Mapping[str, str]], version: str) -> Dict[str, return merged +def _with_install(tags: Dict[str, str], install_id: Optional[str]) -> Dict[str, str]: + """The tags plus ``install``, unless they already carry one, there is no + ID, or adding it would pass :data:`MAX_TAGS`. A copy when it adds.""" + if install_id is None or INSTALL_TAG in tags or len(tags) >= MAX_TAGS: + return tags + merged = dict(tags) + merged[INSTALL_TAG] = install_id + return merged + + def _json(application: str, name: str, value: Optional[float], tags: Optional[Mapping[str, str]]) -> bytes: payload: Dict[str, object] = {"application": application, "name": name} if value is not None and value == value and value not in (float("inf"), float("-inf")): diff --git a/src/usageReporting.py b/src/usageReporting.py index 1ee016e..37bcbb5 100644 --- a/src/usageReporting.py +++ b/src/usageReporting.py @@ -4,8 +4,12 @@ FishE reports two events to https://trace.danielstephenson.dev through the vendored trace client (src/trace_client.py): ``startup`` once per launch and ``save-loaded`` each time a save slot is created or opened. Each carries the -program name and the version from version.txt, and nothing else - no username, -hostname, address, path, slot number or anything about the run. +program name, the version from version.txt and a random installation ID (the +tag ``install``, so installations can be counted rather than launches), and +nothing else - no username, hostname, address, path, slot number or anything +about the run. The ID is a UUID the client keeps in ``trace-install-id`` in the +save directory (``TRACE_INSTALL_ID`` in the environment pins one instead); the +client only reads or creates it when reporting is on. Reporting is on by default and switched off with ``FISHE_USAGE_REPORTING_ENABLED=false`` in the environment (see @@ -41,6 +45,11 @@ # is. SaveFileManager ignores it: only slot_N directories are save slots. NOTICE_MARKER_FILENAME = "usage-reporting-notice-shown" +# The file in the save directory the client keeps this installation's random +# ID in (the tag ``install`` on every event). Deleting it resets the ID; like +# the marker, SaveFileManager does not read it as a save slot. +INSTALL_ID_FILENAME = "trace-install-id" + OPT_OUT_INSTRUCTION = ( "FISHE_USAGE_REPORTING_ENABLED=false or TRACE_USAGE_REPORTING=off " "in the environment" @@ -50,7 +59,8 @@ NOTICE = ( "Usage reporting is on: %s sends a startup event and a save-loaded event " - "(program name and version only) to trace.danielstephenson.dev. " + "(program name, version and a random installation ID only) to " + "trace.danielstephenson.dev. " "Turn it off with %s. Details: %s" % (PROGRAM_NAME, OPT_OUT_INSTRUCTION, DETAILS_URL) ) @@ -89,7 +99,11 @@ def createClient(config): goes through the client's constructor, which decides in this order: TRACE_USAGE_REPORTING / DO_NOT_TRACK in the environment, then FishE's own setting, then whether there is a key. A client switched off by any of - them reports nothing, starts no thread, and says why in disabled_reason.""" + them reports nothing, starts no thread, and says why in disabled_reason. + + Only an enabled client resolves the installation ID: TRACE_INSTALL_ID + when set, otherwise the file at installIdPath(config), created on first + use - so no opt-out ever creates it.""" if isBrowserBuild(): return TraceClient.disabled() return TraceClient( @@ -98,9 +112,15 @@ def createClient(config): programVersion(), key=config.usageReportingKey, enabled=config.usageReportingEnabled, + install_id=os.environ.get("TRACE_INSTALL_ID"), + install_id_file=installIdPath(config), ) +def installIdPath(config): + return os.path.join(config.dataDirectory, INSTALL_ID_FILENAME) + + def noticeMarkerPath(config): return os.path.join(config.dataDirectory, NOTICE_MARKER_FILENAME) diff --git a/tests/conftest.py b/tests/conftest.py index 8f24f8c..c387217 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -17,3 +17,4 @@ def usageReportingOff(monkeypatch): # from a clean slate, so both are cleared here. monkeypatch.delenv("TRACE_USAGE_REPORTING", raising=False) monkeypatch.delenv("DO_NOT_TRACK", raising=False) + monkeypatch.delenv("TRACE_INSTALL_ID", raising=False) diff --git a/tests/test_trace_client.py b/tests/test_trace_client.py index efe9935..812a9a7 100644 --- a/tests/test_trace_client.py +++ b/tests/test_trace_client.py @@ -4,13 +4,16 @@ import json import logging import os +import shutil +import tempfile import threading import time import unittest +import uuid from http.server import BaseHTTPRequestHandler, ThreadingHTTPServer from unittest import mock -from trace_client import MAX_TAG_LENGTH, TraceClient, environment_opts_out +from trace_client import MAX_TAG_LENGTH, MAX_TAGS, TraceClient, environment_opts_out _ENV_VARS = ("TRACE_USAGE_REPORTING", "DO_NOT_TRACK") @@ -211,12 +214,12 @@ def test_environment_opts_out_accepts_an_explicit_mapping(self): def test_user_agent_names_the_client_version(self): from trace_client import __version__ - self.assertEqual("0.3.0", __version__) + self.assertEqual("0.4.0", __version__) client = TraceClient(self.base_url, "MyGame", "1.2.3", key="k") client.report("startup") self.assertTrue(self.capture.arrived.wait(5)) client.close() - self.assertEqual("trace-client-python/0.3.0 (MyGame)", self.capture.requests[0]["user_agent"]) + self.assertEqual("trace-client-python/0.4.0 (MyGame)", self.capture.requests[0]["user_agent"]) def test_report_ignores_a_blank_name(self): client = TraceClient(self.base_url, "MyGame", "1.2.3", key="k") @@ -247,6 +250,30 @@ def test_a_base_url_without_a_scheme_is_logged_not_a_dead_thread(self): % [r.getMessage() for r in self.log]) self.assertTrue(all(r.levelno == logging.DEBUG for r in self.log)) + def test_the_sender_thread_survives_an_unexpected_error_from_send(self): + crashes = [] + real_send = TraceClient._send + calls = [] + + def send_that_fails_once(client, body): + calls.append(body) + if len(calls) == 1: + raise RuntimeError("boom") + real_send(client, body) + + with mock.patch.object(threading, "excepthook", crashes.append), \ + mock.patch.object(TraceClient, "_send", send_that_fails_once): + client = TraceClient(self.base_url, "MyGame", "1.2.3", key="k") + client.report("first") + client.report("second") + self.assertTrue(self.capture.arrived.wait(5), "a later report is still delivered") + client.close() + self.assertEqual([], crashes, "the sender thread must not die with a traceback on stderr") + self.assertEqual(["second"], [json.loads(r["body"])["name"] for r in self.capture.requests]) + self.assertTrue(any("sender failed" in r.getMessage() and "boom" in r.getMessage() for r in self.log), + [r.getMessage() for r in self.log]) + self.assertTrue(all(r.levelno == logging.DEBUG for r in self.log)) + def test_constructor_rejects_a_missing_base_url_or_application(self): for base_url, application in ((None, "MyGame"), (" ", "MyGame"), ("http://x", None), ("http://x", "")): with self.assertRaises(ValueError): @@ -344,6 +371,127 @@ def test_close_is_prompt_and_idempotent(self): self.assertLess(time.monotonic() - before, TraceClient.TIMEOUT_SECONDS + 1) self.assertFalse(client.enabled) + # -- the per-installation ID ------------------------------------------ + + def _tmpdir(self): + directory = tempfile.mkdtemp(prefix="trace-install-") + self.addCleanup(shutil.rmtree, directory, True) + return directory + + def _sent_tags(self, client, name="startup", tags=None): + client.report(name, tags=tags) + self.assertTrue(self.capture.arrived.wait(5), "the report should reach the server") + client.close() + return json.loads(self.capture.requests[-1]["body"])["tags"] + + def test_no_install_id_and_no_file_sends_no_install_tag(self): + client = TraceClient(self.base_url, "MyGame", "1.2.3", key="k") + self.assertIsNone(client.install_id) + self.assertEqual({"version": "1.2.3"}, self._sent_tags(client)) + + def test_install_id_file_is_created_once_and_reused(self): + path = os.path.join(self._tmpdir(), "nested", "deeper", "install-id") + first = TraceClient(self.base_url, "MyGame", "1.2.3", key="k", install_id_file=path) + self.assertEqual(str(uuid.UUID(first.install_id)), first.install_id, "a random UUID") + with open(path, encoding="utf-8") as stored: + self.assertEqual(first.install_id + "\n", stored.read(), "parent directories are created") + first.close() + second = TraceClient(self.base_url, "MyGame", "1.2.3", key="k", install_id_file=path) + self.assertEqual(first.install_id, second.install_id, "the next run reuses it") + self.assertEqual({"version": "1.2.3", "install": first.install_id}, self._sent_tags(second)) + + def test_install_id_from_file_reads_the_first_valid_line(self): + path = os.path.join(self._tmpdir(), "install-id") + with open(path, "w", encoding="utf-8") as f: + f.write("\n# not an id\n my-own.id_1 \nsecond-id\n") + self.assertEqual("my-own.id_1", TraceClient.install_id_from_file(path)) + with open(path, encoding="utf-8") as f: + self.assertIn("# not an id", f.read(), "a file with an ID is never rewritten") + + def test_install_id_from_file_replaces_a_file_with_no_valid_line(self): + path = os.path.join(self._tmpdir(), "install-id") + with open(path, "w", encoding="utf-8") as f: + f.write("not an id\n" + "x" * (MAX_TAG_LENGTH + 1) + "\n") + made = TraceClient.install_id_from_file(path) + self.assertEqual(made, TraceClient.install_id_from_file(path)) + + def test_unwritable_install_id_file_yields_an_in_memory_id_and_never_raises(self): + # A path under a regular file cannot be created, even as root. + blocker = os.path.join(self._tmpdir(), "a-file") + with open(blocker, "w", encoding="utf-8") as f: + f.write("x") + path = os.path.join(blocker, "install-id") + client = TraceClient(self.base_url, "MyGame", "1.2.3", key="k", install_id_file=path) + self.assertEqual(str(uuid.UUID(client.install_id)), client.install_id) + self.assertFalse(os.path.exists(path)) + self.assertNotEqual(client.install_id, TraceClient.install_id_from_file(path), + "in memory: a new one each process") + self.assertEqual({"version": "1.2.3", "install": client.install_id}, self._sent_tags(client)) + + def test_unreadable_install_id_file_is_left_alone(self): + directory = self._tmpdir() # a directory cannot be read as a file + made = TraceClient.install_id_from_file(directory) + self.assertEqual(str(uuid.UUID(made)), made) + self.assertTrue(os.path.isdir(directory)) + self.assertEqual([], os.listdir(directory)) + + def test_install_id_from_file_never_raises_for_a_bad_path(self): + for path in (None, "", " ", 42): + made = TraceClient.install_id_from_file(path) + self.assertEqual(str(uuid.UUID(made)), made, repr(path)) + + def test_a_disabled_client_never_makes_up_or_writes_an_install_id(self): + directory = self._tmpdir() + path = os.path.join(directory, "install-id") + os.environ["DO_NOT_TRACK"] = "1" + by_environment = TraceClient(self.base_url, "MyGame", "1.2.3", key="k", install_id_file=path, + install_id="explicit") + del os.environ["DO_NOT_TRACK"] + by_config = TraceClient(self.base_url, "MyGame", "1.2.3", key="k", enabled=False, install_id_file=path) + by_no_key = TraceClient(self.base_url, "MyGame", "1.2.3", install_id_file=path) + for client in (by_environment, by_config, by_no_key): + self.assertIsNone(client.install_id, client.disabled_reason) + self.assertEqual([], os.listdir(directory), "nothing is written") + + def test_explicit_install_id_is_trimmed_sent_and_wins_over_the_file(self): + path = os.path.join(self._tmpdir(), "install-id") + client = TraceClient(self.base_url, "MyGame", "1.2.3", key="k", install_id=" abc-123 ", + install_id_file=path) + self.assertEqual("abc-123", client.install_id) + self.assertFalse(os.path.exists(path), "the file is not consulted when an ID is given") + self.assertEqual({"name": "home", "version": "1.2.3", "install": "abc-123"}, + self._sent_tags(client, "command", {"name": "home"})) + + def test_blank_install_id_is_none_and_an_overlong_one_is_rejected(self): + for blank in (None, "", " "): + self.assertIsNone(TraceClient(self.base_url, "MyGame", "1.2.3", key="k", install_id=blank).install_id) + with self.assertRaises(ValueError): + TraceClient(self.base_url, "MyGame", "1.2.3", key="k", install_id="x" * (MAX_TAG_LENGTH + 1)) + with self.assertRaises(ValueError, msg="rejected even on a disabled client, like the version"): + TraceClient(self.base_url, "MyGame", "1.2.3", enabled=False, install_id="x" * (MAX_TAG_LENGTH + 1)) + exact = TraceClient(self.base_url, "MyGame", "1.2.3", key="k", install_id="x" * MAX_TAG_LENGTH) + self.assertEqual("x" * MAX_TAG_LENGTH, exact.install_id) + exact.close() + + def test_an_events_own_install_tag_wins(self): + client = TraceClient(self.base_url, "MyGame", "1.2.3", key="k", install_id="mine") + tags = {"install": "theirs"} + self.assertEqual({"install": "theirs", "version": "1.2.3"}, self._sent_tags(client, tags=tags)) + self.assertEqual({"install": "theirs"}, tags, "the caller's tags are never modified") + + def test_install_tag_never_pushes_an_event_past_the_tag_cap(self): + client = TraceClient(self.base_url, "MyGame", "1.2.3", key="k", install_id="mine") + full = {"t%d" % i: "v" for i in range(MAX_TAGS - 1)} # plus version = MAX_TAGS + sent = self._sent_tags(client, tags=full) + self.assertEqual(MAX_TAGS, len(sent)) + self.assertNotIn("install", sent) + self.capture.arrived.clear() + client = TraceClient(self.base_url, "MyGame", "1.2.3", key="k", install_id="mine") + room = {"t%d" % i: "v" for i in range(MAX_TAGS - 2)} + sent = self._sent_tags(client, tags=room) + self.assertEqual(MAX_TAGS, len(sent)) + self.assertEqual("mine", sent["install"]) + if __name__ == "__main__": unittest.main() diff --git a/tests/test_usageReporting.py b/tests/test_usageReporting.py index bc843b3..4e5793c 100644 --- a/tests/test_usageReporting.py +++ b/tests/test_usageReporting.py @@ -174,6 +174,7 @@ def test_the_environment_wins_over_fishe_saying_on( assert client.disabled_reason == "environment" assert output.getvalue() == "" assert not os.path.exists(usageReporting.noticeMarkerPath(reportingOn)) + assert not os.path.exists(usageReporting.installIdPath(reportingOn)) assert not stub.waitFor(1, timeout=0.5) @@ -208,7 +209,8 @@ def test_the_notice_is_printed_once_and_then_never_again(reportingOn): def test_the_notice_names_the_program_what_is_sent_the_opt_outs_and_the_details(): assert usageReporting.NOTICE == ( "Usage reporting is on: FishE sends a startup event and a save-loaded " - "event (program name and version only) to trace.danielstephenson.dev. " + "event (program name, version and a random installation ID only) to " + "trace.danielstephenson.dev. " "Turn it off with FISHE_USAGE_REPORTING_ENABLED=false or " "TRACE_USAGE_REPORTING=off in the environment. " "Details: https://github.com/Stephenson-Software/trace#usage-reporting" @@ -262,7 +264,10 @@ def test_start_prints_the_notice_and_reports_startup_with_the_version( assert request["body"] == { "application": "FishE", "name": "startup", - "tags": {"version": usageReporting.readVersion()}, + "tags": { + "version": usageReporting.readVersion(), + "install": client.install_id, + }, } client.close() @@ -278,6 +283,7 @@ def test_start_says_nothing_and_sends_nothing_when_reporting_is_off( assert not client.enabled assert output.getvalue() == "" assert not os.path.exists(usageReporting.noticeMarkerPath(reportingOn)) + assert not os.path.exists(usageReporting.installIdPath(reportingOn)) assert not stub.waitFor(1, timeout=0.5) @@ -293,6 +299,40 @@ def test_the_second_start_reports_startup_again_but_stays_quiet(reportingOn, stu assert [r["body"]["name"] for r in stub.requests] == ["startup", "startup"] assert quiet.getvalue() == "" + # the same installation both times + assert first.install_id and second.install_id == first.install_id + assert {r["body"]["tags"]["install"] for r in stub.requests} == {first.install_id} + + +# -- the installation ID ------------------------------------------------------ + + +def test_the_installation_id_lives_in_the_save_directory(reportingOn): + client = usageReporting.createClient(reportingOn) + client.close() + + path = os.path.join(reportingOn.dataDirectory, "trace-install-id") + assert usageReporting.installIdPath(reportingOn) == path + assert open(path).read().strip() == client.install_id + + +def test_the_installation_id_does_not_read_as_a_save_slot(reportingOn): + from src.saveFileManager import SaveFileManager + + usageReporting.createClient(reportingOn).close() + + assert SaveFileManager(reportingOn.dataDirectory).list_save_files() == [] + assert SaveFileManager(reportingOn.dataDirectory).get_next_available_slot() == 1 + + +def test_trace_install_id_wins_over_the_file(monkeypatch, reportingOn): + monkeypatch.setenv("TRACE_INSTALL_ID", "pinned-id") + + client = usageReporting.createClient(reportingOn) + client.close() + + assert client.install_id == "pinned-id" + assert not os.path.exists(usageReporting.installIdPath(reportingOn)) # -- wiring into the game ----------------------------------------------------- @@ -333,9 +373,11 @@ def test_the_game_reports_startup_then_save_loaded(reportingOn, stub, capsys): game.play() version = usageReporting.readVersion() + install = open(usageReporting.installIdPath(reportingOn)).read().strip() + tags = {"version": version, "install": install} assert [r["body"] for r in stub.requests] == [ - {"application": "FishE", "name": "startup", "tags": {"version": version}}, - {"application": "FishE", "name": "save-loaded", "tags": {"version": version}}, + {"application": "FishE", "name": "startup", "tags": tags}, + {"application": "FishE", "name": "save-loaded", "tags": tags}, ] assert capsys.readouterr().out == usageReporting.NOTICE + "\n" assert not game.usageReporting.enabled # closed by play() @@ -391,4 +433,5 @@ def test_nothing_identifying_is_sent(reportingOn, stub): for request in stub.requests: assert set(request["body"]) <= {"application", "name", "tags"} - assert set(request["body"]["tags"]) == {"version"} + # the installation ID is random, not derived from anything + assert set(request["body"]["tags"]) == {"version", "install"}