From bad6ea0e75613181153444f4d5e16eb7df722a18 Mon Sep 17 00:00:00 2001 From: Josh Taillon Date: Tue, 1 Sep 2026 18:40:07 -0600 Subject: [PATCH] Configure a local trace exporter, without overriding one already set `enable_trajectory_tracing(log=True)` turns on GenAI content capture and, when nobody has installed a tracer provider, installs one that writes spans to a local file. It reports whether commons will emit conversation spans into a configured provider. Where a provider the caller installed sends them is the caller's to know, so commons does not inspect it. Two things do not carry over from R, both checked against opentelemetry-exporter-otlp-json-file rather than assumed. The Python file exporter appends to one path: no `%N` template, no `trace-latest.jsonl` hardlink, and it ignores `OTEL_EXPORTER_OTLP_TRACES_FILE`. So commons picks the filename, claiming it with an exclusive create, and sets the variable after the provider is installed, because a reader uses it to find the directory and must not be sent to a file nothing writes to. And nothing in Python reads `OTEL_TRACES_EXPORTER` outside auto-instrumentation, so respecting an explicit exporter means checking that variable and the installed provider by hand. A trace file holds whole conversations, so the default directory is created per process, readable only by this user, and the files are `0600`. Leaving them at a predictable path under the shared temporary directory would let other local users read them. `COMMONS_TRACES_DIR` still points somewhere durable, and is what another process needs to read them back. An installed no-op provider counts as configured, not as absent, so commons leaves it alone. `set_tracer_provider` refuses to replace it anyway, and treating it as absent would claim a file and repoint the variable while the provider silently stayed no-op. Installation holds a lock, reads both the exporter variable and the provider under it, and confirms the provider it built became the global one, so a caller configuring tracing from another thread is not overridden and cannot leave the variable naming an empty file. The default directory has its own lock, because installation resolves the directory while holding the setup one. Which files a reader picks up is shared with the R package, since one writes them and the other reads them, and a name outside the set fails silently: the reader finds nothing. `tests/shared/traces.json` pins it, R checks `local_traces_pattern()` against the cases, and Python checks that every name it chooses satisfies them. The rest of the trace contract, the span names and attributes, belongs in that file too and arrives with the code that emits them (kata 4frb). Spans are written as each one ends rather than batched, because a batch is lost when the process dies and the last turn of a conversation is the worst span to lose. Part of kata bvcv (M7). --- pkg-py/src/commons/_tracing.py | 210 ++++++++- pkg-py/tests/test_tracing.py | 440 +++++++++++++++++- .../testthat/fixtures/shared/traces.json | 52 +++ pkg-r/tests/testthat/test-trajectories.R | 16 + tests/shared/traces.json | 52 +++ 5 files changed, 767 insertions(+), 3 deletions(-) create mode 100644 pkg-r/tests/testthat/fixtures/shared/traces.json create mode 100644 tests/shared/traces.json diff --git a/pkg-py/src/commons/_tracing.py b/pkg-py/src/commons/_tracing.py index 292c45dd..773d36bf 100644 --- a/pkg-py/src/commons/_tracing.py +++ b/pkg-py/src/commons/_tracing.py @@ -11,14 +11,52 @@ from __future__ import annotations +import os +import re +import tempfile +import threading +import warnings from collections.abc import Iterator, Mapping from contextlib import contextmanager +from pathlib import Path from typing import Any from opentelemetry import trace from opentelemetry.trace import Span -__all__ = ["TRACER_NAME", "Span", "commons_span"] +__all__ = [ + "CAPTURE_VAR", + "EXPORTER_VAR", + "TRACER_NAME", + "TRACES_DIR_VAR", + "TRACES_FILE_VAR", + "Span", + "commons_span", + "commons_traces_dir", + "enable_content_capture", + "enable_trajectory_tracing", + "next_trace_file", + "provider_unset", + "tracing_configured", +] + +# Where local trace files go, and where a reader looks for them. +TRACES_DIR_VAR = "COMMONS_TRACES_DIR" +TRACES_FILE_VAR = "OTEL_EXPORTER_OTLP_TRACES_FILE" +# Whether the caller has configured an exporter of their own. +EXPORTER_VAR = "OTEL_TRACES_EXPORTER" +# Whether chat spans carry the messages. Without it a trajectory reads empty. +CAPTURE_VAR = "OTEL_INSTRUMENTATION_GENAI_CAPTURE_MESSAGE_CONTENT" + +_NUMBERED_TRACE_FILE = re.compile(r"trace-(\d+)\.jsonl") + +# Guards the read-then-install of the global tracer provider. +_SETUP_LOCK = threading.Lock() +# Guards the private default directory, created once per process. Separate +# from _SETUP_LOCK because installation resolves the directory while holding +# that one. +_DEFAULT_DIR_LOCK = threading.Lock() +_DEFAULT_TRACES_DIR: Path | None = None # Identifies commons as the emitter, alongside chatlas' own spans. R uses # `co.posit.r-package.commons`. @@ -42,3 +80,173 @@ def commons_span( """ with _TRACER.start_as_current_span(name, attributes=attributes) as span: yield span + + +def enable_trajectory_tracing(*, log: bool) -> bool: + """Prepare the process to record trajectories, and say whether to stamp them. + + Returns ``False`` when there is nowhere for conversation spans to go, + either because the caller did not ask or because no provider could be + configured. ``True`` means commons will emit spans into a configured + provider. When that provider is the caller's own, where the spans end up + is theirs to know: commons does not inspect it. + """ + if not log: + return False + + enable_content_capture() + if provider_unset(): + enable_local_tracing() + if not tracing_configured(): + warnings.warn( + "Trajectory logging is enabled but OpenTelemetry tracing is not " + "active. Configure an exporter before the process starts, or let " + f"commons configure one by leaving {EXPORTER_VAR} unset and " + "installing no tracer provider of your own.", + stacklevel=2, + ) + return False + return True + + +def tracing_configured() -> bool: + """Is there a tracer provider that does something with spans? + + Says nothing about whether that provider exports them anywhere. A provider + with no span processor, or one sampling nothing, records and discards, and + only its owner can know that. + """ + provider = trace.get_tracer_provider() + return not isinstance( + provider, trace.ProxyTracerProvider | trace.NoOpTracerProvider + ) + + +def provider_unset() -> bool: + """Has nobody installed a tracer provider yet? + + Distinct from `tracing_enabled()`: an explicitly installed no-op provider + means tracing is off, but it is a decision, and `set_tracer_provider()` + refuses to replace it anyway. + """ + return isinstance(trace.get_tracer_provider(), trace.ProxyTracerProvider) + + +def enable_content_capture() -> bool: + """Ask the GenAI instrumentation to record message content. + + An explicitly configured value is respected either way. A deliberate + opt-out must not be flipped process-wide, but it is worth saying out loud, + because the trajectory it produces looks like a failed capture. + """ + configured = os.environ.get(CAPTURE_VAR, "") + if configured: + if configured.lower() not in ("true", "1"): + warnings.warn( + f"{CAPTURE_VAR} is set to {configured!r}, so logged " + "trajectories will not include message content. Unset it or " + 'set it to "true" to capture full trajectories.', + stacklevel=2, + ) + return False + os.environ[CAPTURE_VAR] = "true" + return True + + +def enable_local_tracing() -> bool: + """Write spans to a local file, for a session that configured nothing. + + Only steps in when no exporter is configured at all. An explicit + ``OTEL_TRACES_EXPORTER``, even ``"none"``, is a decision to respect. + """ + # Checked again under the lock, which is the authoritative read. This one + # only avoids asking for the `tracing` extra when there was never anything + # for commons to configure. + if os.environ.get(EXPORTER_VAR) or not provider_unset(): + return False + + try: + from opentelemetry.exporter.otlp.json.file import FileSpanExporter + from opentelemetry.sdk.trace import TracerProvider + from opentelemetry.sdk.trace.export import SimpleSpanProcessor + except ImportError: + warnings.warn( + "Local trajectory logging needs the OpenTelemetry SDK. Install " + 'commons with the "tracing" extra to enable it.', + stacklevel=2, + ) + return False + + with _SETUP_LOCK: + # Both checks belong inside the lock. Reading them outside leaves a + # window in which a caller configures an exporter that commons then + # overrides. + if os.environ.get(EXPORTER_VAR) or not provider_unset(): + return False + + path = next_trace_file(commons_traces_dir()) + provider = TracerProvider() + # Written as each span ends rather than batched: a batch is lost if + # the process dies, and losing the last turn of a conversation is the + # worst span to lose. Appending a line to a local file is cheap. + provider.add_span_processor(SimpleSpanProcessor(FileSpanExporter(str(path)))) + trace.set_tracer_provider(provider) + + if trace.get_tracer_provider() is not provider: + # Someone installed one from outside this lock. Their spans go + # elsewhere, so leave no claimed file and no variable pointing at + # a file nothing writes to. + provider.shutdown() + path.unlink(missing_ok=True) + return False + + # The file exporter does not read this, but a reader uses it to find + # the directory, so it must name the file the spans actually go to. + os.environ[TRACES_FILE_VAR] = str(path) + return True + + +def commons_traces_dir() -> Path: + """Where commons writes trace files when it configures the exporter. + + The default is a directory readable only by this user, created once per + process, because a trace file holds whole conversations and the shared + temporary directory is not somewhere to leave those. Set + ``COMMONS_TRACES_DIR`` to keep them somewhere durable, and to let another + process read them back. + """ + global _DEFAULT_TRACES_DIR + + configured = os.environ.get(TRACES_DIR_VAR, "") + if configured: + return Path(configured) + with _DEFAULT_DIR_LOCK: + if _DEFAULT_TRACES_DIR is None: + _DEFAULT_TRACES_DIR = Path(tempfile.mkdtemp(prefix="commons-traces-")) + return _DEFAULT_TRACES_DIR + + +def next_trace_file(directory: Path) -> Path: + """Claim an unused numbered trace file in `directory`. + + The Python file exporter appends to one path and rotates nothing, so + commons picks the name. Claiming it rather than only choosing it keeps two + processes starting at once from interleaving their spans into one file. + """ + directory.mkdir(parents=True, exist_ok=True) + numbers = [ + int(match.group(1)) + for entry in directory.iterdir() + if (match := _NUMBERED_TRACE_FILE.fullmatch(entry.name)) + ] + # Gaps are left alone. Filling one would append to a run a reader has + # already taken. + number = max(numbers, default=0) + 1 + while True: + path = directory / f"trace-{number}.jsonl" + try: + os.close(os.open(path, os.O_CREAT | os.O_EXCL | os.O_WRONLY, 0o600)) + except FileExistsError: + number += 1 + else: + return path diff --git a/pkg-py/tests/test_tracing.py b/pkg-py/tests/test_tracing.py index 17438d80..bf5f2d16 100644 --- a/pkg-py/tests/test_tracing.py +++ b/pkg-py/tests/test_tracing.py @@ -8,10 +8,16 @@ from __future__ import annotations +import os +import re +import shutil +import stat import subprocess import sys import textwrap from collections.abc import Iterator +from pathlib import Path +from typing import Any import pytest @@ -26,7 +32,22 @@ InMemorySpanExporter, ) -from commons._tracing import TRACER_NAME, commons_span +from commons._tracing import ( + CAPTURE_VAR, + EXPORTER_VAR, + TRACER_NAME, + TRACES_DIR_VAR, + commons_span, + commons_traces_dir, + enable_content_capture, + next_trace_file, + tracing_configured, +) + +from ._shared import load_shared_fixture + +FILE_NAMING = load_shared_fixture("traces")["file_naming"] +READ_PATTERN = FILE_NAMING["read_pattern"] # The global tracer provider can only be set once per process, so one provider # serves the whole module and each test clears the exporter instead. @@ -127,13 +148,15 @@ def test_the_span_ends_and_records_the_error_when_the_body_raises() -> None: assert [event.name for event in span.events] == ["exception"] -def run_in_fresh_interpreter(body: str) -> str: +def run_in_fresh_interpreter(body: str, **env: str) -> str: """Run `body` in a new process, where commons has configured nothing.""" result = subprocess.run( [sys.executable, "-c", textwrap.dedent(body)], capture_output=True, text=True, check=False, + env={**os.environ, **env}, + timeout=60, ) assert result.returncode == 0, result.stderr assert result.stderr == "" @@ -153,3 +176,416 @@ def test_spans_are_inert_when_no_tracer_provider_is_configured() -> None: """ ) assert output.strip() == "False" + + +def test_the_shared_fixture_covers_both_read_outcomes() -> None: + assert {case["read"] for case in FILE_NAMING["cases"]} == {True, False} + + +@pytest.mark.parametrize("case", FILE_NAMING["cases"], ids=lambda case: case["name"]) +def test_trace_file_names_match_the_shared_fixture(case: dict[str, Any]) -> None: + read = re.fullmatch(READ_PATTERN, case["file"]) is not None + + assert read is case["read"] + + +def test_the_traces_directory_comes_from_the_environment( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + monkeypatch.setenv(TRACES_DIR_VAR, str(tmp_path)) + + assert commons_traces_dir() == tmp_path + + +def test_the_default_traces_directory_is_private_to_this_user( + monkeypatch: pytest.MonkeyPatch, +) -> None: + # Trace files hold whole conversations. The default lands under the shared + # temporary directory, so it must not be readable by other local users. + monkeypatch.delenv(TRACES_DIR_VAR, raising=False) + + directory = commons_traces_dir() + + assert directory.name.startswith("commons-traces") + assert stat.S_IMODE(directory.stat().st_mode) == 0o700 + + +def test_the_default_traces_directory_is_stable_within_a_process( + monkeypatch: pytest.MonkeyPatch, +) -> None: + monkeypatch.delenv(TRACES_DIR_VAR, raising=False) + + assert commons_traces_dir() == commons_traces_dir() + + +def test_a_trace_file_is_not_readable_by_other_users(tmp_path: Path) -> None: + chosen = next_trace_file(tmp_path) + + assert stat.S_IMODE(chosen.stat().st_mode) == 0o600 + + +@pytest.mark.parametrize( + ("existing", "expected"), + [ + ([], "trace-1.jsonl"), + (["trace-1.jsonl"], "trace-2.jsonl"), + (["trace-1.jsonl", "trace-2.jsonl"], "trace-3.jsonl"), + # Gaps are left alone: filling one would append to a run that a reader + # has already taken. + (["trace-1.jsonl", "trace-5.jsonl"], "trace-6.jsonl"), + # Neither of these carries a number to count from. + (["trace-latest.jsonl"], "trace-1.jsonl"), + (["trace.jsonl"], "trace-1.jsonl"), + (["notes.txt"], "trace-1.jsonl"), + ], +) +def test_a_fresh_trace_file_follows_the_numbered_ones( + tmp_path: Path, existing: list[str], expected: str +) -> None: + for name in existing: + (tmp_path / name).touch() + + chosen = next_trace_file(tmp_path) + + assert chosen.name == expected + # A name outside the read pattern is not an error anywhere; the reader + # just finds nothing. + assert re.fullmatch(READ_PATTERN, chosen.name) + + +def test_a_fresh_trace_file_is_claimed_so_a_second_writer_moves_on( + tmp_path: Path, +) -> None: + # Two agents starting at once would otherwise pick the same name and + # interleave their spans into one file. + first = next_trace_file(tmp_path) + second = next_trace_file(tmp_path) + + assert first.name == "trace-1.jsonl" + assert second.name == "trace-2.jsonl" + + +def test_the_traces_directory_is_created_if_it_is_missing(tmp_path: Path) -> None: + directory = tmp_path / "does" / "not" / "exist" + + chosen = next_trace_file(directory) + + assert chosen.is_file() + + +def test_content_capture_is_turned_on_when_nothing_asked_otherwise( + monkeypatch: pytest.MonkeyPatch, +) -> None: + # Without this, spans carry no messages and a trajectory reads back empty. + monkeypatch.delenv(CAPTURE_VAR, raising=False) + + assert enable_content_capture() is True + assert os.environ[CAPTURE_VAR] == "true" + + +@pytest.mark.parametrize("configured", ["true", "TRUE", "1"]) +def test_an_explicit_opt_in_is_left_alone( + monkeypatch: pytest.MonkeyPatch, configured: str +) -> None: + monkeypatch.setenv(CAPTURE_VAR, configured) + + assert enable_content_capture() is False + assert os.environ[CAPTURE_VAR] == configured + + +def test_an_explicit_opt_out_is_respected_with_a_warning( + monkeypatch: pytest.MonkeyPatch, +) -> None: + # A deliberate opt-out must not be flipped process-wide, but the user + # should hear why their trajectories come back without content. + monkeypatch.setenv(CAPTURE_VAR, "false") + + with pytest.warns(UserWarning, match="will not include message content"): + assert enable_content_capture() is False + + assert os.environ[CAPTURE_VAR] == "false" + + +def test_tracing_is_reported_as_configured_once_a_provider_is_installed() -> None: + # The module fixture installed one, so this is the configured case. + assert tracing_configured() is True + + +def test_logging_off_configures_nothing() -> None: + output = run_in_fresh_interpreter( + """ + import os + + from commons._tracing import CAPTURE_VAR, enable_trajectory_tracing + + os.environ.pop(CAPTURE_VAR, None) + print(enable_trajectory_tracing(log=False)) + print(CAPTURE_VAR in os.environ) + """ + ) + assert output.split() == ["False", "False"] + + +def test_logging_on_writes_spans_where_a_reader_looks(tmp_path: Path) -> None: + output = run_in_fresh_interpreter( + """ + import os + + from commons._tracing import ( + TRACES_FILE_VAR, + commons_span, + enable_trajectory_tracing, + ) + + print(enable_trajectory_tracing(log=True)) + with commons_span("commons_agent_create"): + pass + print(os.environ[TRACES_FILE_VAR]) + """, + **{TRACES_DIR_VAR: str(tmp_path)}, + ) + enabled, traces_file = output.split() + + assert enabled == "True" + # R's reader locates the directory through this variable, so it has to be + # set even though the Python exporter does not read it. + assert traces_file == str(tmp_path / "trace-1.jsonl") + written = (tmp_path / "trace-1.jsonl").read_text() + assert "commons_agent_create" in written + + +def test_an_explicitly_configured_exporter_is_not_overridden(tmp_path: Path) -> None: + # Even "none": a deliberate opt-out is a configuration, not an omission. + output = run_in_fresh_interpreter( + """ + import warnings + + from commons._tracing import enable_trajectory_tracing + + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter("always") + print(enable_trajectory_tracing(log=True)) + print(any("not active" in str(w.message) for w in caught)) + """, + **{TRACES_DIR_VAR: str(tmp_path), EXPORTER_VAR: "none"}, + ) + enabled, warned = output.split() + + assert enabled == "False" + assert warned == "True" + assert list(tmp_path.iterdir()) == [] + + +def test_a_provider_the_caller_installed_is_left_alone(tmp_path: Path) -> None: + # commons reports that it will emit spans, not that they will be readable. + # Where a provider the caller installed sends them is the caller's to + # know: this one has no span processor at all, and commons stays out of + # the way rather than second-guessing it. + output = run_in_fresh_interpreter( + """ + from opentelemetry import trace + from opentelemetry.sdk.trace import TracerProvider + + from commons._tracing import TRACES_FILE_VAR, enable_trajectory_tracing + + import os + + mine = TracerProvider() + trace.set_tracer_provider(mine) + + print(enable_trajectory_tracing(log=True)) + print(trace.get_tracer_provider() is mine) + print(TRACES_FILE_VAR in os.environ) + """, + **{TRACES_DIR_VAR: str(tmp_path)}, + ) + + assert output.split() == ["True", "True", "False"] + assert list(tmp_path.iterdir()) == [] + + +def test_a_provider_the_caller_disabled_is_left_alone(tmp_path: Path) -> None: + # Installing a no-op provider is a decision, not an omission. Treating it + # as unconfigured leaves a claimed file behind and repoints the variable a + # reader follows, while `set_tracer_provider` quietly refuses the change. + output = run_in_fresh_interpreter( + """ + import os + import warnings + + from opentelemetry import trace + + from commons._tracing import TRACES_FILE_VAR, enable_trajectory_tracing + + trace.set_tracer_provider(trace.NoOpTracerProvider()) + + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter("always") + print(enable_trajectory_tracing(log=True)) + print(TRACES_FILE_VAR in os.environ) + print(any("not active" in str(w.message) for w in caught)) + """, + **{TRACES_DIR_VAR: str(tmp_path)}, + ) + + assert output.split() == ["False", "False", "True"] + assert list(tmp_path.iterdir()) == [] + + +def test_concurrent_setup_points_the_variable_at_the_file_that_gets_spans( + tmp_path: Path, +) -> None: + # Only one provider can win. A loser that still rewrote the variable would + # send a reader to its own empty file. + output = run_in_fresh_interpreter( + """ + import os + from concurrent.futures import ThreadPoolExecutor + + from commons._tracing import ( + TRACES_FILE_VAR, + commons_span, + enable_trajectory_tracing, + ) + + with ThreadPoolExecutor(max_workers=8) as pool: + results = list(pool.map( + lambda _: enable_trajectory_tracing(log=True), range(8) + )) + + with commons_span("commons_agent_create"): + pass + + print(all(results)) + print(os.environ[TRACES_FILE_VAR]) + """, + **{TRACES_DIR_VAR: str(tmp_path)}, + ) + all_enabled, traces_file = output.split() + + assert all_enabled == "True" + written = [path for path in tmp_path.iterdir() if path.stat().st_size > 0] + assert [path.name for path in written] == [Path(traces_file).name] + assert "commons_agent_create" in Path(traces_file).read_text() + + +def test_logging_on_works_with_no_trace_directory_configured() -> None: + # Every other case here points COMMONS_TRACES_DIR somewhere, which skips + # the default directory entirely. This is the path an ordinary caller + # takes. + output = run_in_fresh_interpreter( + """ + import os + + from commons._tracing import ( + TRACES_DIR_VAR, + TRACES_FILE_VAR, + commons_span, + enable_trajectory_tracing, + ) + + os.environ.pop(TRACES_DIR_VAR, None) + + print(enable_trajectory_tracing(log=True)) + with commons_span("commons_agent_create"): + pass + print(os.environ[TRACES_FILE_VAR]) + """ + ) + enabled, traces_file = output.split() + written = Path(traces_file) + + try: + assert enabled == "True" + assert written.name == "trace-1.jsonl" + assert "commons_agent_create" in written.read_text() + assert stat.S_IMODE(written.parent.stat().st_mode) == 0o700 + finally: + shutil.rmtree(written.parent, ignore_errors=True) + + +def test_an_exporter_configured_while_setup_waits_is_not_overridden( + tmp_path: Path, +) -> None: + # The window between reading OTEL_TRACES_EXPORTER and installing the + # provider. Held open deliberately by taking the setup lock first. + output = run_in_fresh_interpreter( + """ + import os + import threading + import warnings + + from opentelemetry import trace + + from commons import _tracing + from commons._tracing import EXPORTER_VAR, enable_trajectory_tracing + + # The blocked thread warns after this frame moves on, so the filter + # has to be process-wide rather than scoped. + warnings.simplefilter("ignore") + + result = [] + with _tracing._SETUP_LOCK: + waiting = threading.Thread( + target=lambda: result.append( + enable_trajectory_tracing(log=True) + ) + ) + waiting.start() + # The exporter check must happen after the lock is taken, so + # setting this now still has to be honoured. + while not any( + thread is waiting and thread.is_alive() + for thread in threading.enumerate() + ): + pass + os.environ[EXPORTER_VAR] = "none" + waiting.join() + + print(result == [False]) + print(_tracing.provider_unset()) + """, + **{TRACES_DIR_VAR: str(tmp_path)}, + ) + + assert output.split() == ["True", "True"] + assert list(tmp_path.iterdir()) == [] + + +def test_an_explicit_exporter_is_respected_without_asking_for_the_extra( + tmp_path: Path, +) -> None: + # An API-only install that also turned tracing off should hear nothing + # about the `tracing` extra: commons has nothing to install it for. + output = run_in_fresh_interpreter( + """ + import sys + import warnings + + + class BlockTracingExtra: + blocked = ( + "opentelemetry.sdk", + "opentelemetry.exporter", + ) + + def find_spec(self, name, path=None, target=None): + if name.startswith(self.blocked): + raise ImportError(f"no module named {name!r}") + return None + + + sys.meta_path.insert(0, BlockTracingExtra()) + + from commons._tracing import enable_trajectory_tracing + + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter("always") + print(enable_trajectory_tracing(log=True)) + print(any("tracing" in str(w.message) and "extra" in str(w.message) + for w in caught)) + """, + **{TRACES_DIR_VAR: str(tmp_path), EXPORTER_VAR: "none"}, + ) + + assert output.split() == ["False", "False"] diff --git a/pkg-r/tests/testthat/fixtures/shared/traces.json b/pkg-r/tests/testthat/fixtures/shared/traces.json new file mode 100644 index 00000000..4bc5860f --- /dev/null +++ b/pkg-r/tests/testthat/fixtures/shared/traces.json @@ -0,0 +1,52 @@ +{ + "description": "Trace capture contract shared by pkg-r and pkg-py. The source is tests/shared/traces.json; the copy under pkg-r/tests/testthat/fixtures/shared/ is generated by scripts/sync-shared-fixtures.sh. Edit the source and re-run that script. Only file naming is pinned so far; the span names and attributes belong here too (kata 4frb) and arrive with the code that emits them.", + "file_naming": { + "description": "Which files in a trace directory a reader picks up. One package writes them and the other reads them, so a name outside this set is not an error anywhere: the reader simply finds nothing.", + "directory_variable": "COMMONS_TRACES_DIR", + "file_variable": "OTEL_EXPORTER_OTLP_TRACES_FILE", + "read_pattern": "^trace(-[0-9]+)?[.]jsonl$", + "cases": [ + { + "name": "an unnumbered trace file is read", + "file": "trace.jsonl", + "read": true + }, + { + "name": "a numbered trace file is read", + "file": "trace-1.jsonl", + "read": true + }, + { + "name": "numbering is not limited to one digit", + "file": "trace-12.jsonl", + "read": true + }, + { + "name": "numbering may start at zero", + "file": "trace-0.jsonl", + "read": true + }, + { + "name": "the latest hardlink is skipped", + "file": "trace-latest.jsonl", + "read": false, + "why": "an exporter that rotates also links the newest file here, and reading both would count its spans twice" + }, + { + "name": "a suffixed copy is skipped", + "file": "trace-1.jsonl.bak", + "read": false + }, + { + "name": "another prefix is skipped", + "file": "traces-1.jsonl", + "read": false + }, + { + "name": "an unrelated file is skipped", + "file": "notes.txt", + "read": false + } + ] + } +} diff --git a/pkg-r/tests/testthat/test-trajectories.R b/pkg-r/tests/testthat/test-trajectories.R index fcca37b0..1e25f0cf 100644 --- a/pkg-r/tests/testthat/test-trajectories.R +++ b/pkg-r/tests/testthat/test-trajectories.R @@ -749,6 +749,22 @@ test_that("generic parts and empty messages are dropped", { expect_equal(turns[[1]]@text, "Kept.") }) +# Which files a reader picks up is shared with pkg-py, which writes them; see +# tests/shared/traces.json. +test_that("local_traces_pattern matches the shared file-naming cases", { + spec <- shared_fixture("traces")$file_naming + withr::local_envvar(OTEL_EXPORTER_OTLP_TRACES_FILE = NA) + pattern <- local_traces_pattern() + + expect_setequal( + vapply(spec$cases, function(case) case$read, logical(1)), + c(TRUE, FALSE) + ) + for (case in spec$cases) { + expect_equal(grepl(pattern, case$file), case$read, info = case$name) + } +}) + test_that("trajectory_read reads OTLP files from a directory", { path <- withr::local_tempdir() json <- test_turn_json() diff --git a/tests/shared/traces.json b/tests/shared/traces.json new file mode 100644 index 00000000..4bc5860f --- /dev/null +++ b/tests/shared/traces.json @@ -0,0 +1,52 @@ +{ + "description": "Trace capture contract shared by pkg-r and pkg-py. The source is tests/shared/traces.json; the copy under pkg-r/tests/testthat/fixtures/shared/ is generated by scripts/sync-shared-fixtures.sh. Edit the source and re-run that script. Only file naming is pinned so far; the span names and attributes belong here too (kata 4frb) and arrive with the code that emits them.", + "file_naming": { + "description": "Which files in a trace directory a reader picks up. One package writes them and the other reads them, so a name outside this set is not an error anywhere: the reader simply finds nothing.", + "directory_variable": "COMMONS_TRACES_DIR", + "file_variable": "OTEL_EXPORTER_OTLP_TRACES_FILE", + "read_pattern": "^trace(-[0-9]+)?[.]jsonl$", + "cases": [ + { + "name": "an unnumbered trace file is read", + "file": "trace.jsonl", + "read": true + }, + { + "name": "a numbered trace file is read", + "file": "trace-1.jsonl", + "read": true + }, + { + "name": "numbering is not limited to one digit", + "file": "trace-12.jsonl", + "read": true + }, + { + "name": "numbering may start at zero", + "file": "trace-0.jsonl", + "read": true + }, + { + "name": "the latest hardlink is skipped", + "file": "trace-latest.jsonl", + "read": false, + "why": "an exporter that rotates also links the newest file here, and reading both would count its spans twice" + }, + { + "name": "a suffixed copy is skipped", + "file": "trace-1.jsonl.bak", + "read": false + }, + { + "name": "another prefix is skipped", + "file": "traces-1.jsonl", + "read": false + }, + { + "name": "an unrelated file is skipped", + "file": "notes.txt", + "read": false + } + ] + } +}