Add OpenTelemetry local observability - #72
Open
tony wants to merge 109 commits into
Open
Conversation
why: Under concurrent workload the 0.5s git timeout in the VCS resource-attribute probe fired, so `git branch --show-current` returned nothing and the `git name-rev` fallback resolved an arbitrary ref at HEAD — often a local-only `backup/*` branch that is not the checked-out branch and not on the remote. Telemetry source attribution then split between the real branch and an unpushable one, breaking source links. what: - Raise the one-time git probe timeout from 0.5s to 2.0s so the cheap branch lookup survives load and the deterministic path is used. - Skip throwaway local backup refs in the name-rev fallback so VCS attribution never points telemetry at an unpushed `backup/*` branch.
why: The metric->trace pivot relies on exemplars (trace_id + span_id on metric points recorded inside a sampled span). The MeterProvider relied on the SDK default exemplar filter; pinning it keeps the link working regardless of SDK-version default changes or an OTEL_METRICS_EXEMPLAR_FILTER env override. what: - Set MeterProvider(exemplar_filter=TraceBasedExemplarFilter()).
why: The LGTM coverage audit found three gaps. `agentgrep ui` (and the `--ui` overlay) reported as the `agentgrep-cli` service, burying TUI traces, metrics, and profiles under one-shot CLI runs. `find` and the TUI search had spans and logs but no work metrics, so they were invisible on metric dashboards while `search` and `grep` were not. what: - Report the interactive explorer as the `agentgrep-tui` service via a cheap argv check (`ui` subcommand or `--ui` overlay). - Emit `agentgrep.find.sources` / `agentgrep.find.results` on both the CLI engine path (iter_find_events) and the MCP path (run_find_query). - Emit `agentgrep.tui.search.results` for interactive searches.
why: scripts/lgtm/up.sh and the acceptance script tracked :latest, which silently pulled a months-old build. Pinning a known release keeps the dev and CI stacks reproducible. The container config label is bumped so an existing container is recreated (docker run) rather than restarted (docker start) — recreation also re-stages the single-file bind mounts cleanly under Rancher Desktop / WSL, where docker start reuses a stale empty mount folder. what: - Pin grafana/otel-lgtm:0.28.0 (override via AGENTGREP_LGTM_IMAGE) in scripts/lgtm/up.sh and scripts/otel_acceptance.py. - Bump the up.sh container config label to force recreation on change. note: 0.28.0 runs Prometheus 3.11.3 with --web.enable-otlp-receiver and --enable-feature=exemplar-storage, so it ingests the OTLP exemplars the app emits (trace-based filter) and the metric->trace pivot works in Grafana. A host process already listening on :9090 (e.g. a Debian-packaged system Prometheus) shadows the container's published port under Rancher Desktop / WSL, so confirm exemplars in Grafana or via docker exec, not a bare host curl to :9090.
why: The OTel signals agentgrep exports to the local LGTM stack had no curated way to view them. A provisioned dashboard suite turns the raw metric surface into a usable DX and agentic-debugging view that is always present when the stack starts. what: - Add scripts/lgtm/generate_dashboards.py: a deterministic generator that renders six dashboards (Overview RED, Search Engine, Find & Grep, MCP Server, Surfaces & Versions, Agentic Debug Session) from the generic span instruments plus the domain histograms and counters. Latency panels enable exemplars for the metric->trace pivot. - Add the grafana-dashboards-agentgrep.yaml provider and mount it plus the generated folder in up.sh, alongside a startup regeneration step so a fresh checkout always has the boards. Bump the container config label to force recreation with the new mounts. - Suppress E501 for the generator: its panels embed PromQL as f-strings that read better unwrapped, matching the scripts/benchmark.py precedent.
why: agentgrep's exported log bodies are plain text — the structured attributes ride in Loki structured metadata, not the line. The agentic dashboard's logs panel piped the stream through `| json`, which errors on the non-JSON body. The bare stream selector still surfaces every attribute and keeps the trace_id derived-field link to Tempo. what: - Render the agentic Telemetry logs panel with a bare stream selector.
why: The benchmark surface ran as a PEP 723 script in an isolated uv env
where the agentgrep dist is not installed, so
importlib.metadata.version("agentgrep") raised and service.version fell
back to "0+unknown" — a source-attribution gap that violates the
package-version-only rule, while cli/tui/profile-engine (installed dist)
reported the real version.
what:
- package_version() takes an optional repo_root and falls back to the
static [project].version in pyproject.toml (via tomllib) before
"0+unknown"; setup() threads repo_root through.
- Add a unit test covering the dist-missing fallback and the repo_root
None / missing-file degradation to "0+unknown".
why: shutdown() called pyroscope.shutdown() unconditionally. In passive local mode (a git checkout with AGENTGREP_OTEL unset) the backend never calls pyroscope.configure(), so the teardown found no agent to drop and logged "Pyroscope Agent shutdown failed" to stderr via the SDK last-resort handler — user-visible noise on every --help and other early-exit path. what: - Guard the Pyroscope teardown with the existing profiles_started flag, mirroring the start-side guard so teardown matches startup. - Add a unit test exercising the real shutdown path (profiles_started False makes no pyroscope.shutdown call; True does).
why: The trace-to-profile pivot resolves only to a service, never to a single span, and that is worth recording so it is not mistaken for a configuration gap to chase. what: - Document that pyroscope-otel tags only root spans with pyroscope.profile.id and that span id is a per-sample profile attribute, not a queryable Pyroscope series label, so Grafana pivots at service granularity by design.
why: At the 100 Hz default a sub-second one-shot CLI run yields only a handful of CPU samples — too coarse to read — so short-lived profiles were effectively empty while only long-lived TUI/benchmark runs profiled usefully. what: - Raise the Pyroscope sample rate to 997 Hz (prime near 1 kHz) in the explicit profile modes, so a short run captures tens of samples and resolves real call graphs. gil_only stays on; native GIL-free CPU is still excluded by design.
…lity why: One-shot CLI/benchmark/profile-engine processes each export a fresh cumulative metric stream that plateaus at its in-process total and then dies, so rate()/increase() read 0 and instant queries drop the series — only the long-lived TUI showed live throughput. The series labelset is stable across processes, so per-process deltas can accumulate on one stream in the long-lived collector. what: - Export DELTA temporality for counters and histograms in live mode (OTLPMetricExporter preferred_temporality), leaving passive-local, debug, and debug-console cumulative. - Add a repo-owned collector config (scripts/lgtm/otelcol-config.yaml) with a deltatocumulative processor on the metrics pipeline, mounted by up.sh and the acceptance stack; bump both config labels so the container recreates. Exemplars survive the delta->cumulative conversion. - Bring scripts/otel_acceptance.py in line (mount + constant) and correct its stale exemplar note; fix the acceptance test's image assertion.
why: The grep-invert metrics attached the per-call candidate/emitted counts and the duration as metric LABELS. Those are unbounded per-call values, so every grep -v invocation minted a unique single-sample series and rate()/increase() could never aggregate — leaving the Find & Grep dashboard's grep duration, selectivity, and funnel panels empty. what: - Split a low-cardinality metric_attributes set (surface, operation, command, scope, output_mode, invert flags, outcome) from the full span/log attributes, and record the three grep metrics with the low-cardinality set only. The per-call values still ride the span and the structured log.
why: The Overview error-rate stat went blank instead of 0% when there were no errors (an empty rate() numerator yields no series), and the Search board's Scope template variable queried a non-existent `scope` label instead of `agentgrep_scope`, so it never offered values. what: - Wrap the error-rate numerator in `or vector(0)` so the stat renders 0% at zero errors. - Add a label_name override to the template-variable helper and point the Scope variable at the real agentgrep_scope label. - Regenerate the overview and search dashboards.
why: The rebase kept invert coverage but lost line-level selection and reintroduced an eager query import. what: - Select non-matching lines once for every grep renderer. - Preserve whole-line output for only-matching and vimgrep. - Keep CompiledQuery type-only and patch the active render seam.
why: Environment-root warnings exposed local filesystem values through structured log extras after the rebase. what: - Replace the raw override path with a redaction marker. - Retain only bounded length and availability status metadata.
why: Read-only store connections bypassed the branch tracer after the engine and reader split. what: - Resolve telemetry lazily at the shared reader seam. - Construct read-only connections with the traced factory.
why: The rebase moved input ownership to layouts while empty-q behavior and telemetry tests still targeted the old app shell. what: - Publish a typed quit request from empty search and filter inputs. - Handle tracing, logging, and exit at the active layout. - Exercise current DOM ownership and preserve ordinary q editing.
why: The rebase left list helpers and MCP-facing event streams without structured engine lifecycle telemetry. what: - Share bounded planning and terminal telemetry across search and find. - Cover list and event-stream entry points without logging content. - Close spans predictably when a stream consumer stops early.
why: The rebase severed the TUI service root and left layout-owned searches running through teardown. what: - Root each TUI session around construction and the blocking run loop. - Emit content-free lifecycle and shutdown telemetry. - Cooperatively cancel layout work during unmount.
why: Engine spans escaped into caller context across yields and abandoned streams could finalize under the wrong context. Counts and safe environment metadata also regressed during the rebase. what: - Bind full generator protocol and finalization to consumer context. - Restore bounded source/result metrics without double counting. - Preserve terminal outcomes and compatible redacted log metadata.
why: Inverted grep used highlight spans as truth, capped engine candidates before selection, and discarded record field filters. Blank, zero-width, multi-input, and pretty output diverged from ripgrep. what: - Preserve physical-line, EOF, overlap, and zero-width match semantics. - Limit selected records after inversion in eager and stream paths. - Retain safe field filters and reject unsafe boolean decompositions. - Reuse line matches across renderers and keep output shapes stable.
why: Pyroscope fetches .pyroscope.yaml from the profiled repository and exact ref. The generated ignored file was never mounted, fetched, or read, so it could not affect Function Details. what: - Remove the unused generator and its startup and acceptance calls. - Delete tests that only validated the disconnected local artifact. - Document the real committed-at-exact-ref source-link contract.
why: Passive checkout bootstrap performed network export on ordinary help, and raw exception, SQL, or third-party log payloads could escape. Provider lifecycle behavior also multiplied timeouts and produced orphan traces. what: - Require explicit opt-in and honor SDK and signal disable controls. - Export finite metadata and sample only complete app traces. - Preserve help and cancellation semantics and bound shutdown.
why: Middleware observes raw calls before FastMCP validates them. Untrusted identifiers, names, paths, refs, and unknown arguments could reach spans and audit logs. what: - Default-deny summaries and validate finite names and values. - Redact free-form values and omit untrusted context IDs. - Record cancellation as a clean, re-raised outcome.
why: The app-root sampler kept the parent sampling decision but dropped inbound W3C tracestate, breaking vendor correlation at the MCP root. what: - Propagate parent trace state in every sampling result. - Cover a sampled remote MCP parent carrying vendor state.
why: The custom sampler kept sampling decisions but discarded attributes supplied at span start, erasing auto-instrumentation metadata. what: - Return start-time attributes for every sampled span. - Cover attribute retention inside an approved app trace.
why: FastMCP emits caller-controlled names, identifiers, and raw exceptions through the global provider before project middleware can sanitize them. what: - Rebuild dependency spans as finite immutable export views. - Preserve topology while stripping content, IDs, and stack data. - Restrict Pyroscope to approved project app roots.
why: Asyncio auto-instrumentation exports raw coroutine names on metrics and can attach exception text to spans. Project wrappers already preserve context at owned async and thread boundaries. what: - Remove automatic AsyncioInstrumentor setup and lifecycle state. - Drop the unused extra and development dependency and refresh the lock. - Document explicit project spans as the privacy-safe boundary.
why: The complete Sphinx doctest now needs about a minute after the telemetry documentation landed, so the default 60-second cutoff kills a passing build. what: - Give only the just doctest evaluator the existing 120-second cold-build allowance.
why: master now enforces print, naming, and refurb rules. This branch introduced telemetry tooling before those ratchets landed. what: - Mark the dashboard and OTel evidence scripts as terminal-output owners. - Simplify Git suffix removal with str.removesuffix(). - Give the synthetic processor exception an Error suffix.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds a passive OpenTelemetry observability layer for local development while keeping packaged use quiet and best-effort. The implementation keeps SDK and exporter imports behind the lazy telemetry backend, adds optional OTel/Pyroscope dependencies, and instruments the CLI, MCP, TUI, engine, pytest, logs, metrics, and live LGTM smoke paths.
The branch also adds project-level SQLite shortcut tracing. The upstream SQLite DB-API instrumentor sees explicit cursor execution, but agentgrep reads SQLite stores through connection shortcut methods. Those methods now emit child spans under existing app roots without recording bound values, prompt text, file contents, or local database paths.
Validation
just otel-acceptance --timeout 300confirmed traces, metrics, logs, and profiles in LGTM, including bothagentgrep.cli.invocationandagentgrep.otel.smokemulti-span roots plusagentgrep.sqlite.executeandagentgrep.sqlite.executemanyspans.rm -rf docs/_build; uv run ruff check . --fix --show-fixes; uv run ruff format .; uv run ty check; uv run py.test --reruns 0 -vvv; just build-docs;completed successfully.Goal
Idempotent completion condition for
/goal. It asserts an end state, not an action: if the state already holds, the first evaluation passes and nothing is edited.git rev-parse --abbrev-ref HEAD; git diff master...HEAD | rg -c 'field_predicate|field_filter_safe|EmptyInputQuitRequested|cursor-ide-state-vscdb|def on_unmount' || echo 0; rg -c provisional docs/dev/adr/*opentelemetry-local-observability.md docs/dev/adr/*agentic-structured-logging.md; rg -c '^pytestmark' tests/test_telemetry.py tests/test_otel_acceptance_script.py; git -c rerere.enabled=false merge-tree --write-tree master HEAD | rg -c '^CONFLICT' || echo 0; NO_COLOR=1 AGENTGREP_TUI_WATCHDOG=1 env -u VIRTUAL_ENV -u UV_NO_SYNC .venv/bin/python -m pytest -m "" --reruns 0 -q && just build-docs