diff --git a/CLAUDE.md b/CLAUDE.md index a3e0df08..8d8ed59d 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -95,10 +95,19 @@ lmnr datasets pull # Pull dataset ## Environment Variables ``` -LMNR_PROJECT_API_KEY # API key (can also pass to initialize()) -LMNR_BASE_URL # API base URL (default: https://api.lmnr.ai) +LMNR_PROJECT_API_KEY # API key (can also pass to initialize()) +LMNR_BASE_URL # API base URL (default: https://api.lmnr.ai) +LMNR_DISABLE_GIT_METADATA # truthy value disables git state collection at initialize() ``` +## Git metadata (`sdk/git_metadata.py`) + +- `Laminar.initialize()` auto-collects `git.commit` / `git.branch` / `git.dirty` (best-effort `git` subprocess with a 1.5s timeout; CI env-var fallback: `GITHUB_SHA`, `VERCEL_GIT_COMMIT_SHA`, etc.) and merges it into `__global_metadata` at the LOWEST precedence — `LMNR_TRACE_METADATA` and the `metadata=` arg both override git keys. Opt out via `disable_git_metadata=True` or `LMNR_DISABLE_GIT_METADATA`. Eval runs get the same keys in their run metadata via `_with_git_metadata` (composed with `_with_debugger_session_metadata` at the `client.evals.init` call site). +- Collection is cached for the process lifetime in `_collect_git_metadata_cached`; tests that vary cwd/env MUST call `reset_git_metadata_cache()` (the `tests/test_git_metadata.py` autouse fixture does). The disabled check runs OUTSIDE the cache, on every `collect_git_metadata()` call, so the `disable_git_metadata=True` opt-out recorded by `initialize()` (via `set_git_metadata_disabled`) also suppresses eval-run metadata collected later — Bugbot finding on PR #311. +- `tests/conftest.py` sets `LMNR_DISABLE_GIT_METADATA=true` at import time for the whole session (many tests re-run `Laminar.initialize()` themselves) — without it the SDK repo's own git state leaks onto test spans and breaks exact-metadata assertions (e.g. `test_ctx_prop_laminar_span_context`). `tests/test_git_metadata.py` deletes the env var in its autouse fixture and restores `__global_metadata` on teardown. +- Cross-language parity surface with `lmnr-ts/packages/lmnr/src/git-metadata.ts` — keep key names, CI env-var list, precedence, and the truthy set line-comparable. +- `git.dirty` uses `git status --porcelain --untracked-files=no` (tracked changes only); `git.branch` is omitted on a detached HEAD (`rev-parse --abbrev-ref HEAD` returning literal `HEAD`). + ## Instrumentation tests (VCR) - Tests under `tests/test_instrumentations/**` replay via VCR cassettes in `cassettes//`. Sensitive headers are filtered by the root `vcr_config` (`authorization`, `api-key`, `x-api-key`, `x-goog-api-key`). diff --git a/src/lmnr/sdk/evaluations.py b/src/lmnr/sdk/evaluations.py index 85b3b69a..9ff48c7c 100644 --- a/src/lmnr/sdk/evaluations.py +++ b/src/lmnr/sdk/evaluations.py @@ -19,6 +19,7 @@ from lmnr.sdk.client.synchronous.sync_client import LaminarClient from lmnr.sdk.datasets import EvaluationDataset, LaminarDataset from lmnr.sdk.eval_control import EVALUATION_INSTANCES, PREPARE_ONLY +from lmnr.sdk.git_metadata import collect_git_metadata from lmnr.sdk.laminar import Laminar as L from lmnr.sdk.log import get_default_logger from lmnr.sdk.types import ( @@ -113,6 +114,21 @@ def _with_debugger_session_metadata( return {**(metadata or {}), SESSION_METADATA_KEY: session_id} +def _with_git_metadata( + metadata: dict[str, Any] | None, +) -> dict[str, Any] | None: + """Stamp git state (`git.commit` / `git.branch` / `git.dirty`) into eval + run metadata, so the evaluation entity itself — not just its traces — + records which code produced it. Collection is best-effort and cached at + the process level (see `git_metadata.py`); explicit user metadata wins on + key collision. Returns the metadata unchanged when nothing was collected. + """ + git_metadata = collect_git_metadata() + if not git_metadata: + return metadata + return {**git_metadata, **(metadata or {})} + + def get_average_scores(results: list[EvaluationResultDatapoint]) -> dict[str, Numeric]: per_score_values = {} for result in results: @@ -348,7 +364,9 @@ async def _run(self) -> EvaluationRunResult: evaluation = await self.client.evals.init( name=self.name, group_name=self.group_name, - metadata=_with_debugger_session_metadata(self.metadata), + metadata=_with_git_metadata( + _with_debugger_session_metadata(self.metadata) + ), ) evaluation_id = evaluation.id project_id = evaluation.projectId diff --git a/src/lmnr/sdk/git_metadata.py b/src/lmnr/sdk/git_metadata.py new file mode 100644 index 00000000..34a54951 --- /dev/null +++ b/src/lmnr/sdk/git_metadata.py @@ -0,0 +1,128 @@ +"""Best-effort collection of git state for trace metadata. + +Collected once per process at `Laminar.initialize()` and merged into the +global trace metadata at the LOWEST precedence, so both `LMNR_TRACE_METADATA` +and the explicit `metadata=` init argument can override any of the keys. The +keys land on every span as `lmnr.association.properties.metadata.git.*` and +flow into `traces.metadata` server-side with no backend changes. + +Collection must NEVER crash or block initialization: every git subprocess is +wrapped in a broad try/except with a short timeout, and a gitless environment +(CI checkout without `.git`, git binary missing, sandboxed runtime) degrades +to well-known CI environment variables, then to nothing. + +Set `LMNR_DISABLE_GIT_METADATA` to a truthy value ("true", "1", "yes", "on") +to disable collection entirely. + +This is a cross-language parity surface with the TS SDK +`lmnr-ts/packages/lmnr/src/git-metadata.ts` — keep the two line-comparable. +""" + +import os +import subprocess +from functools import lru_cache + +from lmnr.sdk.debug.config import _is_truthy + +GIT_COMMIT_METADATA_KEY = "git.commit" +GIT_BRANCH_METADATA_KEY = "git.branch" +GIT_DIRTY_METADATA_KEY = "git.dirty" + +_GIT_TIMEOUT_SECONDS = 1.5 + +# (commit env var, branch env var) per CI/deploy platform, checked in order. +# Used only when git itself is unavailable — e.g. gitless CI checkouts. +_CI_ENV_VARS: list[tuple[str, str]] = [ + ("GITHUB_SHA", "GITHUB_REF_NAME"), + ("VERCEL_GIT_COMMIT_SHA", "VERCEL_GIT_COMMIT_REF"), + ("CI_COMMIT_SHA", "CI_COMMIT_REF_NAME"), + ("CIRCLE_SHA1", "CIRCLE_BRANCH"), + ("RENDER_GIT_COMMIT", "RENDER_GIT_BRANCH"), + ("RAILWAY_GIT_COMMIT_SHA", "RAILWAY_GIT_BRANCH"), +] + + +# Process-level opt-out recorded by `Laminar.initialize(disable_git_metadata= +# True)`. Kept here (not on the Laminar class) so EVERY collection point — +# global trace metadata AND eval run metadata — honors the same flag without +# re-plumbing the init argument through each call site. +_disabled: bool = False + + +def set_git_metadata_disabled(disabled: bool) -> None: + """Record the initialize()-time opt-out for later collection points.""" + global _disabled + _disabled = disabled + + +def _git_metadata_disabled() -> bool: + return _disabled or _is_truthy(os.environ.get("LMNR_DISABLE_GIT_METADATA")) + + +def _run_git(*args: str) -> str | None: + """Run a git command; return its stripped stdout, None on ANY failure.""" + try: + result = subprocess.run( + ["git", *args], + capture_output=True, + text=True, + timeout=_GIT_TIMEOUT_SECONDS, + ) + except Exception: + return None + if result.returncode != 0: + return None + return result.stdout.strip() + + +def collect_git_metadata() -> dict[str, str | bool]: + """Collect `git.commit` / `git.branch` / `git.dirty`, best-effort. + + Returns {} when collection is disabled — via the `disable_git_metadata` + argument to `Laminar.initialize()` or the LMNR_DISABLE_GIT_METADATA env + var. The disabled check runs on every call (NOT inside the cache) so an + opt-out recorded after a prior collection still applies. + + `git.branch` is omitted on a detached HEAD, and `git.dirty` counts only + tracked-file changes (untracked build artifacts should not flip it). + """ + if _git_metadata_disabled(): + return {} + return _collect_git_metadata_cached() + + +def reset_git_metadata_cache() -> None: + """Reset the process-level collection cache. Exposed for tests only.""" + _collect_git_metadata_cached.cache_clear() + + +@lru_cache(maxsize=1) +def _collect_git_metadata_cached() -> dict[str, str | bool]: + """The actual collection, cached for the process lifetime (git state is + fixed once the process is running; re-running subprocesses per + initialize()/evaluate() call would only add latency). Tests that vary cwd + or env must call `reset_git_metadata_cache()`. + """ + metadata: dict[str, str | bool] = {} + commit = _run_git("rev-parse", "HEAD") + if commit: + metadata[GIT_COMMIT_METADATA_KEY] = commit + branch = _run_git("rev-parse", "--abbrev-ref", "HEAD") + if branch and branch != "HEAD": + metadata[GIT_BRANCH_METADATA_KEY] = branch + status = _run_git("status", "--porcelain", "--untracked-files=no") + if status is not None: + metadata[GIT_DIRTY_METADATA_KEY] = bool(status) + return metadata + + # Not a git repo, no git binary, or an unborn HEAD — fall back to CI env + # vars (a gap in Braintrust's approach: gitless CI checkouts get nothing). + for commit_var, branch_var in _CI_ENV_VARS: + env_commit = os.environ.get(commit_var) + if env_commit: + metadata[GIT_COMMIT_METADATA_KEY] = env_commit + env_branch = os.environ.get(branch_var) + if env_branch: + metadata[GIT_BRANCH_METADATA_KEY] = env_branch + break + return metadata diff --git a/src/lmnr/sdk/laminar.py b/src/lmnr/sdk/laminar.py index 82321290..172c3e59 100644 --- a/src/lmnr/sdk/laminar.py +++ b/src/lmnr/sdk/laminar.py @@ -49,6 +49,10 @@ from lmnr.opentelemetry_lib.tracing.span import LaminarSpan from lmnr.opentelemetry_lib.tracing.tracer import get_tracer_with_context from lmnr.opentelemetry_lib.tracing.utils import set_association_props_in_context +from lmnr.sdk.git_metadata import ( + collect_git_metadata, + set_git_metadata_disabled, +) from lmnr.sdk.utils import ( from_env, get_otel_env_var, @@ -230,6 +234,7 @@ def initialize( session_recording_options: SessionRecordingOptions | None = None, force_http: bool = False, metadata: dict[str, AttributeValue] | None = None, + disable_git_metadata: bool = False, ): """Initialize Laminar context across the application. This method must be called before using any other Laminar methods or @@ -282,6 +287,11 @@ def initialize( Defaults to None (uses default masking behavior). force_http (bool, optional): If set to True, the HTTP OTEL exporter will be\ used instead of the gRPC OTEL exporter. Defaults to False. + disable_git_metadata (bool, optional): If set to True, git state\ + (`git.commit`, `git.branch`, `git.dirty`) is not collected into\ + the global trace metadata. Can also be disabled with the\ + LMNR_DISABLE_GIT_METADATA environment variable.\ + Defaults to False. Raises: ValueError: If project API key is not set """ @@ -347,7 +357,16 @@ def initialize( env_metadata = json.loads(env_metadata_str) except Exception: pass - cls.__global_metadata = {**env_metadata, **(metadata or {})} + # Git state merges at the LOWEST precedence so both LMNR_TRACE_METADATA + # and the explicit `metadata=` argument can override any git.* key. + # The opt-out is recorded process-wide so eval run metadata + # (`_with_git_metadata` in evaluations.py) honors it too. + set_git_metadata_disabled(disable_git_metadata) + cls.__global_metadata = { + **collect_git_metadata(), + **env_metadata, + **(metadata or {}), + } if not os.getenv("OTEL_ATTRIBUTE_COUNT_LIMIT"): # each message is at least 2 attributes: role and content, diff --git a/tests/conftest.py b/tests/conftest.py index 692ec143..2ab64ac0 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,3 +1,4 @@ +import os from typing import Generator import pytest from unittest.mock import patch @@ -14,6 +15,14 @@ pytest_plugins = ("pytest_asyncio",) +# Git collection is disabled for the WHOLE test session (not just the +# session-scoped initialize below): many tests re-run Laminar.initialize() +# themselves, and each such call would otherwise stamp the SDK repo's real +# git state (commit, branch, dirty flag) into the global metadata, breaking +# tests that assert on exact metadata contents. `tests/test_git_metadata.py` +# removes this env var in its fixtures to exercise the collection path. +os.environ["LMNR_DISABLE_GIT_METADATA"] = "true" + @pytest.fixture(scope="session") def span_exporter() -> SpanExporter: diff --git a/tests/test_git_metadata.py b/tests/test_git_metadata.py new file mode 100644 index 00000000..2b98c02b --- /dev/null +++ b/tests/test_git_metadata.py @@ -0,0 +1,255 @@ +import os +import subprocess + +import pytest +from opentelemetry.sdk.trace.export.in_memory_span_exporter import ( + InMemorySpanExporter, +) + +from lmnr.sdk.evaluations import _with_git_metadata +from lmnr.sdk.git_metadata import ( + GIT_BRANCH_METADATA_KEY, + GIT_COMMIT_METADATA_KEY, + GIT_DIRTY_METADATA_KEY, + collect_git_metadata, + reset_git_metadata_cache, + set_git_metadata_disabled, +) +from lmnr.sdk.laminar import Laminar + +METADATA_ATTR_PREFIX = "lmnr.association.properties.metadata." + + +@pytest.fixture(autouse=True) +def setup_and_teardown(monkeypatch): + """Reset Laminar state and the process-level git cache before each test. + + Re-enables git collection (conftest disables it session-wide via + LMNR_DISABLE_GIT_METADATA) and restores `__global_metadata` afterwards so + the git keys collected here never leak onto spans of tests that run after + this module and assert on exact metadata contents. + """ + monkeypatch.delenv("LMNR_DISABLE_GIT_METADATA", raising=False) + set_git_metadata_disabled(False) + + original_initialized = Laminar._Laminar__initialized + original_base_http_url = Laminar._Laminar__base_http_url + original_project_api_key = Laminar._Laminar__project_api_key + original_global_metadata = Laminar._Laminar__global_metadata + + Laminar._Laminar__initialized = False + Laminar._Laminar__base_http_url = None + Laminar._Laminar__project_api_key = None + reset_git_metadata_cache() + + yield + + Laminar._Laminar__initialized = original_initialized + Laminar._Laminar__base_http_url = original_base_http_url + Laminar._Laminar__project_api_key = original_project_api_key + Laminar._Laminar__global_metadata = original_global_metadata + set_git_metadata_disabled(False) + reset_git_metadata_cache() + + +def _git(cwd, *args): + subprocess.run( + ["git", *args], + cwd=cwd, + check=True, + capture_output=True, + env={ + **os.environ, + "GIT_AUTHOR_NAME": "t", + "GIT_AUTHOR_EMAIL": "t@t", + "GIT_COMMITTER_NAME": "t", + "GIT_COMMITTER_EMAIL": "t@t", + }, + ) + + +@pytest.fixture +def git_repo(tmp_path, monkeypatch): + """A fresh git repo with one commit on branch `main`, as the cwd.""" + _git(tmp_path, "init", "--initial-branch=main") + (tmp_path / "tracked.txt").write_text("v1") + _git(tmp_path, "add", "tracked.txt") + _git(tmp_path, "commit", "-m", "initial") + monkeypatch.chdir(tmp_path) + return tmp_path + + +@pytest.fixture +def no_git_dir(tmp_path, monkeypatch): + """A cwd outside any git repo (git walks up, so hide parents via env).""" + monkeypatch.chdir(tmp_path) + # GIT_CEILING_DIRECTORIES does not apply to cwd itself, but tmp_path is + # never a repo; it stops discovery from walking up into /tmp or /. + monkeypatch.setenv("GIT_CEILING_DIRECTORIES", str(tmp_path.parent)) + return tmp_path + + +def _head_sha(cwd) -> str: + return subprocess.run( + ["git", "rev-parse", "HEAD"], + cwd=cwd, + capture_output=True, + text=True, + ).stdout.strip() + + +def test_collects_commit_branch_and_clean_state(git_repo): + metadata = collect_git_metadata() + head = _head_sha(git_repo) + assert metadata[GIT_COMMIT_METADATA_KEY] == head + assert metadata[GIT_BRANCH_METADATA_KEY] == "main" + assert metadata[GIT_DIRTY_METADATA_KEY] is False + + +def test_dirty_flag_tracks_modified_files(git_repo): + (git_repo / "tracked.txt").write_text("v2") + metadata = collect_git_metadata() + assert metadata[GIT_DIRTY_METADATA_KEY] is True + + +def test_untracked_files_do_not_flip_dirty(git_repo): + (git_repo / "untracked.txt").write_text("scratch") + metadata = collect_git_metadata() + assert metadata[GIT_DIRTY_METADATA_KEY] is False + + +def test_detached_head_omits_branch(git_repo): + head = _head_sha(git_repo) + _git(git_repo, "checkout", "--detach", head) + metadata = collect_git_metadata() + assert metadata[GIT_COMMIT_METADATA_KEY] == head + assert GIT_BRANCH_METADATA_KEY not in metadata + + +def test_no_repo_and_no_ci_env_collects_nothing(no_git_dir, monkeypatch): + for var in ("GITHUB_SHA", "GITHUB_REF_NAME", "VERCEL_GIT_COMMIT_SHA"): + monkeypatch.delenv(var, raising=False) + assert collect_git_metadata() == {} + + +def test_ci_env_fallback_when_not_a_repo(no_git_dir, monkeypatch): + monkeypatch.setenv("GITHUB_SHA", "abc123") + monkeypatch.setenv("GITHUB_REF_NAME", "feature-x") + metadata = collect_git_metadata() + assert metadata[GIT_COMMIT_METADATA_KEY] == "abc123" + assert metadata[GIT_BRANCH_METADATA_KEY] == "feature-x" + assert GIT_DIRTY_METADATA_KEY not in metadata + + +def test_git_wins_over_ci_env(git_repo, monkeypatch): + monkeypatch.setenv("GITHUB_SHA", "not-the-real-sha") + metadata = collect_git_metadata() + assert metadata[GIT_COMMIT_METADATA_KEY] != "not-the-real-sha" + + +def test_env_opt_out_disables_collection(git_repo, monkeypatch): + monkeypatch.setenv("LMNR_DISABLE_GIT_METADATA", "true") + assert collect_git_metadata() == {} + + +def test_initialize_stamps_git_metadata_on_spans( + git_repo, span_exporter: InMemorySpanExporter +): + span_exporter.clear() + Laminar.initialize(project_api_key="test_key") + span = Laminar.start_span("test") + span.end() + + spans = span_exporter.get_finished_spans() + assert len(spans) == 1 + attributes = spans[0].attributes + assert attributes[METADATA_ATTR_PREFIX + GIT_COMMIT_METADATA_KEY] + assert attributes[METADATA_ATTR_PREFIX + GIT_BRANCH_METADATA_KEY] == "main" + assert attributes[METADATA_ATTR_PREFIX + GIT_DIRTY_METADATA_KEY] is False + + +def test_initialize_user_metadata_overrides_git( + git_repo, span_exporter: InMemorySpanExporter +): + span_exporter.clear() + Laminar.initialize( + project_api_key="test_key", + metadata={GIT_COMMIT_METADATA_KEY: "user-override"}, + ) + span = Laminar.start_span("test") + span.end() + + spans = span_exporter.get_finished_spans() + assert ( + spans[0].attributes[METADATA_ATTR_PREFIX + GIT_COMMIT_METADATA_KEY] + == "user-override" + ) + + +def test_initialize_env_trace_metadata_overrides_git( + git_repo, monkeypatch, span_exporter: InMemorySpanExporter +): + span_exporter.clear() + monkeypatch.setenv( + "LMNR_TRACE_METADATA", '{"git.commit": "env-override"}' + ) + Laminar.initialize(project_api_key="test_key") + span = Laminar.start_span("test") + span.end() + + spans = span_exporter.get_finished_spans() + assert ( + spans[0].attributes[METADATA_ATTR_PREFIX + GIT_COMMIT_METADATA_KEY] + == "env-override" + ) + + +def test_initialize_disable_git_metadata_param( + git_repo, span_exporter: InMemorySpanExporter +): + span_exporter.clear() + Laminar.initialize(project_api_key="test_key", disable_git_metadata=True) + span = Laminar.start_span("test") + span.end() + + spans = span_exporter.get_finished_spans() + attributes = spans[0].attributes + assert METADATA_ATTR_PREFIX + GIT_COMMIT_METADATA_KEY not in attributes + assert METADATA_ATTR_PREFIX + GIT_BRANCH_METADATA_KEY not in attributes + assert METADATA_ATTR_PREFIX + GIT_DIRTY_METADATA_KEY not in attributes + + +def test_with_git_metadata_stamps_eval_run_metadata(git_repo): + metadata = _with_git_metadata({"user": "value"}) + assert metadata["user"] == "value" + assert metadata[GIT_COMMIT_METADATA_KEY] + assert metadata[GIT_BRANCH_METADATA_KEY] == "main" + + +def test_with_git_metadata_user_metadata_wins(git_repo): + metadata = _with_git_metadata({GIT_COMMIT_METADATA_KEY: "user-override"}) + assert metadata[GIT_COMMIT_METADATA_KEY] == "user-override" + + +def test_with_git_metadata_no_git_returns_unchanged(no_git_dir, monkeypatch): + for var in ("GITHUB_SHA", "GITHUB_REF_NAME", "VERCEL_GIT_COMMIT_SHA"): + monkeypatch.delenv(var, raising=False) + assert _with_git_metadata(None) is None + assert _with_git_metadata({"a": 1}) == {"a": 1} + + +def test_initialize_opt_out_also_disables_eval_git_metadata(git_repo): + """The disable_git_metadata init param must apply to eval run metadata + too, not just global trace metadata (Bugbot finding on PR #311).""" + Laminar.initialize(project_api_key="test_key", disable_git_metadata=True) + assert _with_git_metadata(None) is None + assert _with_git_metadata({"a": 1}) == {"a": 1} + + +def test_opt_out_applies_even_after_prior_collection(git_repo): + """A collection cached BEFORE the opt-out was recorded must not resurface + after it: the disabled check runs per call, outside the lru_cache.""" + assert collect_git_metadata()[GIT_BRANCH_METADATA_KEY] == "main" + set_git_metadata_disabled(True) + assert collect_git_metadata() == {} + assert _with_git_metadata(None) is None