From 2e454dee7c5664e2fd915efbc7f56b969661fb13 Mon Sep 17 00:00:00 2001 From: kl3inIT Date: Wed, 29 Jul 2026 22:10:24 +0700 Subject: [PATCH 1/8] docs: open the observability pipeline and payload boundary increment Production has logged an OTLP metrics export failure every minute since 2026-07-25. The cause is a transitive default: the OpenTelemetry starter resolves micrometer-registry-otlp, whose export is opt-out and defaults to a localhost URL nothing listens on. OTLP tracing behaves oppositely and is not involved. Scoping that work surfaced a larger finding. A proposal to replace the flat payload-free rule with configurable tiers went through an independent architecture challenge, which rejected it and produced two verified bypasses of the boundary the runbook claims: Spring AI provider libraries concatenate the prompt into WARN logs that log-prompt=false does not reach, and the Micrometer OTel bridge records exception events and messages on error spans. Both are live paths carrying ACL-scoped evidence. The policy stays unchanged; the repository claims more than it enforces. Closing those bypasses is phase 0, ahead of the pipeline work. The challenge brief and verdict are kept with the increment because their working location is not versioned. Co-Authored-By: Claude Opus 5 (1M context) --- .../challenge-brief.md | 104 +++ .../challenge-verdict.md | 780 ++++++++++++++++++ .../design.md | 191 +++++ .../2026-07-29-observability-pipeline/plan.md | 100 +++ docs/roadmap.md | 1 + 5 files changed, 1176 insertions(+) create mode 100644 docs/increments/active/2026-07-29-observability-pipeline/challenge-brief.md create mode 100644 docs/increments/active/2026-07-29-observability-pipeline/challenge-verdict.md create mode 100644 docs/increments/active/2026-07-29-observability-pipeline/design.md create mode 100644 docs/increments/active/2026-07-29-observability-pipeline/plan.md diff --git a/docs/increments/active/2026-07-29-observability-pipeline/challenge-brief.md b/docs/increments/active/2026-07-29-observability-pipeline/challenge-brief.md new file mode 100644 index 00000000..38b80d84 --- /dev/null +++ b/docs/increments/active/2026-07-29-observability-pipeline/challenge-brief.md @@ -0,0 +1,104 @@ +# Architecture challenge: payload-free telemetry policy for OrgMemory + +You are an independent architecture reviewer. Your job is to attack the proposal below, +not to validate it. `CLAUDE.md` in this repo requires an independent challenge before +decisions about authorization, persistence, or publication boundaries are implemented — +this is that challenge. Verify claims against the code yourself; do not take the summary +on trust. File paths are given so you can check. + +## What OrgMemory is + +A governed organizational memory layer. Its product promise is permission-aware retrieval: +every chunk of evidence is scoped by an OpenFGA-backed ACL per Knowledge Asset, and +retrieval re-verifies the full evidence closure against the canonical ledger before +anything reaches a model or a user. Multi-tenant, enterprise-facing. + +## The rule under review + +`docs/runbooks/graph-rag-production-hardening.md`, section "Payload-Free Tracing": + +> The application allowlist is limited to operation and organization UUIDs, stage/outcome, +> monotonic duration, bounded input/output counts, an optional lowercase SHA-256 +> model-route fingerprint, and a bounded machine failure code. Never add query, prompt, +> completion, evidence/chunk text, document title/URI, embedding values, actor identity, +> ACL subjects or exception messages. Spring AI prompt and completion observation logging +> is explicitly disabled in API and worker configuration. + +Enforcement is structural, not by convention. See +`components/graph-rag-core/src/main/java/com/orgmemory/graphrag/observability/GraphRagEventSink.java` +— the `GraphRagEvent` record's compact constructor rejects anything outside the allowlist: +fingerprints must match `[0-9a-f]{64}`, `failureCode` must match `[a-z0-9_]{1,64}`, and +there is no field that can carry free text. The OpenTelemetry adapter beside it +(`integrations/graph-rag-observability/.../OpenTelemetryGraphRagEventSink.java`) can only +emit what the record allows. + +## Evidence gathered from comparable systems + +Read from source where marked, otherwise from vendor documentation. + +| System | Prompt/completion leaves the process? | Mechanism | +|---|---|---| +| Dify | Yes, by default | Full input/output history logged; community issue #19345 asks for opt-out docs | +| Onyx (source read at `tmp/onyx`) | Yes, once a provider is configured | `backend/onyx/tracing/framework/span_data.py` — `GenerationSpanData` carries `input`/`output`; `langfuse_tracing_processor.py:178` sends both. `enable_masking=True` by default but `masking.py` only redacts `private_key` / `authorization: bearer` and truncates at 500k — prompt text survives. Provider config per tenant in DB; env-only on multi-tenant cloud | +| Sentry MCP (source read at `tmp/upstream-sentry-mcp-20260726`) | Yes | `sendDefaultPii: true` explicitly set, above Sentry's own `false` default. `packages/mcp-core/src/telem/sentry.ts` adds a `beforeSend` hook with pattern-based `SCRUB_PATTERNS` (API keys, bearer tokens), recursive to depth 20, and logs a warning whenever it actually scrubs something | +| LibreChat | Via proxy | Backend validates the session and strips app auth headers before forwarding telemetry | +| OpenTelemetry GenAI semconv | Opt-in, default off | Single env var `OTEL_INSTRUMENTATION_GENAI_CAPTURE_MESSAGE_CONTENT` | +| Spring AI 2.0 | Opt-in, default off | `spring.ai.chat.observations.log-prompt` / `log-completion`, both `matchIfMissing = false` | +| RAGFlow | Yes, via Langfuse integration | Configured in API settings | +| **OrgMemory** | **Never** | Closed allowlist enforced by type | + +No comparable system chose "never". OrgMemory is alone at that end. + +## Observed operational cost + +Production emitted `Failed to publish metrics to OTLP receiver (url=http://localhost:4318/v1/metrics)` +every minute for four days without anyone diagnosing it. Root cause was that +`spring-boot-starter-opentelemetry` transitively pulls `micrometer-registry-otlp`, whose +`OtlpMetricsExportAutoConfiguration` is opt-out and defaults to a localhost URL that +nothing was listening on. Contributing factor: the team's habit is that telemetry does +not say anything useful, partly a product of the policy above. + +## The proposal + +Replace the flat "never" with tiers: + +- **Tier 0** — default, unchanged. Current allowlist, enforced by type. Add a payload-free + counter for how often a redactor had to block something, so silent near-misses become + visible (borrowed from Sentry's scrub-visibility idea). +- **Tier 1** — opt-in per deployment, default off. Exception class and message for + *infrastructure* exceptions, passed through a pattern-based scrubber; hashed actor id. +- **Tier 2** — opt-in per organization, time-boxed, consent recorded. Query and completion + text. Modelled on Onyx: per-tenant config in the database, reloaded without restart. +- **Never at any tier** — evidence/chunk text, document title/URI, ACL subjects, + embedding values. + +A reduced variant is also on the table: **Tier 0 + Tier 1 only**, keeping "never" for +query and completion. This takes the operational win without touching the governance +promise. + +## The strongest counterargument already identified + +An absolute rule is auditable in one line and cannot be misconfigured. Three tiers create +three states to audit, and one environment variable set wrong at Tier 2 leaks customer +content. For a product sold on governance, "cannot be wrong" has real value against +"is not wrong when configured correctly". The current design also enforces the rule in the +type system, which no amount of configuration discipline matches. + +## What to answer + +1. Is the operational-cost argument for Tier 1 actually sound, or is the four-day + undiagnosed warning better explained by something other than the payload policy? + Check whether infrastructure exception detail was genuinely unavailable, or merely + unused — `apps/api/src/main/resources/application-prod.yml` sets log levels, and the + warning did reach stdout. +2. Does the ACL-scoped-evidence distinction actually justify diverging from every + comparable system, or is that reasoning motivated? Attack it. +3. If tiering is adopted, what is the failure mode you would most expect in practice, and + what structural control (not process) prevents it? +4. Is there a fourth option neither variant covers — for example, keeping "never" for the + sink while allowing detail on a separate egress path with different controls? +5. Which variant would you ship: all three tiers, Tier 0+1, or unchanged? Commit to one. + +Answer in prose, be specific about file paths and code you checked, and state plainly +where you think the summary above is wrong or overstated. Disagreement is the useful +output here. diff --git a/docs/increments/active/2026-07-29-observability-pipeline/challenge-verdict.md b/docs/increments/active/2026-07-29-observability-pipeline/challenge-verdict.md new file mode 100644 index 00000000..71894ee3 --- /dev/null +++ b/docs/increments/active/2026-07-29-observability-pipeline/challenge-verdict.md @@ -0,0 +1,780 @@ +# Architecture Verdict: Payload-Free Telemetry Policy for OrgMemory + +Date: 2026-07-29 +Repository: `D:\OrgMemory` +Review basis: commit `86ae6bc02a369471f2b77c87523eb6902988d1ba` on branch `fix/graph-rag-observability-wiring` + +## Committed recommendation + +Ship the **unchanged payload policy**. + +Do not add the proposed Tier 1 exception-message and hashed-actor fields. Do not +add Tier 2 query/completion capture. Keep payload-bearing values structurally +outside `GraphRagEventSink`. + +“Unchanged” here means unchanged policy, not an assertion that the present +application is already proven payload-free. The custom GraphRAG event adapter is +narrow, but Spring AI observations, Micrometer error spans, provider library +WARN paths, global OpenTelemetry resource data, and ordinary application logs +sit outside that type boundary. Those paths must be audited and hardened before +the deployment is described as globally payload-free. + +For richer operational troubleshooting, use the fourth option described below: +a separate diagnostic path with its own data type, endpoint, credentials, +retention, access controls, and failure semantics. + +## Verification performed + +The verdict is based on direct inspection of the current repository, the +resolved dependency sources used by the build, the checked-in upstream +comparables under `tmp`, and focused tests. + +The focused tests passed: + +```text +.\gradlew.bat --no-daemon \ + :components:graph-rag-core:test \ + --tests "*GraphRagEventSinkTests" \ + :integrations:graph-rag-observability:test \ + --tests "*OpenTelemetryGraphRagEventSinkTests" +``` + +Gradle dependency inspection resolved: + +```text +org.springframework.boot:spring-boot-starter-opentelemetry:4.1.0 +io.micrometer:micrometer-registry-otlp:1.17.0 +io.micrometer:micrometer-tracing-bridge-otel:1.7.0 +org.springframework.ai:spring-ai-model:2.0.0 +org.springframework.ai:spring-ai-openai:2.0.0 +org.springframework.ai:spring-ai-anthropic:2.0.0 +``` + +Context7 was attempted for current framework documentation but its monthly +quota was exhausted. Framework behavior was therefore checked against the +actual resolved source JARs plus official Spring AI, Spring Boot, Micrometer, +OpenTelemetry, and Sentry documentation. + +## Claim-by-claim audit + +### 1. Permission-aware evidence closure + +**Challenge claim:** every contributing evidence chunk is authorization-scoped +and the complete evidence closure is re-verified against the canonical ledger +before anything reaches a model. + +**Verdict:** substantially verified for the GraphRAG retrieval path. + +Evidence: + +- `core/src/main/java/com/orgmemory/core/knowledge/GraphRagKnowledgeRetrievalService.java:271-331` + re-resolves the current scope, constructs the complete grounding closure, + checks its size, verifies it through OpenFGA, canonical-rechecks it, compares + the rechecked evidence identity, and only then calls the grounding renderer. +- `core/src/main/java/com/orgmemory/core/knowledge/GraphRagKnowledgeRetrievalService.java:666-726` + performs the exact OpenFGA `BatchCheck` and canonical recheck. +- `core/src/main/java/com/orgmemory/core/knowledge/GraphRagKnowledgeRetrievalService.java:810-836` + compares organization, asset, source revision, ACL snapshot, and projection + generation identities. +- `core/src/main/java/com/orgmemory/core/knowledge/SecureKnowledgeRetrievalStore.java:92-212` + applies organization, lifecycle, publication/model, sealed-ingestion ACL, + current ACL, deny/allow, mapped source user/group, and classification filters. +- `core/src/test/java/com/orgmemory/core/knowledge/GraphRagKnowledgeRetrievalServiceTests.java:214-492` + covers revocation, entity/relation/direct-chunk contributions, canonical + recheck, model mismatch, and prevention of model egress on failure. +- `docs/guidelines/agent-safety.md:11-14` states that generated summaries, + facts, and other derivatives inherit the permissions of every contributing + evidence item. + +The claim should still be scoped to this implemented retrieval path rather than +presented as a theorem about every possible future model invocation. + +### 2. The runbook accurately describes the `GraphRagEvent` schema + +**Challenge claim:** the application allowlist consists of operation and +organization UUIDs, stage/outcome, monotonic duration, bounded input/output +counts, an optional model-route fingerprint, and a bounded failure code. + +**Verdict:** incomplete and partially wrong. + +`components/graph-rag-core/src/main/java/com/orgmemory/graphrag/observability/GraphRagEventSink.java:63-75` +actually defines: + +```text +operationId +organizationId +stage +outcome +duration +inputCount +outputCount +modelRouteFingerprint +scopeFingerprint +cacheStatus +failureCode +occurredAt +``` + +The runbook at +`docs/runbooks/graph-rag-production-hardening.md:23-29` omits +`scopeFingerprint`, `cacheStatus`, and the explicit occurrence time. + +Specific corrections: + +- Counts are only checked for non-negativity at + `GraphRagEventSink.java:86-88`. There is no policy maximum, bucketing, or + stage-specific bound. Java `int` is a representation bound, not the + “bounded counts” policy described by the runbook. +- The record only rejects a negative `Duration` at + `GraphRagEventSink.java:82-85`. Current producers generally calculate + duration from `System.nanoTime()`, but the type itself cannot prove monotonic + provenance. +- `failureCode` is matched against `[a-z0-9_]{1,64}` at + `GraphRagEventSink.java:17-18,102-110`. This limits syntax and length, but it + is not a closed error taxonomy. Any conforming arbitrary token can be passed. +- Failed outcomes require a code, but successful or cancelled outcomes are not + forbidden from carrying one. +- Fingerprints are required to look like lowercase SHA-256 at + `GraphRagEventSink.java:89-100`, but the type cannot prove what was hashed. A + future producer could hash query text and create a stable equality oracle + while still satisfying the constructor. + +Current producer provenance appears intentional: + +- `apps/api/src/main/java/com/orgmemory/api/graphrag/GraphRagRuntimeConfiguration.java:78-82` + hashes the configured gateway/model route. +- `core/src/main/java/com/orgmemory/core/knowledge/GraphRagKnowledgeRetrievalService.java:621-629` + hashes organization and knowledge-space scope. +- `core/src/main/java/com/orgmemory/core/knowledge/GraphRagKnowledgeRetrievalService.java:769-771` + hashes the rerank provider. + +That is a useful current convention, but it is not enforced by the event type. + +### 3. The custom record and adapter structurally exclude direct payload text + +**Challenge claim:** the compact constructor rejects anything outside the +allowlist, there is no field for free text, and the OpenTelemetry adapter can +only emit what the record permits. + +**Verdict:** verified for direct fields on this one custom adapter, overstated as +an application-wide guarantee. + +The custom path is narrow: + +- `components/graph-rag-core/src/main/java/com/orgmemory/graphrag/observability/GraphRagEventSink.java:63-112` + has no query, prompt, completion, evidence, title, URI, actor, ACL subject, or + exception-message field. +- `integrations/graph-rag-observability/src/main/java/com/orgmemory/integrations/graphrag/observability/OpenTelemetryGraphRagEventSink.java:23-45` + declares eleven explicit attribute keys. +- `OpenTelemetryGraphRagEventSink.java:54-91` emits only those event-derived + attributes, span name, status, and timing. Failed custom events set ERROR + status without calling `recordException`. +- `integrations/graph-rag-observability/src/test/java/com/orgmemory/integrations/graphrag/observability/OpenTelemetryGraphRagEventSinkTests.java:22-80` + verifies the exact eleven-key attribute set for one manually constructed + custom span. + +The absolute claim is nevertheless too strong: + +- `failureCode` remains an open string constrained only by character class and + length. +- The OpenTelemetry SDK adds span identity, parent/trace context, resource + attributes, instrumentation scope, timestamps, and status outside the record. +- Span processors and other instrumentation share the exporter and can add or + export unrelated data. +- The custom test does not inspect resource attributes, span events, parent + spans, Spring AI spans, HTTP spans, real application wiring, exporter + behavior, or logs. +- The adapter starts a span with the current context rather than explicitly + creating a root span, so it can be exported as part of a larger trace whose + other spans have a different data policy. + +The runbook itself recognizes that a whole-export test is still required: +`docs/runbooks/graph-rag-production-hardening.md:31-38` requires scanning every +attribute and event and confirming that no exception event or stack trace is +present. The narrow unit test does not satisfy that release gate. + +### 4. Spring AI prompt and completion observation logging is disabled + +**Challenge claim:** Spring AI prompt and completion observation logging is +explicitly disabled in API and worker configuration. + +**Verdict:** verified as the repository default, but not structural and not +sufficient to prove application-wide payload-free egress. + +Evidence: + +- `apps/api/src/main/resources/application.yml:4-13` sets: + + ```yaml + spring: + ai: + chat: + observations: + log-prompt: false + log-completion: false + ``` + +- `apps/worker/src/main/resources/application.yml:20-24` sets the same values. +- Spring AI 2.0's resolved auto-configuration creates the prompt/completion + logging handlers only when the relevant property equals `true`. +- The official Spring AI observability documentation also describes prompt and + completion content as sensitive and disabled by default: + . + +However: + +- External configuration can override the YAML values. +- The settings control Spring AI's content logging handlers, not every + observation field, error event, provider-library WARN, or application log. +- The application programmatically supplies the real shared + `ObservationRegistry` to both model implementations: + - `integrations/ai-model-gateways/src/main/java/com/orgmemory/integrations/ai/gateway/openai/OpenAiCompatibleChatModelFactory.java:20-45` + - `integrations/ai-model-gateways/src/main/java/com/orgmemory/integrations/ai/gateway/anthropic/AnthropicMessagesChatModelFactory.java:21-46` + +Resolved Spring AI 2.0 source +`org/springframework/ai/chat/observation/DefaultChatModelObservationConvention.java:40-116` +therefore emits `gen_ai.client.operation` observations containing, among other +things: + +- request and response model IDs; +- response IDs; +- stop sequences and tool names; +- temperature, token limits, penalties, and streaming status; +- finish reasons and token counts. + +Those fields do not normally contain the full prompt, but raw model identifiers +already bypass the runbook's model-route fingerprint policy. + +More seriously, resolved Micrometer tracing source +`io/micrometer/tracing/handler/TracingObservationHandler.java:133-137` calls +`Span.error(Throwable)` for observation errors. Resolved +`micrometer-tracing-bridge-otel:1.7.0` source +`io/micrometer/tracing/otel/bridge/OtelSpan.java:170-176` records the complete +exception event and sets the OpenTelemetry error-status description to +`throwable.getMessage()`. + +That contradicts an application-wide reading of the runbook's requirement that +error spans contain no exception event or stack trace. + +### 5. OrgMemory currently has no payload escape paths outside the custom sink + +**Challenge implication:** the closed `GraphRagEvent` type makes the whole +application's observability payload-free. + +**Verdict:** wrong. + +In the resolved Spring AI 2.0 provider source: + +- `org/springframework/ai/openai/OpenAiChatModel.java:219-224` logs + `"No choices returned for prompt: " + prompt` at WARN. +- `org/springframework/ai/anthropic/AnthropicChatModel.java:551-556` logs + `"No content blocks returned for prompt: " + prompt` at WARN. +- `AnthropicChatModel.java:1015-1019` logs unsupported content blocks. +- `AnthropicChatModel.java:1216-1221` logs the raw malformed tool-argument JSON + plus the exception. + +The application's prompts can contain exactly the categories forbidden by the +runbook: + +- `integrations/graph-rag-spring-ai/src/main/java/com/orgmemory/integrations/graphrag/springai/SpringAiQueryAnswerModel.java:57-69` + sends the raw query and verified grounded prompt. +- `integrations/graph-rag-spring-ai/src/main/java/com/orgmemory/integrations/graphrag/springai/SpringAiKeywordPlanningModel.java:35-44` + sends a prompt containing the query. +- `integrations/graph-rag-spring-ai/src/main/java/com/orgmemory/integrations/graphrag/springai/SpringAiExtractionModel.java:47-61` + sends chunk and extraction-conversation content. +- `integrations/graph-rag-spring-ai/src/main/java/com/orgmemory/integrations/graphrag/springai/SpringAiDescriptionSummaryModel.java:34-56` + sends evidence-derived descriptions. + +Production config permits those WARNs: + +- `apps/api/src/main/resources/application-prod.yml:81-86` sets root logging to + INFO by default. +- `apps/worker/src/main/resources/application-prod.yml:46-50` does the same. + +Therefore `log-prompt:false` does not suppress these provider-library WARN +paths. The custom event type cannot protect a log statement that never touches +that type. + +There is already an intentional separate diagnostic path in worker indexing: + +- `apps/worker/src/main/java/com/orgmemory/worker/graph/GraphIndexingProcessor.java:202-265` + emits a bounded machine failure code to `GraphRagEventSink`. +- `GraphIndexingProcessor.java:268-282` logs diagnostic detail separately. + +That separation is architecturally useful, but the logging channel still needs +its own access, retention, and payload audit. + +### 6. Exact counts and stable fingerprints are harmless + +**Challenge implication:** counts and SHA-256 fingerprints are unconditionally +safe payload-free fields. + +**Verdict:** overstated. + +Exact counts can disclose organization or corpus topology, including authorized +asset, evidence-closure, chunk, entity, relation, or keyword cardinalities. +Stable unkeyed hashes allow cross-event correlation and dictionary testing when +the input domain is low entropy. + +The current values are materially safer than text, but production policy should +consider: + +- stage-specific upper bounds or buckets; +- domain-separated keyed hashes rather than bare SHA-256; +- explicit constructors for route and scope fingerprints so arbitrary producers + cannot hash payload text; +- retention and access limits even for “payload-free” telemetry. + +### 7. The comparable-systems table establishes an industry baseline + +**Challenge claim:** every comparable system permits prompt/completion egress, +so OrgMemory is alone at “never.” + +**Verdict:** not established. The table mixes unlike mechanisms and several +rows are overstated. + +#### Dify + +The cited issue reports that a +user observed several telemetry domains and asks for documentation and opt-out +instructions. It does not demonstrate that full prompt and completion history +is exported by default. Configured Langfuse workflow tracing is different from +unconditional native telemetry. + +The challenge's “Yes, by default” conclusion is unsupported by that citation. + +#### Onyx + +The main capture claim is verified: + +- `tmp/onyx/backend/onyx/tracing/framework/span_data.py:89-146` defines + generation span data containing input, output, and reasoning. +- `tmp/onyx/backend/onyx/tracing/langfuse_tracing_processor.py:295-321` + sends input/output and other generation information. +- `tmp/onyx/backend/onyx/tracing/langfuse_tracing_processor.py:143-149` + sends user and session IDs. +- `tmp/onyx/backend/onyx/tracing/masking.py:7-71` only handles a narrow set of + private-key/authorization patterns and a 500,000-character limit. Normal + prompt text survives. +- `tmp/onyx/backend/onyx/tracing/langfuse_tracing_processor.py:76-86` fails open: + if masking throws, it logs a warning and returns the original unmasked value. +- Error status data at + `tmp/onyx/backend/onyx/tracing/langfuse_tracing_processor.py:335-340` is not + passed through the same masking helper. + +Live reload is also real: + +- `tmp/onyx/backend/onyx/tracing/dynamic_processor.py:104-159` reloads provider + configuration. +- `tmp/onyx/backend/onyx/configs/app_configs.py:1618-1619` gives the cache a + short TTL. + +But the proposed “per-organization database setting modelled on Onyx” is wrong: + +- `tmp/onyx/backend/onyx/tracing/provider_config.py:123-128` explicitly bypasses + database configuration in `MULTI_TENANT` mode and uses environment-wide + settings. +- `tmp/onyx/backend/onyx/server/manage/tracing/api.py:36-46` rejects tracing + configuration management in multi-tenant mode. +- In non-multi-tenant mode, the database model holds one unique configuration + row per provider rather than a SaaS organization-by-organization consent + model. + +Onyx is therefore evidence that live-reloaded capture and narrow masking exist. +It is not evidence that a per-organization Tier 2 is safe or already solved. + +Onyx is also permission-aware: + +- `tmp/onyx/backend/onyx/context/search/preprocessing/access_filters.py:8-22` + applies user access filters to search. +- `tmp/onyx/backend/onyx/db/document_access.py:26-83` filters by public, + email, and group ACL. + +OrgMemory's full canonical closure recheck is stronger, but “permission-aware +retrieval is unique to OrgMemory” is not true. + +#### Sentry MCP + +The repository confirms conditional prompt/output capture, but the challenge +overstates it as unconditional and overstates the protection provided by its +scrubber. + +- `tmp/upstream-sentry-mcp-20260726/TELEMETRY.md:258-265` says Node/stdio and + Cloudflare telemetry are disabled when the DSN is absent. +- `tmp/upstream-sentry-mcp-20260726/packages/mcp-server/src/index.ts:279-302` + explicitly enables Vercel AI input/output recording when Sentry is + configured. +- `tmp/upstream-sentry-mcp-20260726/packages/mcp-core/src/telem/sentry.ts:7-28` + has only three secret patterns and a recursion depth of twenty. +- `tmp/upstream-sentry-mcp-20260726/packages/mcp-core/src/telem/sentry.ts:34-117` + recursively scrubs events and warns when it changes an event. + +The custom hook is `beforeSend`; the source contains no `beforeSendSpan`. +Sentry's official documentation exposes `beforeSendSpan` as the span-specific +hook. Therefore the event scrubber cannot be treated as structural protection +for GenAI span input/output: +. + +This is evidence that scrub visibility is useful, but also that a narrow +pattern scrubber is not a proof of absence. + +#### LibreChat + +The described backend RUM proxy validates the session and removes application +authorization headers before forwarding browser telemetry. That is a browser +RUM transport boundary, not proof that prompt/completion content is routinely +captured or a comparable design for ACL-derived model output. + +#### OpenTelemetry GenAI semantic conventions + +Content capture is commonly opt-in and disabled by instrumentation defaults, +but describing it as one universal semantic-convention environment variable is +too broad. Semantic conventions define names and meanings; individual +instrumentations decide which switches and defaults they implement. Official +OpenTelemetry material documents opt-in message-content capture, but it is not +a universal structural policy for every SDK and instrumentation: +. + +#### Spring AI + +The Spring AI row is substantially correct: prompt and completion content +logging is opt-in/default-off. However, it does not imply that model metadata, +exception events, status descriptions, or provider-library WARNs are absent. + +#### RAGFlow + +The inspected material supports the existence of a configured Langfuse +integration. It does not establish unconditional default prompt/completion +egress, nor does it establish governance equivalence with OrgMemory. + +#### OrgMemory + +The table heading must be qualified as “leaves the process through +observability.” Literally, prompts necessarily leave the application for the +configured model provider: + +- `core/src/main/java/com/orgmemory/core/assistant/AssistantService.java:58-63` + sends the verified request through `ChatModelPort`. + +Even under the intended observability-only interpretation, “Never” is currently +false as an application-wide statement because the Spring AI observation, +Micrometer exception, and WARN paths described above bypass the custom sink. + +### 8. The four-day OTLP warning demonstrates the operational cost of the payload policy + +**Challenge claim:** the warning remained undiagnosed partly because telemetry +does not say anything useful, supporting Tier 1. + +**Verdict:** the dependency/default mechanism is verified; the causal conclusion +is unsupported. + +Verified mechanism: + +- `apps/api/build.gradle.kts:23` and `apps/worker/build.gradle.kts:15` add + `spring-boot-starter-opentelemetry`. +- Gradle resolves + `micrometer-registry-otlp:1.17.0 <- spring-boot-starter-opentelemetry:4.1.0`. +- Resolved Spring Boot source + `org/springframework/boot/micrometer/metrics/autoconfigure/export/otlp/OtlpMetricsExportAutoConfiguration.java:58-65` + uses `@ConditionalOnEnabledMetricsExport("otlp")`. +- Resolved Spring Boot source + `OnMetricsExportEnabledCondition.java:39-77` defaults exporters to enabled + when no product-specific or default disable property exists. +- Resolved Micrometer source + `io/micrometer/registry/otlp/OtlpConfig.java:54-77` defaults to + `http://localhost:4318/v1/metrics`. +- Resolved Micrometer source + `io/micrometer/registry/otlp/OtlpMeterRegistry.java:193-213` catches the + publishing exception and calls `logger.warn(message, exception)`. The + configuration context includes the destination URL and resource attributes. + +Repository deployment configuration does not supply an OTLP metrics endpoint or +explicitly disable the exporter: + +- `apps/api/src/main/resources/application-prod.yml:70-79` +- `apps/worker/src/main/resources/application-prod.yml:37-44` +- `infrastructure/deployment/compose.production.yaml:21-53` +- API and worker inherit the shared environment at + `infrastructure/deployment/compose.production.yaml:329-333,376-380`. + +Exception detail was therefore available in the logging call, and production +root INFO admits WARN. Docker's `json-file` logging configuration captures +stdout/stderr at `infrastructure/deployment/compose.production.yaml:3-14`. + +Even the one-line warning identified: + +- the failed subsystem: OTLP metrics publishing; +- the exact destination: localhost port 4318; +- the metrics endpoint path. + +The repository provides a stronger explanation for delayed detection: + +- `infrastructure/deployment/scripts/smoke-production.sh:31-147` checks health + and functional HTTP behavior, but not exporter delivery or repeated log + failures. +- `infrastructure/deployment/scripts/deploy.sh:213-226` declares deployment + success after those smokes. +- The checked-in infrastructure contains no corresponding collector-delivery or + log-alert verification. + +The repository history makes four days temporally plausible because the starter +was introduced on 2026-07-25, but it does not prove continuous duration, who +observed the warning, or that a team belief that telemetry is useless caused the +delay. + +Most importantly, the OTLP meter registry never passes through +`GraphRagEventSink`. Adding exception messages to `GraphRagEvent` would neither +enrich nor detect that metrics warning. Sending the diagnostic through the same +broken OTLP route would also be circular. + +## Answers to the five questions + +## 1. Is the operational-cost argument for Tier 1 sound? + +No. + +The incident demonstrates three operational problems: + +1. a transitive exporter was enabled without an intentional endpoint; +2. deployment validation did not verify telemetry delivery or scan for exporter + failures; +3. repeated WARNs lacked alert ownership. + +It does not demonstrate that infrastructure exception detail was unavailable. +`OtlpMeterRegistry` logged the full caught exception, production log levels +admitted it, and the warning's own message contained the failing URL. + +It also does not demonstrate a limitation of `GraphRagEventSink`, because the +warning originates in an independent Micrometer registry publisher. Tier 1 +would not have changed that code path. + +The appropriate fixes are: + +- set `management.otlp.metrics.export.enabled=false` when no receiver exists; +- when enabled, require an explicit endpoint and fail deployment or startup if + the endpoint is absent; +- add a smoke check that exports a known metric/span and verifies receipt; +- alert on repeated exporter WARNs and dropped-signal indicators; +- define responsibility for collector and exporter health. + +There is a separate real issue: GraphRAG event producers often catch and ignore +sink failures: + +- `core/src/main/java/com/orgmemory/core/knowledge/GraphRagKnowledgeRetrievalService.java:149-172,565-610,630-646,772-788` +- `apps/worker/src/main/java/com/orgmemory/worker/graph/GraphIndexingProcessor.java:249-265` + +That should produce a local payload-free health counter or bounded log signal. +It still does not justify putting arbitrary exception messages into the remote +GraphRAG telemetry type. + +## 2. Does ACL-scoped evidence justify diverging from every comparable system? + +The argument is partly motivated and partly sound. + +The motivated part is the assertion of uniqueness: + +- permission-aware retrieval is not unique to OrgMemory; +- the comparable table does not establish that every peer exports payload by + default; +- the rows describe different mechanisms and threat models; +- competitor behavior would not by itself establish a safe authorization or + persistence design. + +The sound part is the local authorization consequence: + +- a query can contain newly supplied confidential material that is not yet a + governed Knowledge Asset; +- a completion can quote, combine, or paraphrase material from several assets; +- the completion inherits the effective permissions of every contributor; +- an organization-level consent flag is coarser than per-asset and current + source ACLs; +- consent expiry does not retract data already exported to a telemetry backend; +- an ordinary trace viewer cannot enforce current source revocation, deletion, + retention, evidence-closure authorization, or operator access. + +Tier 2 would therefore create a second persistence/publication plane whose +authorization semantics do not match the plane that allowed the model request. + +The defensible principle is not “OrgMemory is unique, therefore content must +never exist anywhere.” It is: + +> Query and completion content must not enter an ordinary observability store +> unless that store models the same authorization, retention, revocation, +> deletion, and operator-access semantics as the governed evidence and its +> derivatives. + +Under the proposed design, it does not. That is enough to reject Tier 2. + +## 3. If tiering is adopted, what failure is most likely, and what structural control prevents it? + +The most likely failure is **tier bleed caused by stale or missing tenant +context**. + +Examples: + +- a shared/global observation handler sees a Tier 2 flag and captures another + organization's request; +- a cached organization setting remains enabled after consent expires; +- asynchronous work loses the organization identity but retains capture state; +- a database setting changes while a trace is in flight; +- a global environment override silently wins over the database; +- a payload-bearing event enters the ordinary Tier 0 exporter; +- the telemetry backend retains payload after the capture lease ends; +- a scrubber misses content in a nested exception, provider body, URL, filename, + SQL message, tool argument, alternate encoding, or unknown secret format; +- the scrubber itself fails and returns the original data. + +This risk is especially high in the current application because: + +- OpenAI and Anthropic model factories share the application + `ObservationRegistry`; +- GraphRAG model construction and some model invocations do not consistently + carry an organization-bound capture capability; +- the proposed Onyx precedent is global in multi-tenant mode, not per + organization. + +If tiering were nevertheless adopted, the structural control must be a +physically and logically distinct payload capability: + +1. `GraphRagEvent` remains incapable of carrying content. +2. Payload capture uses a different sealed type and API. +3. Capture requires a non-forgeable, organization- and operation-bound + capability containing consent version and expiry. +4. The capability is revalidated immediately before capture and again before + egress; absent or stale context fails closed. +5. No environment variable or process-global Boolean may enable tenant payload + capture. +6. The payload path uses a separate process or exporter, endpoint, credentials, + network policy, queue, encryption key, storage, and RBAC. +7. The destination enforces hard TTL, deletion, tenant partitioning, access + audit, and lease expiry. +8. Completion capture additionally carries the evidence-closure identity and + enforces current authorization for every viewer. + +For Tier 1 specifically, arbitrary exception messages should not be admitted at +all. Use a closed or sealed infrastructure taxonomy with safe structured fields +such as dependency alias, status family, timeout, retry count, errno, and stack +fingerprint. Pattern scrubbing is defense in depth, not a structural +authorization or data-absence control. + +The proposed Tier 0 redactor counter also needs correction. The existing sink +does not receive payload and has no redactor. To count rejected near-misses, the +system would first need to create a raw candidate data path, weakening the +current type boundary. Prefer: + +- closed construction APIs; +- semantic tests for every permitted field; +- whole-export allowlist/denylist tests; +- collector-side defense-in-depth filtering; +- local counters for invalid event construction or rejected attributes, without + carrying the rejected value. + +## 4. Is there a fourth option? + +Yes. Keep the GraphRAG OTLP sink payload-free and create a separate, +deployment-controlled diagnostics path. + +The ordinary remote telemetry path should retain: + +- operation and organization identifiers under the approved identity policy; +- stage and outcome; +- bounded or bucketed counts; +- duration; +- closed failure codes; +- safe route/scope correlation values; +- exporter health counters. + +The diagnostic path should have a different schema and governance boundary. Its +default structured envelope can contain: + +- sealed exception class or error family; +- subsystem and dependency alias; +- status-code family or errno; +- timeout, retryability, and attempt count; +- deployment or build identity; +- bounded cause-chain taxonomy; +- stack fingerprint, not stack text. + +It should use: + +- different endpoint and credentials; +- separate network egress policy; +- short retention; +- restricted operator access; +- explicit access audit; +- encryption; +- independent availability and failure alerts. + +Full stack details can remain in protected local/deployment logs where justified, +provided those logs receive their own payload audit and retention controls. +Existing worker code already separates the machine-code event from diagnostic +logging in `GraphIndexingProcessor.java:221-282`. + +If a rare support case truly requires query, completion, or evidence-derived +content, it should not be “Tier 2 telemetry.” It should be an explicit, +single-operation support capture represented as a governed derived resource: + +- organization- and operation-bound; +- authorized under OpenFGA or an equivalent current-policy check; +- associated with the evidence closure; +- encrypted and stored in a dedicated support store; +- accessible only to approved support principals; +- fully audited; +- subject to hard TTL and verified deletion. + +That is a new governed product capability and should receive its own +authorization, persistence, and publication challenge. It is not a telemetry +configuration flag. + +## 5. Which variant should ship? + +Ship **unchanged** among the offered variants. + +Reject all three tiers because Tier 2's organization-level consent cannot model +asset-level ACL, evidence inheritance, revocation, deletion, retention, or +operator access. + +Reject Tier 0 + Tier 1 because arbitrary exception messages remain payload. A +pattern scrubber cannot prove that unknown customer text, credentials, provider +bodies, filenames, SQL, URLs, tool arguments, nested causes, or alternative +encodings were removed. The operational incident cited in the proposal also +would not have been improved by Tier 1. + +Retain the current payload-free GraphRAG event policy, but do not ship a release +claiming global enforcement until the following implementation gaps are closed: + +1. Explicitly disable OTLP metrics when no receiver is configured and require a + valid endpoint when enabled. +2. Audit or suppress Spring AI's standard model observations when they violate + the GraphRAG allowlist, including raw model IDs and exception events. +3. Prevent provider-library WARN paths from logging raw prompts, response + blocks, or tool arguments. +4. Replace open-string `failureCode` with a closed taxonomy. +5. Make route/scope fingerprint construction domain-specific and preferably + keyed/domain-separated. +6. Bound or bucket counts according to stage and disclosure risk. +7. Add a payload-free local health signal when custom event emission fails. +8. Add a real application/exporter integration test that scans: + - every span and event; + - resource and instrumentation-scope attributes; + - Spring AI and HTTP spans; + - success and failure paths; + - relevant stdout/stderr logs; + - actual collector output. +9. Retain sanitized export evidence before claiming the runbook's payload-free + production gate has passed. + +## Final decision statement + +The operational incident does not justify weakening the GraphRAG telemetry +boundary. The custom event type is valuable precisely because payload cannot be +added through ordinary configuration, but the repository currently overstates +how far that protection extends. + +The selected architecture is: + +> Keep `GraphRagEventSink` payload-free and reject Tier 1 and Tier 2. Close the +> existing Spring AI, Micrometer, and logging bypasses. Put richer operational +> diagnostics on a separate, access-controlled egress path. Treat any future +> content capture as a governed, short-lived derived resource—not telemetry. + diff --git a/docs/increments/active/2026-07-29-observability-pipeline/design.md b/docs/increments/active/2026-07-29-observability-pipeline/design.md new file mode 100644 index 00000000..3148b37f --- /dev/null +++ b/docs/increments/active/2026-07-29-observability-pipeline/design.md @@ -0,0 +1,191 @@ +# Observability pipeline and payload boundary + +Date: 2026-07-29 + +## Outcome + +Give production a telemetry pipeline that reaches a real collector, stop the +payload paths that bypass the GraphRAG event boundary, and stop the exporter +failure that has been logging every minute since 2026-07-25. + +## Production evidence + +`orgmemory-api-1` and `orgmemory-worker-1` log every minute: + +```text +i.m.registry.otlp.OtlpMeterRegistry : Failed to publish metrics to OTLP receiver +(context: url=http://localhost:4318/v1/metrics, resource-attributes={service.name=orgmemory-api}) +``` + +The mechanism is a transitive default, not a misconfiguration: + +- `apps/api/build.gradle.kts:23` and `apps/worker/build.gradle.kts:15` add + `spring-boot-starter-opentelemetry`, which resolves + `io.micrometer:micrometer-registry-otlp:1.17.0`. +- `OtlpMetricsExportAutoConfiguration` is `@ConditionalOnEnabledMetricsExport("otlp")`, + and `OnMetricsExportEnabledCondition` treats exporters as enabled unless a + property disables them. Metrics export is opt-out. +- Micrometer's `OtlpConfig.url()` defaults to `http://localhost:4318/v1/metrics`. +- No repository configuration sets an OTLP metrics endpoint. + +OTLP **tracing** behaves oppositely and is not involved: `OtlpTracingConfigurations` +gates its connection-details bean on +`management.opentelemetry.tracing.export.otlp.endpoint`, so no endpoint means no +exporter and no log noise. + +`localhost:4318` inside the API container is that container, so the export never +had a destination. The ZM host does run a Grafana stack — +`zeromail-alloy`, `zeromail-prometheus`, `zeromail-loki`, `zeromail-tempo`, +`zeromail-grafana`, all `Exited` since 2026-07-25 — but `zeromail-alloy` publishes +4317/4318 to the host loopback on network `zeromail-internal`, which OrgMemory +containers do not join. Its shutdown is coincident, not causal. + +Readiness stayed `UP` throughout, which is correct: exporter delivery is not a +readiness signal. + +## Payload boundary: challenge and finding + +`docs/runbooks/graph-rag-production-hardening.md` states that telemetry never +carries query, prompt, completion, evidence text, document title/URI, embedding +values, actor identity, ACL subjects, or exception messages. A proposal to +replace that flat rule with three configurable tiers was put through an +independent architecture challenge (Codex, `gpt-5.6-sol`, ultra effort). The +challenge brief and full verdict were produced during the review; their +substance is recorded here because that scratch location is not versioned. + +### Proposal + +- Tier 0: current allowlist, unchanged, plus a counter for redactor near-misses. +- Tier 1: opt-in per deployment — infrastructure exception class and message + through a pattern scrubber, hashed actor id. +- Tier 2: opt-in per organization, time-boxed, consent recorded — query and + completion text, modelled on Onyx. +- Never at any tier: evidence/chunk text, document title/URI, ACL subjects, + embedding values. + +Supporting argument: no comparable open-source system chose "never". Dify, Onyx, +Sentry MCP, LibreChat and RAGFlow all permit content egress; OpenTelemetry's +GenAI conventions and Spring AI make it opt-in and default-off. The operational +cost of the strict rule was argued from the four-day undiagnosed OTLP warning. + +### Strongest counterargument + +An absolute rule is auditable in one line and cannot be misconfigured. Three +tiers create three states to audit, and one environment variable set wrong at +Tier 2 leaks customer content. The current design enforces the rule in the type +system, which configuration discipline cannot match. + +### Repository evidence produced by the challenge + +The review rejected the proposal and produced two verified findings that +outweigh it. Both were independently confirmed against the resolved sources. + +**Provider libraries log raw prompts.** `log-prompt: false` disables Spring AI's +observation handlers, not the provider libraries' own logging: + +```text +org/springframework/ai/openai/OpenAiChatModel.java:222 + logger.warn("No choices returned for prompt: " + prompt); +org/springframework/ai/anthropic/AnthropicChatModel.java:554 + logger.warn("No content blocks returned for prompt: " + prompt); +org/springframework/ai/anthropic/AnthropicChatModel.java:1219 + logger.warn("Failed to parse tool arguments JSON: " + argumentsJson, e); +``` + +OrgMemory prompts carry exactly the forbidden categories — +`SpringAiQueryAnswerModel` sends the query and grounded prompt, +`SpringAiExtractionModel` sends chunk content, +`SpringAiDescriptionSummaryModel` sends evidence-derived descriptions. Production +root logging is INFO, so WARN is emitted, and Compose retains stdout in +`json-file` logs on the host. + +**Error spans carry exception events and messages.** +`io/micrometer/tracing/otel/bridge/OtelSpan.java:170-179` calls +`recordException(throwable)` and sets the status description to +`throwable.getMessage()`. The runbook's release gate — "confirm error spans +contain no exception event or stack trace" — cannot pass with the current wiring. + +The review also corrected the proposal's own evidence: the runbook omits three +fields the record actually carries (`scopeFingerprint`, `cacheStatus`, +`occurredAt`); "bounded counts" is only a non-negativity check; and Onyx's +tracing configuration is environment-wide in `MULTI_TENANT` mode +(`provider_config.py:123-128`), so it is not a precedent for per-organization +consent. + +## Decision + +**Keep the payload-free policy. Reject Tier 1 and Tier 2.** + +The governing principle is not that OrgMemory is unique — permission-aware +retrieval is not unique, and Onyx applies access filters too. It is: + +> Query and completion content must not enter an ordinary observability store +> unless that store models the same authorization, retention, revocation, +> deletion, and operator-access semantics as the governed evidence and its +> derivatives. + +A completion inherits the effective permissions of every contributing evidence +item. An organization-level consent flag is coarser than per-asset ACLs, consent +expiry cannot retract exported data, and a trace viewer cannot enforce current +revocation. That is sufficient to reject Tier 2. Tier 1 is rejected because an +arbitrary exception message is payload, and a pattern scrubber cannot prove that +customer text, provider bodies, filenames, SQL, URLs, or tool arguments were +removed. + +The Tier 0 redactor counter is also rejected: the sink has no redactor because +payload cannot reach it. Counting near-misses would require building a raw +candidate path, weakening the boundary it was meant to observe. + +**Rejected alternative:** enabling `spring.ai.chat.observations.log-prompt` and +exporting to Langfuse. Langfuse remains supported as an OTLP destination for +payload-free spans; it needs no adapter code, only an endpoint and headers. + +**Accepted alternative for operability:** richer diagnostics belong on a separate +egress path with its own schema (sealed exception family, dependency alias, +status family, retry count, stack fingerprint rather than stack text), +credentials, retention, and access control — not inside `GraphRagEventSink`. + +The policy is unchanged but the repository currently claims more than it +enforces. The bypasses above must close before any release describes the +deployment as payload-free. + +## Pipeline architecture + +- **Push over pull.** OTLP export is already wired and the worker has no HTTP + surface to scrape. Onyx needed a separate metrics server per Celery worker for + exactly that reason. Accepted cost: a push failure is invisible to the + monitoring system where a scrape failure is not; Compose healthchecks already + cover liveness. +- **One collector.** Alloy receives metrics and traces. Langfuse was rejected as + the primary destination because it does not accept JVM, HTTP, or database + metrics and would require a second pipeline. +- **Standard configuration.** `OTEL_EXPORTER_OTLP_ENDPOINT` maps to metrics, + traces, and logs through Boot's environment post-processor. OTLP log export is + disabled explicitly because Alloy already tails the `json-file` driver. +- **Silent when unconfigured.** No endpoint must mean no exporter and no + recurring log line. Tracing already behaves this way; metrics must match. +- **Readiness stays independent of exporter health.** + +## Trace completeness gaps + +- Worker jobs have no root span, so every span in an indexing job is an orphan + root correlated only by the `operation_id` attribute. +- `GraphRagKnowledgeRetrievalService:449` and `GraphIndexingProcessor:303` use + raw `Executors.newVirtualThreadPerTaskExecutor()`. Stage events are emitted on + the calling thread and keep their parent, but model calls inside those tasks + detach. `io.micrometer:context-propagation:1.2.1` is already on the classpath. +- `apps/mcp` has no OpenTelemetry starter, so an agent trace begins at the API + and its `RestClient` calls carry no `traceparent`. +- Resource attributes lack `service.version` and `deployment.environment`; + image tags already carry the commit SHA. +- Sampling is 0.1 for both API and worker. A low-volume batch workload should be + 1.0. + +## Exit proof + +- Production emits no recurring exporter warning. +- A provider failure path produces no prompt in any log stream. +- One retrieval and one indexing job appear in the collector as connected traces + with the expected stages, and a sanitized export shows no forbidden attribute. +- Stage latency is answerable from metrics at full coverage rather than from a + 10 % trace sample. diff --git a/docs/increments/active/2026-07-29-observability-pipeline/plan.md b/docs/increments/active/2026-07-29-observability-pipeline/plan.md new file mode 100644 index 00000000..aaa976a5 --- /dev/null +++ b/docs/increments/active/2026-07-29-observability-pipeline/plan.md @@ -0,0 +1,100 @@ +# Observability pipeline plan + +## 0. Close the payload bypasses — not started + +Highest priority. These are live paths, independent of the pipeline work. + +- [ ] Pin `org.springframework.ai.openai` and `org.springframework.ai.anthropic` + logger levels to `ERROR` in API and worker production configuration, so the + provider WARN paths that concatenate the prompt cannot emit. +- [ ] Search retained production logs for those three WARN signatures. Treat a + hit as a data incident, not a defect: record scope and affected + organizations before changing anything. +- [ ] Decide how `OtelSpan.error` is handled. Either suppress exception recording + for observations that can carry OrgMemory content, or amend the runbook + gate that currently cannot pass. +- [ ] Add a payload-free health signal for swallowed `GraphRagEventSink` + failures. Producers currently catch and ignore them + (`GraphRagKnowledgeRetrievalService:149-172,565-610,630-646,772-788`, + `GraphIndexingProcessor:249-265`), so a broken sink is invisible. + +## 1. Silence the unconfigured exporter — not started + +- [ ] `management.otlp.metrics.export.enabled` defaults to false; enabling it + requires an explicit endpoint. +- [ ] `management.logging.export.otlp.enabled: false` — Alloy tails `json-file`. +- [ ] `management.opentelemetry.map-environment-variables: false` in production + so host `OTEL_*` cannot enable export implicitly. +- [ ] Declare `spring.ai.chat.observations.include-error-logging: false` + explicitly rather than relying on the framework default. +- [ ] Add `service.version` and `deployment.environment` resource attributes. +- [ ] Worker tracing sampling to 1.0; API stays at 0.1. + +Gate: `compileJava`, `:core:test`. + +## 2. Metrics that answer stage latency — not started + +Depends on the composite sink merged in PR #132. + +- [ ] Remove `@ConditionalOnMissingBean(GraphRagEventSink.class)` from + `GraphRagObservabilityAutoConfiguration`; a sink is not an exclusive port. + Add a per-sink `@ConditionalOnProperty` toggle. +- [ ] Add `MicrometerGraphRagEventSink` — timers by stage, outcome and cache + status; counters by failure code. Metrics are not sampled, so stage p95 + becomes answerable at full coverage. +- [ ] Export `ContextTokenUsage`, which core already computes and nothing + publishes, plus a counter for `finish_reason=length` truncation. +- [ ] Time to first token on the streaming assistant path. +- [ ] Tests: both sinks enabled receive the event; both disabled compose to + `NO_OP`. + +Gate: module tests, `:core:test`. + +## 3. Collector and dashboards — not started + +- [ ] `compose.observability.yaml` behind a profile, restoring the existing + Alloy/Loki/Tempo/Prometheus/Grafana stack. +- [ ] Join API and worker to the collector network; set + `OTEL_EXPORTER_OTLP_ENDPOINT`. Prometheus stays off the proxy network; + Grafana is published through Nginx Proxy Manager with Keycloak OIDC. +- [ ] Commit dashboards to the repository: infrastructure, and a separate AI + cost and quality board covering tokens by organization, truncation rate, + cache hit rate and TTFT. +- [ ] Extend `smoke-production.sh` to export a known signal and verify receipt, + so a silent exporter fails deployment instead of passing it. + +## 4. Trace continuity — not started + +Highest regression risk; run it last, once a collector can show the result. + +- [ ] Root span per worker indexing job. +- [ ] Wrap both virtual-thread executors with `ContextSnapshot` so model calls + inside tasks keep their parent. +- [ ] Add the OpenTelemetry starter to `apps/mcp` and propagate `traceparent` on + its API client. +- [ ] Prove: a job's child spans share the root `traceId`, and a span created + inside a virtual-thread task has the expected parent. + +## 5. Consolidate — not started + +- [ ] Whole-export test: scan every span, event, resource and + instrumentation-scope attribute, plus the log stream, on success and + failure paths, against the allowlist. This is the runbook's release gate + and it has never been executed. +- [ ] Reconcile `docs/specs/domains/secure-graph-rag.md` and its test matrix. +- [ ] Correct the runbook's field list — it omits `scopeFingerprint`, + `cacheStatus` and `occurredAt`, and describes counts as bounded when only + non-negativity is checked. +- [ ] Record the payload-boundary decision under `docs/decisions/`, superseding + nothing but documenting the challenge outcome. + +## Deferred, with reasons + +- Prometheus scrape endpoint. OTLP push is already wired and the worker has no + scrape target. +- Langfuse as a destination. It accepts payload-free OTLP spans today with no + adapter code; it earns a pipeline only if governed content capture is ever + built. +- Closed `failureCode` taxonomy, keyed fingerprints, bucketed counts. Raised by + the architecture challenge as hardening beyond the current gap; worth doing, + not blocking. diff --git a/docs/roadmap.md b/docs/roadmap.md index de584b88..a72af622 100644 --- a/docs/roadmap.md +++ b/docs/roadmap.md @@ -40,6 +40,7 @@ The table is a delivery index, not a second description of current behavior. | [MCP search reliability](increments/active/2026-07-28-mcp-search-reliability/plan.md) | active | deploy merged timeout repair and prove the production MCP call | | [SCIM provisioning foundation](increments/active/2026-07-27-scim-provisioning-foundation/plan.md) | active | previous-binary/restore rehearsals and two-organization negative evidence | | [Public docs co-authoring and information architecture](increments/active/2026-07-29-public-docs-coauthoring/plan.md) | active | migrate the four-root taxonomy, then co-author What is OrgMemory? with the owner | +| [Observability pipeline and payload boundary](increments/active/2026-07-29-observability-pipeline/plan.md) | active | close the provider-library prompt logging bypass, then reach a collector and prove a sanitized whole-export | The other SCIM directories under `increments/active/` are dependency-ordered future designs inside the active native identity program. They do not become From de426fafc80935eecb61501b5d91c1c7f0058d3a Mon Sep 17 00:00:00 2001 From: kl3inIT Date: Wed, 29 Jul 2026 22:53:28 +0700 Subject: [PATCH 2/8] fix(observability): stop the provider libraries from logging prompts `spring.ai.chat.observations.log-prompt: false` governs Spring AI's own observation handlers. It does not reach the client libraries, which build their own WARN messages by concatenating the prompt or the response content: org/springframework/ai/openai/OpenAiChatModel.java:222 org/springframework/ai/anthropic/AnthropicChatModel.java:554 org/springframework/ai/anthropic/AnthropicChatModel.java:1219 org/springframework/ai/anthropic/AnthropicChatModel.java:1018 OrgMemory prompts carry the query, the grounded context and chunk content, so those lines put customer text into a log the deployment retains on the host. Production runs at root INFO, so nothing was suppressing them. Every one of those call sites is guarded by isWarnEnabled(), so pinning the two packages above WARN stops the message from being built rather than only from being printed. The pin lives in the base configuration, not the production profile, because a development database holds real uploaded documents too, and it is deliberately not environment-overridable: the payload-free boundary is not a per-deployment setting. The tests read the shipped YAML rather than a copy of it, and fail if a profile lowers either package back to WARN. Co-Authored-By: Claude Opus 5 (1M context) --- apps/api/src/main/resources/application.yml | 12 +++ .../ProviderLoggingBoundaryTests.java | 100 ++++++++++++++++++ .../worker/src/main/resources/application.yml | 12 +++ .../ProviderLoggingBoundaryTests.java | 100 ++++++++++++++++++ 4 files changed, 224 insertions(+) create mode 100644 apps/api/src/test/java/com/orgmemory/api/observability/ProviderLoggingBoundaryTests.java create mode 100644 apps/worker/src/test/java/com/orgmemory/worker/observability/ProviderLoggingBoundaryTests.java diff --git a/apps/api/src/main/resources/application.yml b/apps/api/src/main/resources/application.yml index 74601d8b..b92b37b3 100644 --- a/apps/api/src/main/resources/application.yml +++ b/apps/api/src/main/resources/application.yml @@ -179,3 +179,15 @@ management: web: exposure: include: health,info,modulith + +logging: + level: + # The provider libraries concatenate the prompt and the response content into + # WARN messages of their own, which `spring.ai.chat.observations.log-prompt` + # does not reach because it only governs Spring AI observation handlers. + # Every such site is guarded by isWarnEnabled(), so pinning these packages + # above WARN stops the payload from being built at all. Deliberately not + # environment-overridable: the payload-free boundary is not a per-deployment + # setting. See docs/runbooks/graph-rag-production-hardening.md. + org.springframework.ai.openai: ERROR + org.springframework.ai.anthropic: ERROR diff --git a/apps/api/src/test/java/com/orgmemory/api/observability/ProviderLoggingBoundaryTests.java b/apps/api/src/test/java/com/orgmemory/api/observability/ProviderLoggingBoundaryTests.java new file mode 100644 index 00000000..8be03d31 --- /dev/null +++ b/apps/api/src/test/java/com/orgmemory/api/observability/ProviderLoggingBoundaryTests.java @@ -0,0 +1,100 @@ +package com.orgmemory.api.observability; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.fail; + +import java.io.File; +import java.io.IOException; +import java.util.List; +import java.util.Locale; +import java.util.Map; +import java.util.Set; +import org.junit.jupiter.api.Test; +import org.springframework.boot.env.YamlPropertySourceLoader; +import org.springframework.core.env.PropertySource; +import org.springframework.core.io.ClassPathResource; +import org.springframework.core.io.FileSystemResource; + +/** + * The OpenAI and Anthropic client libraries build WARN messages that concatenate + * the prompt and the response content. {@code spring.ai.chat.observations.log-prompt} + * does not reach them because it only governs Spring AI observation handlers. + * + *

Every such call site is guarded by {@code isWarnEnabled()}, so the level + * configured for those packages is what keeps the payload from being built at all. + * That makes the configured level part of the payload-free boundary rather than an + * operator preference, and this test is what stops it from drifting. + */ +class ProviderLoggingBoundaryTests { + + /** Packages whose own logging concatenates OrgMemory prompt or completion text. */ + private static final List PAYLOAD_LOGGING_PACKAGES = List.of( + "org.springframework.ai.openai", + "org.springframework.ai.anthropic"); + + /** Levels at or above ERROR; anything else lets a guarded WARN site build its message. */ + private static final Set LEVELS_ABOVE_WARN = Set.of("ERROR", "FATAL", "OFF"); + + @Test + void theBaseConfigurationPinsEveryPayloadLoggingPackageAboveWarn() throws IOException { + Map properties = load(new ClassPathResource("application.yml")); + + for (String payloadPackage : PAYLOAD_LOGGING_PACKAGES) { + // The loader returns origin-tracked char sequences rather than plain strings. + Object level = properties.get("logging.level." + payloadPackage); + assertEquals( + "ERROR", + String.valueOf(level), + () -> payloadPackage + + " must be pinned to ERROR in application.yml so its guarded WARN" + + " sites cannot concatenate the prompt"); + } + } + + @Test + void noProfileLowersAPayloadLoggingPackageBackToWarn() throws IOException { + File[] profiles = profileConfigurationFiles(); + assertTrue(profiles.length > 0, "no application-.yml was scanned, so this test proves nothing"); + + for (File profile : profiles) { + Map properties = load(new FileSystemResource(profile)); + + properties.forEach((key, value) -> { + if (!key.startsWith("logging.level.")) { + return; + } + String logger = key.substring("logging.level.".length()); + if (PAYLOAD_LOGGING_PACKAGES.stream().noneMatch(p -> p.equals(logger) || p.startsWith(logger + "."))) { + return; + } + assertTrue( + LEVELS_ABOVE_WARN.contains(String.valueOf(value).toUpperCase(Locale.ROOT)), + () -> profile.getName() + " sets " + key + "=" + value + + ", which re-enables the provider WARN sites that log the prompt"); + }); + } + } + + private static Map load(org.springframework.core.io.Resource resource) throws IOException { + List> sources = new YamlPropertySourceLoader().load(resource.getFilename(), resource); + if (sources.size() != 1) { + fail(resource.getFilename() + " must contain exactly one YAML document, found " + sources.size()); + } + @SuppressWarnings("unchecked") + Map properties = (Map) sources.getFirst().getSource(); + return properties; + } + + /** + * Profile files live beside {@code application.yml} in this module's resources. Gradle + * extracts them to a directory, so listing the parent is enough and keeps the test from + * hard-coding a profile list that a new profile would silently escape. + */ + private static File[] profileConfigurationFiles() throws IOException { + File resources = new ClassPathResource("application.yml").getFile().getParentFile(); + File[] profiles = resources.listFiles( + (directory, name) -> name.startsWith("application-") && name.endsWith(".yml")); + return profiles == null ? new File[0] : profiles; + } +} diff --git a/apps/worker/src/main/resources/application.yml b/apps/worker/src/main/resources/application.yml index 8d973183..f4704f3a 100644 --- a/apps/worker/src/main/resources/application.yml +++ b/apps/worker/src/main/resources/application.yml @@ -114,3 +114,15 @@ management: tracing: sampling: probability: ${ORGMEMORY_TRACING_SAMPLING_PROBABILITY:0.1} + +logging: + level: + # The provider libraries concatenate the prompt and the response content into + # WARN messages of their own, which `spring.ai.chat.observations.log-prompt` + # does not reach because it only governs Spring AI observation handlers. + # Every such site is guarded by isWarnEnabled(), so pinning these packages + # above WARN stops the payload from being built at all. Deliberately not + # environment-overridable: the payload-free boundary is not a per-deployment + # setting. See docs/runbooks/graph-rag-production-hardening.md. + org.springframework.ai.openai: ERROR + org.springframework.ai.anthropic: ERROR diff --git a/apps/worker/src/test/java/com/orgmemory/worker/observability/ProviderLoggingBoundaryTests.java b/apps/worker/src/test/java/com/orgmemory/worker/observability/ProviderLoggingBoundaryTests.java new file mode 100644 index 00000000..76ca6c49 --- /dev/null +++ b/apps/worker/src/test/java/com/orgmemory/worker/observability/ProviderLoggingBoundaryTests.java @@ -0,0 +1,100 @@ +package com.orgmemory.worker.observability; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.fail; + +import java.io.File; +import java.io.IOException; +import java.util.List; +import java.util.Locale; +import java.util.Map; +import java.util.Set; +import org.junit.jupiter.api.Test; +import org.springframework.boot.env.YamlPropertySourceLoader; +import org.springframework.core.env.PropertySource; +import org.springframework.core.io.ClassPathResource; +import org.springframework.core.io.FileSystemResource; + +/** + * The OpenAI and Anthropic client libraries build WARN messages that concatenate + * the prompt and the response content. {@code spring.ai.chat.observations.log-prompt} + * does not reach them because it only governs Spring AI observation handlers. + * + *

Every such call site is guarded by {@code isWarnEnabled()}, so the level + * configured for those packages is what keeps the payload from being built at all. + * The worker runs graph extraction over chunk content, so its prompts carry evidence + * text directly. + */ +class ProviderLoggingBoundaryTests { + + /** Packages whose own logging concatenates OrgMemory prompt or completion text. */ + private static final List PAYLOAD_LOGGING_PACKAGES = List.of( + "org.springframework.ai.openai", + "org.springframework.ai.anthropic"); + + /** Levels at or above ERROR; anything else lets a guarded WARN site build its message. */ + private static final Set LEVELS_ABOVE_WARN = Set.of("ERROR", "FATAL", "OFF"); + + @Test + void theBaseConfigurationPinsEveryPayloadLoggingPackageAboveWarn() throws IOException { + Map properties = load(new ClassPathResource("application.yml")); + + for (String payloadPackage : PAYLOAD_LOGGING_PACKAGES) { + // The loader returns origin-tracked char sequences rather than plain strings. + Object level = properties.get("logging.level." + payloadPackage); + assertEquals( + "ERROR", + String.valueOf(level), + () -> payloadPackage + + " must be pinned to ERROR in application.yml so its guarded WARN" + + " sites cannot concatenate the prompt"); + } + } + + @Test + void noProfileLowersAPayloadLoggingPackageBackToWarn() throws IOException { + File[] profiles = profileConfigurationFiles(); + assertTrue(profiles.length > 0, "no application-.yml was scanned, so this test proves nothing"); + + for (File profile : profiles) { + Map properties = load(new FileSystemResource(profile)); + + properties.forEach((key, value) -> { + if (!key.startsWith("logging.level.")) { + return; + } + String logger = key.substring("logging.level.".length()); + if (PAYLOAD_LOGGING_PACKAGES.stream().noneMatch(p -> p.equals(logger) || p.startsWith(logger + "."))) { + return; + } + assertTrue( + LEVELS_ABOVE_WARN.contains(String.valueOf(value).toUpperCase(Locale.ROOT)), + () -> profile.getName() + " sets " + key + "=" + value + + ", which re-enables the provider WARN sites that log the prompt"); + }); + } + } + + private static Map load(org.springframework.core.io.Resource resource) throws IOException { + List> sources = new YamlPropertySourceLoader().load(resource.getFilename(), resource); + if (sources.size() != 1) { + fail(resource.getFilename() + " must contain exactly one YAML document, found " + sources.size()); + } + @SuppressWarnings("unchecked") + Map properties = (Map) sources.getFirst().getSource(); + return properties; + } + + /** + * Profile files live beside {@code application.yml} in this module's resources. Gradle + * extracts them to a directory, so listing the parent is enough and keeps the test from + * hard-coding a profile list that a new profile would silently escape. + */ + private static File[] profileConfigurationFiles() throws IOException { + File resources = new ClassPathResource("application.yml").getFile().getParentFile(); + File[] profiles = resources.listFiles( + (directory, name) -> name.startsWith("application-") && name.endsWith(".yml")); + return profiles == null ? new File[0] : profiles; + } +} From cc7b5ee3a705c69e30d6db1f62704244080921e9 Mon Sep 17 00:00:00 2001 From: kl3inIT Date: Wed, 29 Jul 2026 22:53:46 +0700 Subject: [PATCH 3/8] fix(observability): strip exception text from every exported span Micrometer's OpenTelemetry bridge ends a failed span by calling recordException(throwable) and setStatus(ERROR, throwable.getMessage()), so the message and the stack trace travel with the span. An OrgMemory exception can be raised while holding a query, an evidence chunk or a provider response body, and nothing upstream can prove otherwise. The production hardening runbook already promises that error spans carry no exception event or stack trace; until now nothing enforced it, and the release gate could not have passed. ExceptionSanitizingSpanExporter wraps every exporter and drops all event attributes except exception.type, which is a class name fixed by the source, and clears the status description. Attribute counts keep their original values so a stripped attribute still reports as dropped rather than as never recorded. Micrometer's SpanFilter is the natural hook for the events but cannot reach the status description, which DelegatingSpanData does not expose as mutable. Wrapping the exporter covers both and runs after every filter, making it the last gate before egress. Spring Boot contributes its own SpanExporters only when one is missing, so declaring the collection ahead of it replaces every exporter with a wrapped copy; the ordering itself is asserted, because a wrong beforeName would silently hand the unwrapped collection back. There is no toggle. A payload guard a deployment can switch off is not a guard. Span attributes are deliberately left alone: their allowlist is a wider question than exception handling and belongs with the whole-export test. Co-Authored-By: Claude Opus 5 (1M context) --- .../graph-rag-observability/build.gradle.kts | 7 + .../ExceptionSanitizingSpanExporter.java | 136 ++++++++++++++++++ ...anExportSanitizationAutoConfiguration.java | 31 ++++ ...ot.autoconfigure.AutoConfiguration.imports | 1 + .../ExceptionSanitizingSpanExporterTests.java | 113 +++++++++++++++ ...ortSanitizationAutoConfigurationTests.java | 87 +++++++++++ 6 files changed, 375 insertions(+) create mode 100644 integrations/graph-rag-observability/src/main/java/com/orgmemory/integrations/graphrag/observability/ExceptionSanitizingSpanExporter.java create mode 100644 integrations/graph-rag-observability/src/main/java/com/orgmemory/integrations/graphrag/observability/SpanExportSanitizationAutoConfiguration.java create mode 100644 integrations/graph-rag-observability/src/test/java/com/orgmemory/integrations/graphrag/observability/ExceptionSanitizingSpanExporterTests.java create mode 100644 integrations/graph-rag-observability/src/test/java/com/orgmemory/integrations/graphrag/observability/SpanExportSanitizationAutoConfigurationTests.java diff --git a/integrations/graph-rag-observability/build.gradle.kts b/integrations/graph-rag-observability/build.gradle.kts index 2f45dc9d..499bd88a 100644 --- a/integrations/graph-rag-observability/build.gradle.kts +++ b/integrations/graph-rag-observability/build.gradle.kts @@ -5,10 +5,17 @@ plugins { dependencies { api(project(":components:graph-rag-core")) implementation("io.opentelemetry:opentelemetry-api") + implementation("io.opentelemetry:opentelemetry-sdk-trace") implementation("org.springframework.boot:spring-boot-autoconfigure") + // Supplies the SpanExporters collection this module replaces with a sanitized copy. + implementation("org.springframework.boot:spring-boot-micrometer-tracing-opentelemetry") testImplementation("io.opentelemetry:opentelemetry-sdk") testImplementation("io.opentelemetry:opentelemetry-sdk-testing") + // Drives the real bridge path this module sanitizes rather than a copy of it. + testImplementation("io.micrometer:micrometer-tracing-bridge-otel") testImplementation("org.junit.jupiter:junit-jupiter") + testImplementation("org.springframework.boot:spring-boot-test") + testImplementation("org.assertj:assertj-core") testRuntimeOnly("org.junit.platform:junit-platform-launcher") } diff --git a/integrations/graph-rag-observability/src/main/java/com/orgmemory/integrations/graphrag/observability/ExceptionSanitizingSpanExporter.java b/integrations/graph-rag-observability/src/main/java/com/orgmemory/integrations/graphrag/observability/ExceptionSanitizingSpanExporter.java new file mode 100644 index 00000000..6c93aa66 --- /dev/null +++ b/integrations/graph-rag-observability/src/main/java/com/orgmemory/integrations/graphrag/observability/ExceptionSanitizingSpanExporter.java @@ -0,0 +1,136 @@ +package com.orgmemory.integrations.graphrag.observability; + +import io.opentelemetry.api.common.AttributeKey; +import io.opentelemetry.api.common.Attributes; +import io.opentelemetry.api.common.AttributesBuilder; +import io.opentelemetry.sdk.common.CompletableResultCode; +import io.opentelemetry.sdk.trace.data.DelegatingSpanData; +import io.opentelemetry.sdk.trace.data.EventData; +import io.opentelemetry.sdk.trace.data.SpanData; +import io.opentelemetry.sdk.trace.data.StatusData; +import io.opentelemetry.sdk.trace.export.SpanExporter; +import java.util.ArrayList; +import java.util.Collection; +import java.util.List; +import java.util.Objects; +import java.util.Set; + +/** + * Removes exception text from every span on its way out of the process. + * + *

Micrometer's OpenTelemetry bridge ends an errored span by calling + * {@code recordException(throwable)} and {@code setStatus(ERROR, throwable.getMessage())}. + * Both carry an arbitrary message and a stack trace, and an OrgMemory exception can be + * raised while holding a query, an evidence chunk, a document identifier or a provider + * response body. Nothing upstream can prove otherwise, so the message and the stack trace + * are treated as payload and dropped. + * + *

The exception's type survives: it is a class name fixed by the source, it is the part + * an operator actually needs, and it cannot carry customer text. Attribute counts are left + * at their original values so a stripped attribute still shows up as dropped rather than as + * never recorded. + * + *

This exporter deliberately does not filter span attributes. Their allowlist is a wider + * question than exception handling, and pretending to cover it here would make a guarantee + * this class does not enforce. + * + *

Micrometer's {@code SpanFilter} is the natural hook for the events but cannot reach the + * status description, which {@code DelegatingSpanData} does not make mutable. Wrapping the + * exporter covers both, and it runs after every filter, so it is the last gate before egress. + */ +public final class ExceptionSanitizingSpanExporter implements SpanExporter { + + /** Event attributes that survive sanitization; everything else is treated as payload. */ + static final Set RETAINED_EVENT_ATTRIBUTES = Set.of("exception.type"); + + private final SpanExporter delegate; + + public ExceptionSanitizingSpanExporter(SpanExporter delegate) { + this.delegate = Objects.requireNonNull(delegate, "delegate"); + } + + @Override + public CompletableResultCode export(Collection spans) { + List sanitized = new ArrayList<>(spans.size()); + for (SpanData span : spans) { + sanitized.add(new SanitizedSpanData(span)); + } + return delegate.export(sanitized); + } + + @Override + public CompletableResultCode flush() { + return delegate.flush(); + } + + @Override + public CompletableResultCode shutdown() { + return delegate.shutdown(); + } + + @Override + public String toString() { + return "ExceptionSanitizingSpanExporter{" + delegate + "}"; + } + + private static final class SanitizedSpanData extends DelegatingSpanData { + + private final List events; + private final StatusData status; + + private SanitizedSpanData(SpanData delegate) { + super(delegate); + this.events = sanitizeEvents(delegate.getEvents()); + this.status = sanitizeStatus(delegate.getStatus()); + } + + @Override + public List getEvents() { + return events; + } + + @Override + public StatusData getStatus() { + return status; + } + + private static List sanitizeEvents(List events) { + List sanitized = new ArrayList<>(events.size()); + for (EventData event : events) { + sanitized.add(sanitizeEvent(event)); + } + return List.copyOf(sanitized); + } + + private static EventData sanitizeEvent(EventData event) { + Attributes retained = retainAllowedAttributes(event.getAttributes()); + if (retained.size() == event.getAttributes().size()) { + return event; + } + return EventData.create( + event.getEpochNanos(), + event.getName(), + retained, + event.getTotalAttributeCount()); + } + + private static Attributes retainAllowedAttributes(Attributes attributes) { + AttributesBuilder builder = Attributes.builder(); + attributes.forEach((key, value) -> { + if (RETAINED_EVENT_ATTRIBUTES.contains(key.getKey())) { + @SuppressWarnings("unchecked") + AttributeKey typedKey = (AttributeKey) key; + builder.put(typedKey, value); + } + }); + return builder.build(); + } + + private static StatusData sanitizeStatus(StatusData status) { + if (status.getDescription().isEmpty()) { + return status; + } + return StatusData.create(status.getStatusCode(), ""); + } + } +} diff --git a/integrations/graph-rag-observability/src/main/java/com/orgmemory/integrations/graphrag/observability/SpanExportSanitizationAutoConfiguration.java b/integrations/graph-rag-observability/src/main/java/com/orgmemory/integrations/graphrag/observability/SpanExportSanitizationAutoConfiguration.java new file mode 100644 index 00000000..bb1fa880 --- /dev/null +++ b/integrations/graph-rag-observability/src/main/java/com/orgmemory/integrations/graphrag/observability/SpanExportSanitizationAutoConfiguration.java @@ -0,0 +1,31 @@ +package com.orgmemory.integrations.graphrag.observability; + +import io.opentelemetry.sdk.trace.export.SpanExporter; +import org.springframework.beans.factory.ObjectProvider; +import org.springframework.boot.autoconfigure.AutoConfiguration; +import org.springframework.boot.autoconfigure.condition.ConditionalOnClass; +import org.springframework.boot.micrometer.tracing.opentelemetry.autoconfigure.SpanExporters; +import org.springframework.context.annotation.Bean; + +/** + * Puts {@link ExceptionSanitizingSpanExporter} in front of every span exporter. + * + *

Spring Boot contributes its own {@code SpanExporters} bean only when one is missing, so + * declaring this configuration ahead of it replaces the collection with a wrapped copy. Every + * exporter is covered, including ones added later, and there is no toggle: a payload guard + * that a deployment can switch off is not a guard. + */ +@AutoConfiguration( + beforeName = + "org.springframework.boot.micrometer.tracing.opentelemetry.autoconfigure" + + ".OpenTelemetryTracingAutoConfiguration") +@ConditionalOnClass({SpanExporters.class, SpanExporter.class}) +public class SpanExportSanitizationAutoConfiguration { + + @Bean + SpanExporters spanExporters(ObjectProvider spanExporters) { + return SpanExporters.of(spanExporters.orderedStream() + .map(ExceptionSanitizingSpanExporter::new) + .toList()); + } +} diff --git a/integrations/graph-rag-observability/src/main/resources/META-INF/spring/org.springframework.boot.autoconfigure.AutoConfiguration.imports b/integrations/graph-rag-observability/src/main/resources/META-INF/spring/org.springframework.boot.autoconfigure.AutoConfiguration.imports index 38153248..5870283c 100644 --- a/integrations/graph-rag-observability/src/main/resources/META-INF/spring/org.springframework.boot.autoconfigure.AutoConfiguration.imports +++ b/integrations/graph-rag-observability/src/main/resources/META-INF/spring/org.springframework.boot.autoconfigure.AutoConfiguration.imports @@ -1 +1,2 @@ com.orgmemory.integrations.graphrag.observability.GraphRagObservabilityAutoConfiguration +com.orgmemory.integrations.graphrag.observability.SpanExportSanitizationAutoConfiguration diff --git a/integrations/graph-rag-observability/src/test/java/com/orgmemory/integrations/graphrag/observability/ExceptionSanitizingSpanExporterTests.java b/integrations/graph-rag-observability/src/test/java/com/orgmemory/integrations/graphrag/observability/ExceptionSanitizingSpanExporterTests.java new file mode 100644 index 00000000..58263d2f --- /dev/null +++ b/integrations/graph-rag-observability/src/test/java/com/orgmemory/integrations/graphrag/observability/ExceptionSanitizingSpanExporterTests.java @@ -0,0 +1,113 @@ +package com.orgmemory.integrations.graphrag.observability; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import io.micrometer.tracing.otel.bridge.OtelSpan; +import io.opentelemetry.api.common.AttributeKey; +import io.opentelemetry.api.trace.StatusCode; +import io.opentelemetry.api.trace.Tracer; +import io.opentelemetry.sdk.testing.exporter.InMemorySpanExporter; +import io.opentelemetry.sdk.trace.SdkTracerProvider; +import io.opentelemetry.sdk.trace.data.EventData; +import io.opentelemetry.sdk.trace.data.SpanData; +import io.opentelemetry.sdk.trace.export.SimpleSpanProcessor; +import java.util.function.Consumer; +import org.junit.jupiter.api.Test; + +/** + * The message under test is one an OrgMemory exception could realistically carry: a query and + * an evidence excerpt concatenated into a diagnostic string. If any assertion here regresses, + * that text reaches whatever collector the deployment points at. + */ +class ExceptionSanitizingSpanExporterTests { + + private static final String PAYLOAD = + "no evidence for query 'quarterly revenue for ACME' in chunk 'ACME booked 4.2M in Q3'"; + + private static final AttributeKey EXCEPTION_TYPE = AttributeKey.stringKey("exception.type"); + private static final AttributeKey EXCEPTION_MESSAGE = AttributeKey.stringKey("exception.message"); + private static final AttributeKey EXCEPTION_STACKTRACE = + AttributeKey.stringKey("exception.stacktrace"); + + @Test + void stripsTheMessageAndStackTraceMicrometerRecordsWhenASpanFails() { + SpanData exported = export(span -> new OtelSpan(span).error(new IllegalStateException(PAYLOAD))); + + EventData exception = onlyEvent(exported); + assertEquals("exception", exception.getName()); + assertNull(exception.getAttributes().get(EXCEPTION_MESSAGE), "the exception message is payload"); + assertNull(exception.getAttributes().get(EXCEPTION_STACKTRACE), "the stack trace is payload"); + } + + @Test + void keepsTheExceptionTypeBecauseAClassNameCannotCarryCustomerText() { + SpanData exported = export(span -> new OtelSpan(span).error(new IllegalStateException(PAYLOAD))); + + assertEquals( + IllegalStateException.class.getName(), + onlyEvent(exported).getAttributes().get(EXCEPTION_TYPE)); + } + + @Test + void clearsTheStatusDescriptionMicrometerCopiesFromTheExceptionMessage() { + SpanData exported = export(span -> new OtelSpan(span).error(new IllegalStateException(PAYLOAD))); + + assertEquals(StatusCode.ERROR, exported.getStatus().getStatusCode(), "the failure itself must survive"); + assertEquals("", exported.getStatus().getDescription()); + } + + @Test + void leavesTheStrippedAttributesVisibleAsDroppedRatherThanNeverRecorded() { + SpanData exported = export(span -> new OtelSpan(span).error(new IllegalStateException(PAYLOAD))); + + EventData exception = onlyEvent(exported); + assertEquals(3, exception.getTotalAttributeCount(), "the original count is the honest one"); + assertEquals(2, exception.getDroppedAttributesCount()); + } + + @Test + void leavesASuccessfulSpanExactlyAsItWas() { + SpanData exported = export(span -> span.setAttribute("orgmemory.graph_rag.stage", "retrieve")); + + assertTrue(exported.getEvents().isEmpty()); + assertEquals("", exported.getStatus().getDescription()); + assertEquals( + "retrieve", + exported.getAttributes().get(AttributeKey.stringKey("orgmemory.graph_rag.stage")), + "sanitization must not disturb the payload-free attributes the pipeline exists to carry"); + } + + @Test + void refusesToLetAnUnmodelledEventAttributeThroughUninspected() { + SpanData exported = export(span -> span.addEvent( + "orgmemory.diagnostic", + io.opentelemetry.api.common.Attributes.of(AttributeKey.stringKey("note"), PAYLOAD))); + + EventData event = onlyEvent(exported); + assertEquals("orgmemory.diagnostic", event.getName(), "event names are code-derived and survive"); + assertNull(event.getAttributes().get(AttributeKey.stringKey("note"))); + } + + /** Runs one span through the real SDK and the exporter, and returns what the collector would see. */ + private static SpanData export(Consumer work) { + InMemorySpanExporter collector = InMemorySpanExporter.create(); + try (SdkTracerProvider provider = SdkTracerProvider.builder() + .addSpanProcessor(SimpleSpanProcessor.create(new ExceptionSanitizingSpanExporter(collector))) + .build()) { + Tracer tracer = provider.get("test"); + io.opentelemetry.api.trace.Span span = tracer.spanBuilder("test").startSpan(); + work.accept(span); + span.end(); + return collector.getFinishedSpanItems().getFirst(); + } + } + + private static EventData onlyEvent(SpanData span) { + assertFalse(span.getEvents().isEmpty(), "the event itself must survive so the failure stays visible"); + assertEquals(1, span.getEvents().size()); + return span.getEvents().getFirst(); + } +} diff --git a/integrations/graph-rag-observability/src/test/java/com/orgmemory/integrations/graphrag/observability/SpanExportSanitizationAutoConfigurationTests.java b/integrations/graph-rag-observability/src/test/java/com/orgmemory/integrations/graphrag/observability/SpanExportSanitizationAutoConfigurationTests.java new file mode 100644 index 00000000..2f4e2522 --- /dev/null +++ b/integrations/graph-rag-observability/src/test/java/com/orgmemory/integrations/graphrag/observability/SpanExportSanitizationAutoConfigurationTests.java @@ -0,0 +1,87 @@ +package com.orgmemory.integrations.graphrag.observability; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertInstanceOf; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import io.opentelemetry.sdk.testing.exporter.InMemorySpanExporter; +import io.opentelemetry.sdk.trace.export.SpanExporter; +import java.util.ArrayList; +import java.util.List; +import org.junit.jupiter.api.Test; +import org.springframework.boot.autoconfigure.AutoConfiguration; +import org.springframework.boot.autoconfigure.AutoConfigurations; +import org.springframework.boot.context.annotation.ImportCandidates; +import org.springframework.boot.micrometer.tracing.opentelemetry.autoconfigure.OpenTelemetryTracingAutoConfiguration; +import org.springframework.boot.micrometer.tracing.opentelemetry.autoconfigure.SpanExporters; +import org.springframework.boot.opentelemetry.autoconfigure.OpenTelemetrySdkAutoConfiguration; +import org.springframework.boot.test.context.runner.ApplicationContextRunner; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; + +/** + * The sanitizer only protects anything if it is actually in the export path. Spring Boot + * contributes its own unwrapped {@code SpanExporters} whenever one is missing, so the two + * questions are whether this module is discovered at all and whether it is ordered ahead of + * Boot's copy. + */ +class SpanExportSanitizationAutoConfigurationTests { + + private final ApplicationContextRunner runner = new ApplicationContextRunner() + .withConfiguration(AutoConfigurations.of(SpanExportSanitizationAutoConfiguration.class)) + .withUserConfiguration(ExporterConfiguration.class); + + @Test + void staysDiscoverableWithoutAnApplicationNamingIt() { + assertTrue( + registeredAutoConfigurations() + .contains(SpanExportSanitizationAutoConfiguration.class.getName()), + "META-INF/spring/…AutoConfiguration.imports no longer names this class, so spans " + + "would reach the collector with their exception messages intact"); + } + + @Test + void wrapsEveryExporterTheApplicationDeclares() { + runner.run(context -> { + List exporters = context.getBean(SpanExporters.class).list(); + assertEquals(2, exporters.size(), "no exporter may be dropped or left unwrapped"); + exporters.forEach(exporter -> assertInstanceOf(ExceptionSanitizingSpanExporter.class, exporter)); + }); + } + + @Test + void winsOverSpringBootsUnwrappedCollection() { + runner.withConfiguration(AutoConfigurations.of( + OpenTelemetrySdkAutoConfiguration.class, + OpenTelemetryTracingAutoConfiguration.class)) + .run(context -> context.getBean(SpanExporters.class) + .list() + .forEach(exporter -> assertInstanceOf( + ExceptionSanitizingSpanExporter.class, + exporter, + "the beforeName ordering no longer beats Boot's own SpanExporters bean"))); + } + + private static List registeredAutoConfigurations() { + List names = new ArrayList<>(); + ImportCandidates.load( + AutoConfiguration.class, + SpanExportSanitizationAutoConfigurationTests.class.getClassLoader()) + .forEach(names::add); + return names; + } + + @Configuration(proxyBeanMethods = false) + static class ExporterConfiguration { + + @Bean + SpanExporter firstExporter() { + return InMemorySpanExporter.create(); + } + + @Bean + SpanExporter secondExporter() { + return InMemorySpanExporter.create(); + } + } +} From f17256b3ddcca7198acd2d71ac7429956d1508a1 Mon Sep 17 00:00:00 2001 From: kl3inIT Date: Wed, 29 Jul 2026 22:54:02 +0700 Subject: [PATCH 4/8] fix(observability): make a failing telemetry sink visible MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every producer wraps emit() in a catch that discards the failure, which is the right call — an observability backend must never decide whether a retrieval or an indexing job succeeds. The cost was that a sink broken since startup looked exactly like a sink with nothing to report, across five call sites in GraphRagKnowledgeRetrievalService and one in GraphIndexingProcessor. FailureTolerantGraphRagEventSink keeps the absorption and adds the signal that was missing: a running count, the type of the most recent failure, and one log line each time the failure changes kind, so a permanently broken sink cannot flood the log at event rate while one that starts failing differently still says so. Only class names are recorded. A telemetry backend's exception message can quote the request it failed to send, so the message and the stack trace stay out on the same grounds as any other payload. Logging goes through System.Logger, so components/graph-rag-core keeps its property of having no runtime dependencies; Boot's JUL bridge routes it into the application log. The wrapper is applied where the composite is built, so production always gets it. The existing call-site catches stay as defence in depth for anyone constructing the service with a raw sink. Co-Authored-By: Claude Opus 5 (1M context) --- .../worker/graph/GraphIndexingProcessor.java | 3 +- .../FailureTolerantGraphRagEventSink.java | 78 +++++++++++++++++ .../observability/GraphRagEventSink.java | 10 +++ ...FailureTolerantGraphRagEventSinkTests.java | 84 +++++++++++++++++++ ...aphRagKnowledgeRetrievalConfiguration.java | 3 +- 5 files changed, 176 insertions(+), 2 deletions(-) create mode 100644 components/graph-rag-core/src/main/java/com/orgmemory/graphrag/observability/FailureTolerantGraphRagEventSink.java create mode 100644 components/graph-rag-core/src/test/java/com/orgmemory/graphrag/observability/FailureTolerantGraphRagEventSinkTests.java diff --git a/apps/worker/src/main/java/com/orgmemory/worker/graph/GraphIndexingProcessor.java b/apps/worker/src/main/java/com/orgmemory/worker/graph/GraphIndexingProcessor.java index 5a3b4c4a..1c89412f 100644 --- a/apps/worker/src/main/java/com/orgmemory/worker/graph/GraphIndexingProcessor.java +++ b/apps/worker/src/main/java/com/orgmemory/worker/graph/GraphIndexingProcessor.java @@ -79,7 +79,8 @@ class GraphIndexingProcessor { embeddingModels, routes, properties, - GraphRagEventSink.composite(eventSinks.orderedStream().toList())); + GraphRagEventSink.failureTolerant( + GraphRagEventSink.composite(eventSinks.orderedStream().toList()))); } GraphIndexingProcessor( diff --git a/components/graph-rag-core/src/main/java/com/orgmemory/graphrag/observability/FailureTolerantGraphRagEventSink.java b/components/graph-rag-core/src/main/java/com/orgmemory/graphrag/observability/FailureTolerantGraphRagEventSink.java new file mode 100644 index 00000000..7e51973e --- /dev/null +++ b/components/graph-rag-core/src/main/java/com/orgmemory/graphrag/observability/FailureTolerantGraphRagEventSink.java @@ -0,0 +1,78 @@ +package com.orgmemory.graphrag.observability; + +import java.lang.System.Logger; +import java.util.Objects; +import java.util.concurrent.atomic.AtomicLong; +import java.util.concurrent.atomic.AtomicReference; + +/** + * Absorbs a failing telemetry backend without letting the failure disappear. + * + *

Producers already treat emission as non-critical and catch whatever the sink + * throws, which is correct — an observability backend must never decide whether a + * retrieval or an indexing job succeeds. The cost is that a sink broken since + * startup looks exactly like a sink with nothing to report. This wrapper keeps the + * absorption and adds the signal that was missing: a running count, the type of the + * most recent failure, and one log line each time the failure changes kind. + * + *

Only class names are recorded. A telemetry backend's exception message can + * quote the request it failed to send, so the message and the stack trace are + * treated the same way as any other payload and never leave this class. + * + *

Logging goes through {@link System.Logger} so that + * {@code components/graph-rag-core} keeps its property of having no runtime + * dependencies; Spring Boot's JUL bridge routes it into the application log. + */ +public final class FailureTolerantGraphRagEventSink implements GraphRagEventSink { + + private static final Logger LOGGER = + System.getLogger(FailureTolerantGraphRagEventSink.class.getName()); + + private final GraphRagEventSink delegate; + private final AtomicLong swallowedFailures = new AtomicLong(); + private final AtomicReference lastFailureType = new AtomicReference<>(); + + FailureTolerantGraphRagEventSink(GraphRagEventSink delegate) { + this.delegate = Objects.requireNonNull(delegate, "delegate"); + } + + @Override + public void emit(GraphRagEvent event) { + try { + delegate.emit(event); + } catch (RuntimeException failure) { + record(failure); + } + } + + /** How many events this sink has dropped since startup. Never resets. */ + public long swallowedFailureCount() { + return swallowedFailures.get(); + } + + /** Class name of the most recent failure, or {@code null} while nothing has failed. */ + public String lastFailureType() { + return lastFailureType.get(); + } + + private void record(RuntimeException failure) { + swallowedFailures.incrementAndGet(); + String failureType = failure.getClass().getName(); + // One line per change of kind: a permanently broken sink must not flood the + // log at event rate, but a sink that starts failing differently must say so. + if (!failureType.equals(lastFailureType.getAndSet(failureType))) { + LOGGER.log( + Logger.Level.WARNING, + "GraphRAG telemetry sink {0} is failing with {1}; events are being dropped." + + " Message and stack trace are withheld because a telemetry failure" + + " can quote the event it could not send.", + delegate.getClass().getName(), + failureType); + } + } + + @Override + public String toString() { + return "FailureTolerantGraphRagEventSink{" + delegate + "}"; + } +} diff --git a/components/graph-rag-core/src/main/java/com/orgmemory/graphrag/observability/GraphRagEventSink.java b/components/graph-rag-core/src/main/java/com/orgmemory/graphrag/observability/GraphRagEventSink.java index 885cebf0..cd2f2d63 100644 --- a/components/graph-rag-core/src/main/java/com/orgmemory/graphrag/observability/GraphRagEventSink.java +++ b/components/graph-rag-core/src/main/java/com/orgmemory/graphrag/observability/GraphRagEventSink.java @@ -60,6 +60,16 @@ static GraphRagEventSink composite(List sinks) { }; } + /** + * Wraps a sink so an emission failure is absorbed, counted and reported + * instead of being silently discarded at every call site. + * + * @see FailureTolerantGraphRagEventSink + */ + static FailureTolerantGraphRagEventSink failureTolerant(GraphRagEventSink sink) { + return new FailureTolerantGraphRagEventSink(sink); + } + record GraphRagEvent( UUID operationId, UUID organizationId, diff --git a/components/graph-rag-core/src/test/java/com/orgmemory/graphrag/observability/FailureTolerantGraphRagEventSinkTests.java b/components/graph-rag-core/src/test/java/com/orgmemory/graphrag/observability/FailureTolerantGraphRagEventSinkTests.java new file mode 100644 index 00000000..2ec02283 --- /dev/null +++ b/components/graph-rag-core/src/test/java/com/orgmemory/graphrag/observability/FailureTolerantGraphRagEventSinkTests.java @@ -0,0 +1,84 @@ +package com.orgmemory.graphrag.observability; + +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNull; + +import java.time.Duration; +import java.time.Instant; +import java.util.UUID; +import org.junit.jupiter.api.Test; + +/** + * Before this wrapper existed, every producer caught the sink's failure and dropped it, so a + * backend that had been failing since startup was indistinguishable from a quiet one. These + * tests hold both halves of the fix in place: the work still succeeds, and the failure is + * still countable afterwards. + */ +class FailureTolerantGraphRagEventSinkTests { + + @Test + void letsTheObservedWorkSucceedWhenTheBackendIsDown() { + var sink = GraphRagEventSink.failureTolerant(event -> { + throw new IllegalStateException("collector down"); + }); + + assertDoesNotThrow(() -> sink.emit(event())); + } + + @Test + void countsEveryDroppedEventSoABrokenBackendStopsBeingInvisible() { + var sink = GraphRagEventSink.failureTolerant(event -> { + throw new IllegalStateException("collector down"); + }); + + sink.emit(event()); + sink.emit(event()); + sink.emit(event()); + + assertEquals(3, sink.swallowedFailureCount()); + assertEquals(IllegalStateException.class.getName(), sink.lastFailureType()); + } + + @Test + void reportsNothingWhileTheBackendIsHealthy() { + var received = new java.util.ArrayList(); + var sink = GraphRagEventSink.failureTolerant(received::add); + + sink.emit(event()); + + assertEquals(1, received.size(), "a healthy backend must still receive the event"); + assertEquals(0, sink.swallowedFailureCount()); + assertNull(sink.lastFailureType()); + } + + @Test + void namesOnlyTheFailureTypeSoATelemetryErrorCannotQuoteTheEventItCouldNotSend() { + var sink = GraphRagEventSink.failureTolerant(event -> { + throw new IllegalStateException( + "POST /v1/traces failed for query 'quarterly revenue for ACME'"); + }); + + sink.emit(event()); + + assertEquals(IllegalStateException.class.getName(), sink.lastFailureType()); + assertFalse(sink.lastFailureType().contains("ACME")); + } + + private static GraphRagEventSink.GraphRagEvent event() { + return new GraphRagEventSink.GraphRagEvent( + UUID.randomUUID(), + UUID.randomUUID(), + GraphRagEventSink.Stage.RETRIEVE, + GraphRagEventSink.Outcome.SUCCEEDED, + Duration.ofMillis(5), + 1, + 1, + null, + null, + null, + null, + Instant.now()); + } +} diff --git a/core/src/main/java/com/orgmemory/core/knowledge/GraphRagKnowledgeRetrievalConfiguration.java b/core/src/main/java/com/orgmemory/core/knowledge/GraphRagKnowledgeRetrievalConfiguration.java index aa970e14..349063ff 100644 --- a/core/src/main/java/com/orgmemory/core/knowledge/GraphRagKnowledgeRetrievalConfiguration.java +++ b/core/src/main/java/com/orgmemory/core/knowledge/GraphRagKnowledgeRetrievalConfiguration.java @@ -40,6 +40,7 @@ GraphRagKnowledgeRetrievalService graphRagKnowledgeRetrievalService( policy, audit, retrievalProperties, - GraphRagEventSink.composite(eventSinks.orderedStream().toList())); + GraphRagEventSink.failureTolerant( + GraphRagEventSink.composite(eventSinks.orderedStream().toList()))); } } From 8aec3dfeb70c721985ffd9317e3276a3c98f4eb7 Mon Sep 17 00:00:00 2001 From: kl3inIT Date: Wed, 29 Jul 2026 22:55:25 +0700 Subject: [PATCH 5/8] docs: record LightRAG's tracing model and the stage coverage it exposes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The semantic port came from LightRAG, so its tracing was read from the pinned v1.5.4 checkout rather than from recollection. An earlier note in this increment said it has no observability; that was wrong. It has exactly one integration: lightrag/llm/openai.py swaps openai.AsyncOpenAI for langfuse.openai.AsyncOpenAI when both Langfuse keys are set, which captures requests and responses verbatim. No other provider binding is traced, and there is no masking, no OpenTelemetry, no metrics and no spans over its own stages. The upstream therefore does what this increment's decision rejects, through a vendor drop-in rather than a designed boundary — which supports the finding that "no comparable system chose never" reflects an absence of deliberation. The comparison also surfaced a gap on this side. Stage declares fourteen values and production emits ten: PARSE, CHUNK, GLEAN and GENERATE have no producer outside tests, so a stage-grouped dashboard would carry four empty series and no parsing, chunking, gleaning or generation latency. Deletion and rebuild are absent from the enum altogether, though LightRAG tracks that path and the hardening runbook requires a drill for it. Both are now phase 2 work. Phase 0 is recorded as code-complete apart from the production log search, which needs the owner's go-ahead because it may surface customer content. Co-Authored-By: Claude Opus 5 (1M context) --- .../design.md | 39 ++++++++++++++ .../2026-07-29-observability-pipeline/plan.md | 53 ++++++++++++++----- 2 files changed, 78 insertions(+), 14 deletions(-) diff --git a/docs/increments/active/2026-07-29-observability-pipeline/design.md b/docs/increments/active/2026-07-29-observability-pipeline/design.md index 3148b37f..2579048e 100644 --- a/docs/increments/active/2026-07-29-observability-pipeline/design.md +++ b/docs/increments/active/2026-07-29-observability-pipeline/design.md @@ -149,6 +149,45 @@ The policy is unchanged but the repository currently claims more than it enforces. The bypasses above must close before any release describes the deployment as payload-free. +## What LightRAG instruments, and what it does not + +The semantic port came from LightRAG, so its tracing was checked directly against +the pinned `v1.5.4` checkout rather than from recollection. An earlier reading in +this increment recorded that it has no observability at all; that was wrong. + +Its only instrumentation is Langfuse, and only for the OpenAI-compatible binding. +`lightrag/llm/openai.py:44-66` swaps `openai.AsyncOpenAI` for +`langfuse.openai.AsyncOpenAI` when `LANGFUSE_PUBLIC_KEY` and +`LANGFUSE_SECRET_KEY` are both present at import time. That drop-in captures the +full request and response, so enabling it exports prompts and completions +verbatim. `git grep -ln langfuse -- 'lightrag/llm/*.py'` returns that one file: +Anthropic, Gemini, Bedrock, Ollama and the rest are not traced at all. There is +no masking, redaction or scrubbing anywhere in the tree, no OpenTelemetry, no +metrics, and no spans over LightRAG's own pipeline stages. + +Two consequences. + +First, the upstream does precisely what the decision above rejects, at LLM-call +granularity, and reaches it through a vendor drop-in rather than a designed +boundary. It strengthens rather than weakens the finding that +"no comparable system chose never" describes an absence of deliberation, not a +considered industry position. It is not a precedent to adopt. + +Second, LightRAG is no source of stage coverage, because it instruments none. +Comparing the two pipelines instead surfaced a gap on this side: +`GraphRagEventSink.Stage` declares fourteen stages and production emits ten. +`PARSE`, `CHUNK`, `GLEAN` and `GENERATE` have no producer outside tests, so a +dashboard grouped by stage would show four permanently empty series and no +parsing, chunking, gleaning or answer-generation latency at all. Parsing and +chunking happen in the ingestion pipeline, which holds no sink; gleaning runs +inside extraction and is folded into it; generation is never reported, because +retrieval stops at `ASSEMBLE_CONTEXT`. + +Deletion and rebuild are absent from both the enum and the producers. LightRAG +tracks that path through `pipeline_status`, and the hardening runbook requires a +deletion-then-rebuild drill, so it is the one stage the comparison says is +missing outright rather than merely unwired. + ## Pipeline architecture - **Push over pull.** OTLP export is already wired and the worker has no HTTP diff --git a/docs/increments/active/2026-07-29-observability-pipeline/plan.md b/docs/increments/active/2026-07-29-observability-pipeline/plan.md index aaa976a5..d6cf3c9c 100644 --- a/docs/increments/active/2026-07-29-observability-pipeline/plan.md +++ b/docs/increments/active/2026-07-29-observability-pipeline/plan.md @@ -1,22 +1,31 @@ # Observability pipeline plan -## 0. Close the payload bypasses — not started +## 0. Close the payload bypasses — code done, production evidence outstanding Highest priority. These are live paths, independent of the pipeline work. -- [ ] Pin `org.springframework.ai.openai` and `org.springframework.ai.anthropic` - logger levels to `ERROR` in API and worker production configuration, so the - provider WARN paths that concatenate the prompt cannot emit. -- [ ] Search retained production logs for those three WARN signatures. Treat a - hit as a data incident, not a defect: record scope and affected - organizations before changing anything. -- [ ] Decide how `OtelSpan.error` is handled. Either suppress exception recording - for observations that can carry OrgMemory content, or amend the runbook - gate that currently cannot pass. -- [ ] Add a payload-free health signal for swallowed `GraphRagEventSink` - failures. Producers currently catch and ignore them - (`GraphRagKnowledgeRetrievalService:149-172,565-610,630-646,772-788`, - `GraphIndexingProcessor:249-265`), so a broken sink is invisible. +- [x] Pin `org.springframework.ai.openai` and `org.springframework.ai.anthropic` + logger levels to `ERROR`. Done in the base configuration of both apps + rather than the production profile, because a development database holds + real uploaded documents too, and without an environment override, because + the boundary is not a per-deployment setting. `ProviderLoggingBoundaryTests` + in each app reads the shipped YAML and fails if a profile lowers either + package. A fourth site was found during the sweep, + `AnthropicChatModel:1018`, which logs a response content block. +- [ ] Search retained production logs for those WARN signatures. Treat a hit as a + data incident, not a defect: record scope and affected organizations before + changing anything. **Needs the owner's go-ahead — it runs on production and + may surface customer content.** +- [x] Decide how `OtelSpan.error` is handled. Suppressed, not tolerated: + `ExceptionSanitizingSpanExporter` drops every event attribute except + `exception.type` and clears the status description, as the last gate before + egress. The runbook's wording still needs the amendment in phase 5, because + the event itself now survives with its type. +- [x] Add a payload-free health signal for swallowed `GraphRagEventSink` + failures. `FailureTolerantGraphRagEventSink` counts them, records the + failure type and logs once per change of kind. Publishing that count as a + metric belongs with the Micrometer sink in phase 2; until then the signal + is the log line. ## 1. Silence the unconfigured exporter — not started @@ -45,6 +54,14 @@ Depends on the composite sink merged in PR #132. - [ ] Export `ContextTokenUsage`, which core already computes and nothing publishes, plus a counter for `finish_reason=length` truncation. - [ ] Time to first token on the streaming assistant path. +- [ ] Publish `FailureTolerantGraphRagEventSink.swallowedFailureCount()` as a + counter, so a broken sink is a number rather than only a log line. +- [ ] Close the stage gap the LightRAG comparison surfaced. `Stage` declares + fourteen and production emits ten: `PARSE` and `CHUNK` need a sink in the + ingestion pipeline, `GLEAN` needs separating from extraction, and + `GENERATE` needs emitting where retrieval currently stops at + `ASSEMBLE_CONTEXT`. Decide deletion and rebuild separately — it is missing + from the enum entirely and the runbook requires a drill for it. - [ ] Tests: both sinks enabled receive the event; both disabled compose to `NO_OP`. @@ -85,6 +102,14 @@ Highest regression risk; run it last, once a collector can show the result. - [ ] Correct the runbook's field list — it omits `scopeFingerprint`, `cacheStatus` and `occurredAt`, and describes counts as bounded when only non-negativity is checked. +- [ ] Amend the runbook's "no exception event or stack trace" gate. The event now + survives carrying `exception.type` alone, which is what the exporter + enforces; the gate should say that rather than something stricter than the + code. +- [ ] Resolve where telemetry egress lives. `integrations/graph-rag-observability` + is named for one domain but now owns the span sanitizer, which protects + every span in the process. Renaming it is a module-boundary change and + needs its own challenge, so it is recorded here rather than done quietly. - [ ] Record the payload-boundary decision under `docs/decisions/`, superseding nothing but documenting the challenge outcome. From 04d364c32134f477a6a60c2e3e3cb3d2d6882c82 Mon Sep 17 00:00:00 2001 From: kl3inIT Date: Wed, 29 Jul 2026 22:59:11 +0700 Subject: [PATCH 6/8] docs: reconcile secure-graph-rag with the payload boundary changes The spec's source paths cover components/graph-rag-core, integrations/graph-rag-*, the worker graph package and core/knowledge, all of which moved in this change, so reconciliation belongs here rather than at the end of the increment. Records the span sanitizer, the failure-tolerant sink wrapper and the provider logger pin as current behaviour, and states plainly what they do not cover: span attributes are unfiltered, and no test asserts the whole export against the allowlist, so "payload-free" is enforced at named points rather than proven end to end. Also records that Stage declares fourteen values while production emits ten, and that deletion and rebuild have no stage at all. Co-Authored-By: Claude Opus 5 (1M context) --- docs/specs/domains/secure-graph-rag.md | 21 +++++++++++++++++++-- docs/tests/domains/secure-graph-rag.md | 25 ++++++++++++++++++++++++- 2 files changed, 43 insertions(+), 3 deletions(-) diff --git a/docs/specs/domains/secure-graph-rag.md b/docs/specs/domains/secure-graph-rag.md index 9f6ece89..018591af 100644 --- a/docs/specs/domains/secure-graph-rag.md +++ b/docs/specs/domains/secure-graph-rag.md @@ -5,7 +5,7 @@ Source: `components/graph-rag-core`, `components/graph-rag-testkit`, `core/src/main/java/com/orgmemory/core/knowledge`, and `apps/web/src/features/knowledge`. -Reconciled: `2026-07-29-graph-rag-observability-wiring (fd495d0)`. +Reconciled: `2026-07-29-observability-pipeline (f17256b)`. ## Current Contract @@ -140,10 +140,27 @@ Reconciled: `2026-07-29-graph-rag-observability-wiring (fd495d0)`. - Payload-free OpenTelemetry stages separate keyword planning/cache status, embedding, hashed per-snapshot retrieval, consolidation, authorization and provider-only reranking duration. +- `Stage` declares fourteen values; production emits ten. `PARSE`, `CHUNK`, + `GLEAN` and `GENERATE` have no producer, so no parsing, chunking, gleaning or + answer-generation latency is reported. Deletion and rebuild have no stage. - Indexing and retrieval fan one stage event out to every registered `GraphRagEventSink`, so an application may observe the same stage through more than one backend. Sinks fail independently and emission never controls - indexing or retrieval availability. + indexing or retrieval availability. Both composition sites wrap the composite + in `FailureTolerantGraphRagEventSink`, which absorbs an emission failure while + counting it, recording its type and logging once per change of kind, so a + backend broken since startup is distinguishable from a quiet one. Only class + names are recorded. +- Every exported span passes through `ExceptionSanitizingSpanExporter` before + leaving the process. It keeps `exception.type` alone from each event and clears + the status description, because Micrometer's bridge copies + `throwable.getMessage()` into both and an OrgMemory exception can be raised + holding query, evidence or provider-response text. It applies to all spans, not + only GraphRAG ones, and has no toggle. Span attributes are not filtered. +- The OpenAI and Anthropic client packages are pinned above WARN in both apps, + without an environment override, because their own logging concatenates the + prompt or the response content into messages that Spring AI's + `log-prompt`/`log-completion` settings do not govern. ## Graph Explorer diff --git a/docs/tests/domains/secure-graph-rag.md b/docs/tests/domains/secure-graph-rag.md index c2c742e1..58460038 100644 --- a/docs/tests/domains/secure-graph-rag.md +++ b/docs/tests/domains/secure-graph-rag.md @@ -6,7 +6,7 @@ Source: `components/graph-rag-core/src/test`, `core/src/test/java/com/orgmemory/core/knowledge`, and `apps/web/test/e2e`. -Reconciled: `2026-07-29-graph-rag-observability-wiring (fd495d0)`. +Reconciled: `2026-07-29-observability-pipeline (f17256b)`. ## Automated @@ -55,6 +55,22 @@ Reconciled: `2026-07-29-graph-rag-observability-wiring (fd495d0)`. failure, that the first failure is the one propagated with later ones suppressed, that two backends raising one shared failure instance do not trip self-suppression, and that an application with no backend emits nothing. +- Failure-tolerance tests prove the observed work still succeeds while the + backend is down, that every dropped event is counted, that a healthy backend + still receives the event and reports nothing, and that only the failure's class + name is retained when its message quotes the query it could not send. +- Span sanitization tests drive Micrometer's real `OtelSpan.error` through the + SDK and prove the exported span keeps `exception.type` and the error status + while losing the message, the stack trace and the status description, that the + stripped attributes still report as dropped rather than as never recorded, that + a successful span is untouched, and that an unmodelled event attribute does not + pass. Wiring tests prove the auto-configuration stays discoverable, wraps every + declared exporter, and is ordered ahead of Spring Boot's unwrapped collection. +- Provider-logging tests in each app read the shipped `application.yml` and fail + if the pin is absent or if any profile lowers `org.springframework.ai.openai` + or `org.springframework.ai.anthropic` back to WARN. They assert that at least + one profile file was scanned, so an unreadable resource directory cannot make + them pass vacuously. - Storage adapter auto-configuration tests prove PostgreSQL, OpenSearch and Neo4j stay discoverable through their registration files, that PostgreSQL owns the canonical ports without an opt-in, that OpenSearch and Neo4j claim no port @@ -87,6 +103,13 @@ The exact graph node/edge response and permission-negative metadata contract is covered at the API/service layer; a focused real-browser graph rendering and interaction test remains a gap. +No test asserts the whole telemetry export against the payload allowlist. Span +attributes, resource attributes, instrumentation-scope attributes and the log +stream are each unchecked; only the GraphRAG adapter's own attribute set, the +exception paths and the provider logger levels are covered. Until that test +exists, "payload-free" is enforced at the points listed above rather than proven +end to end. + `PostgresAuthorizedGraphSqlTests.graphVisibilityUsesTheLatestSealedCompleteAclAfterFreshnessExpiry` pins ADR 0015 parity: GraphRAG requires the current sealed `COMPLETE` ACL but does not add an expiry denial absent from canonical knowledge retrieval. From eb01be7775e7ff25f2ab12479694c2578b4cc3c2 Mon Sep 17 00:00:00 2001 From: kl3inIT Date: Wed, 29 Jul 2026 23:02:40 +0700 Subject: [PATCH 7/8] docs: close the historical log question for the POC deployment The provider logging fix is preventive, not remedial. The project owner confirmed on 2026-07-29 that the ZM deployment is a proof of concept with no real users and no customer data, so the retained logs hold no exposure to scope and the planned production log search has nothing to find. Recorded as an attributable statement about today's deployment rather than about the design, because the same code against a deployment holding real evidence would have been writing customer text to the host since it shipped. Co-Authored-By: Claude Opus 5 (1M context) --- .../active/2026-07-29-observability-pipeline/design.md | 7 +++++++ .../active/2026-07-29-observability-pipeline/plan.md | 10 ++++++---- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/docs/increments/active/2026-07-29-observability-pipeline/design.md b/docs/increments/active/2026-07-29-observability-pipeline/design.md index 2579048e..d4249439 100644 --- a/docs/increments/active/2026-07-29-observability-pipeline/design.md +++ b/docs/increments/active/2026-07-29-observability-pipeline/design.md @@ -149,6 +149,13 @@ The policy is unchanged but the repository currently claims more than it enforces. The bypasses above must close before any release describes the deployment as payload-free. +Nothing has leaked yet. The project owner confirmed on 2026-07-29 that the ZM +deployment is a proof of concept carrying no real users and no customer data, so +the retained logs hold no exposure to scope and closing the bypasses is +preventive. That is a statement about today's deployment, not about the design: +the same code against a deployment holding real evidence would have been writing +customer text to the host since it shipped. + ## What LightRAG instruments, and what it does not The semantic port came from LightRAG, so its tracing was checked directly against diff --git a/docs/increments/active/2026-07-29-observability-pipeline/plan.md b/docs/increments/active/2026-07-29-observability-pipeline/plan.md index d6cf3c9c..66a0c928 100644 --- a/docs/increments/active/2026-07-29-observability-pipeline/plan.md +++ b/docs/increments/active/2026-07-29-observability-pipeline/plan.md @@ -12,10 +12,12 @@ Highest priority. These are live paths, independent of the pipeline work. in each app reads the shipped YAML and fails if a profile lowers either package. A fourth site was found during the sweep, `AnthropicChatModel:1018`, which logs a response content block. -- [ ] Search retained production logs for those WARN signatures. Treat a hit as a - data incident, not a defect: record scope and affected organizations before - changing anything. **Needs the owner's go-ahead — it runs on production and - may surface customer content.** +- [x] Search retained production logs for those WARN signatures. Not required: + the project owner confirmed on 2026-07-29 that the ZM deployment is a proof + of concept with no real users and no customer data, so there is no + historical exposure to scope and the fix is preventive rather than remedial. + If that ever stops being true before the fix ships to a deployment holding + real data, the search becomes a data-incident question again. - [x] Decide how `OtelSpan.error` is handled. Suppressed, not tolerated: `ExceptionSanitizingSpanExporter` drops every event attribute except `exception.type` and clears the status description, as the last gate before From 77aa27655f1192b46b67e513f0bd6a481c32d1ee Mon Sep 17 00:00:00 2001 From: kl3inIT Date: Wed, 29 Jul 2026 23:44:49 +0700 Subject: [PATCH 8/8] fix(observability): enforce the provider logging boundary at runtime MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses three review findings, one of which corrects a claim this branch made about its own change. **The YAML pin is a default, not enforcement.** The previous commit described it as "deliberately not environment-overridable". That was wrong: LOGGING_LEVEL_ORG_SPRINGFRAMEWORK_AI_OPENAI, a system property, a logback configuration, or a level on a more specific logger all outrank application.yml. Configuration cannot enforce a boundary configuration can undo. ProviderLoggingBoundaryVerifier therefore checks the resolved state instead of any source of it: each class holding a payload-logging call site is asked whether WARN is enabled on its own logger — the exact question its isWarnEnabled() guard will ask, with inheritance already applied — and startup fails if any answers yes. Failing beats starting, because the alternative is an application that looks healthy while writing customer text to disk. No disable property. **The sink log could still flood.** Reporting on a change of kind meant a backend alternating between two exceptions logged at event rate, which a timeout retrying as a connection failure produces immediately. Reporting is now keyed on the set of kinds seen, bounded at ten, with an A->B->A test. **The profile scan had two holes.** It skipped `.yaml` files, and it matched only ancestors of the guarded packages, so a child such as `org.springframework.ai.openai.api: WARN` under an ERROR parent passed. The YAML comments and test javadoc that repeated the overclaim now say plainly that they cover the shipped default and point at the verifier for enforcement. Co-Authored-By: Claude Opus 5 (1M context) --- apps/api/src/main/resources/application.yml | 10 +- .../ProviderLoggingBoundaryTests.java | 19 ++- .../worker/src/main/resources/application.yml | 10 +- .../ProviderLoggingBoundaryTests.java | 19 ++- .../FailureTolerantGraphRagEventSink.java | 28 +++- ...FailureTolerantGraphRagEventSinkTests.java | 22 +++ docs/specs/domains/secure-graph-rag.md | 11 +- docs/tests/domains/secure-graph-rag.md | 15 +- .../graph-rag-observability/build.gradle.kts | 6 + ...viderLoggingBoundaryAutoConfiguration.java | 22 +++ .../ProviderLoggingBoundaryVerifier.java | 55 ++++++++ ...ot.autoconfigure.AutoConfiguration.imports | 1 + .../ProviderLoggingBoundaryVerifierTests.java | 132 ++++++++++++++++++ 13 files changed, 318 insertions(+), 32 deletions(-) create mode 100644 integrations/graph-rag-observability/src/main/java/com/orgmemory/integrations/graphrag/observability/ProviderLoggingBoundaryAutoConfiguration.java create mode 100644 integrations/graph-rag-observability/src/main/java/com/orgmemory/integrations/graphrag/observability/ProviderLoggingBoundaryVerifier.java create mode 100644 integrations/graph-rag-observability/src/test/java/com/orgmemory/integrations/graphrag/observability/ProviderLoggingBoundaryVerifierTests.java diff --git a/apps/api/src/main/resources/application.yml b/apps/api/src/main/resources/application.yml index b92b37b3..da652bb3 100644 --- a/apps/api/src/main/resources/application.yml +++ b/apps/api/src/main/resources/application.yml @@ -186,8 +186,12 @@ logging: # WARN messages of their own, which `spring.ai.chat.observations.log-prompt` # does not reach because it only governs Spring AI observation handlers. # Every such site is guarded by isWarnEnabled(), so pinning these packages - # above WARN stops the payload from being built at all. Deliberately not - # environment-overridable: the payload-free boundary is not a per-deployment - # setting. See docs/runbooks/graph-rag-production-hardening.md. + # above WARN stops the payload from being built at all. + # + # This is the safe default, not the enforcement: LOGGING_LEVEL_* environment + # variables, system properties and logback configuration all outrank it. + # ProviderLoggingBoundaryVerifier checks the resolved level at startup and + # refuses to run with these open. + # See docs/runbooks/graph-rag-production-hardening.md. org.springframework.ai.openai: ERROR org.springframework.ai.anthropic: ERROR diff --git a/apps/api/src/test/java/com/orgmemory/api/observability/ProviderLoggingBoundaryTests.java b/apps/api/src/test/java/com/orgmemory/api/observability/ProviderLoggingBoundaryTests.java index 8be03d31..f69ad500 100644 --- a/apps/api/src/test/java/com/orgmemory/api/observability/ProviderLoggingBoundaryTests.java +++ b/apps/api/src/test/java/com/orgmemory/api/observability/ProviderLoggingBoundaryTests.java @@ -21,10 +21,11 @@ * the prompt and the response content. {@code spring.ai.chat.observations.log-prompt} * does not reach them because it only governs Spring AI observation handlers. * - *

Every such call site is guarded by {@code isWarnEnabled()}, so the level - * configured for those packages is what keeps the payload from being built at all. - * That makes the configured level part of the payload-free boundary rather than an - * operator preference, and this test is what stops it from drifting. + *

Every such call site is guarded by {@code isWarnEnabled()}, so the level decides + * whether the payload is built at all. This test covers the shipped default only. + * The default is not the enforcement — environment variables and system properties + * outrank it — which is what {@code ProviderLoggingBoundaryVerifier} checks at startup + * against the resolved level. This test stops the default from drifting. */ class ProviderLoggingBoundaryTests { @@ -65,7 +66,11 @@ void noProfileLowersAPayloadLoggingPackageBackToWarn() throws IOException { return; } String logger = key.substring("logging.level.".length()); - if (PAYLOAD_LOGGING_PACKAGES.stream().noneMatch(p -> p.equals(logger) || p.startsWith(logger + "."))) { + // Ancestors and descendants both matter: a parent pinned above the packages + // relaxes them, and a child such as org.springframework.ai.openai.api overrides + // the parent pin for everything under it. + if (PAYLOAD_LOGGING_PACKAGES.stream().noneMatch( + p -> p.equals(logger) || p.startsWith(logger + ".") || logger.startsWith(p + "."))) { return; } assertTrue( @@ -93,8 +98,8 @@ private static Map load(org.springframework.core.io.Resource res */ private static File[] profileConfigurationFiles() throws IOException { File resources = new ClassPathResource("application.yml").getFile().getParentFile(); - File[] profiles = resources.listFiles( - (directory, name) -> name.startsWith("application-") && name.endsWith(".yml")); + File[] profiles = resources.listFiles((directory, name) -> name.startsWith("application-") + && (name.endsWith(".yml") || name.endsWith(".yaml"))); return profiles == null ? new File[0] : profiles; } } diff --git a/apps/worker/src/main/resources/application.yml b/apps/worker/src/main/resources/application.yml index f4704f3a..886b6ea8 100644 --- a/apps/worker/src/main/resources/application.yml +++ b/apps/worker/src/main/resources/application.yml @@ -121,8 +121,12 @@ logging: # WARN messages of their own, which `spring.ai.chat.observations.log-prompt` # does not reach because it only governs Spring AI observation handlers. # Every such site is guarded by isWarnEnabled(), so pinning these packages - # above WARN stops the payload from being built at all. Deliberately not - # environment-overridable: the payload-free boundary is not a per-deployment - # setting. See docs/runbooks/graph-rag-production-hardening.md. + # above WARN stops the payload from being built at all. + # + # This is the safe default, not the enforcement: LOGGING_LEVEL_* environment + # variables, system properties and logback configuration all outrank it. + # ProviderLoggingBoundaryVerifier checks the resolved level at startup and + # refuses to run with these open. + # See docs/runbooks/graph-rag-production-hardening.md. org.springframework.ai.openai: ERROR org.springframework.ai.anthropic: ERROR diff --git a/apps/worker/src/test/java/com/orgmemory/worker/observability/ProviderLoggingBoundaryTests.java b/apps/worker/src/test/java/com/orgmemory/worker/observability/ProviderLoggingBoundaryTests.java index 76ca6c49..b37ce494 100644 --- a/apps/worker/src/test/java/com/orgmemory/worker/observability/ProviderLoggingBoundaryTests.java +++ b/apps/worker/src/test/java/com/orgmemory/worker/observability/ProviderLoggingBoundaryTests.java @@ -21,10 +21,11 @@ * the prompt and the response content. {@code spring.ai.chat.observations.log-prompt} * does not reach them because it only governs Spring AI observation handlers. * - *

Every such call site is guarded by {@code isWarnEnabled()}, so the level - * configured for those packages is what keeps the payload from being built at all. - * The worker runs graph extraction over chunk content, so its prompts carry evidence - * text directly. + *

Every such call site is guarded by {@code isWarnEnabled()}, so the level decides + * whether the payload is built at all. The worker runs graph extraction over chunk + * content, so its prompts carry evidence text directly. This test covers the shipped + * default only; {@code ProviderLoggingBoundaryVerifier} enforces the resolved level at + * startup, which is where an environment override is caught. */ class ProviderLoggingBoundaryTests { @@ -65,7 +66,11 @@ void noProfileLowersAPayloadLoggingPackageBackToWarn() throws IOException { return; } String logger = key.substring("logging.level.".length()); - if (PAYLOAD_LOGGING_PACKAGES.stream().noneMatch(p -> p.equals(logger) || p.startsWith(logger + "."))) { + // Ancestors and descendants both matter: a parent pinned above the packages + // relaxes them, and a child such as org.springframework.ai.openai.api overrides + // the parent pin for everything under it. + if (PAYLOAD_LOGGING_PACKAGES.stream().noneMatch( + p -> p.equals(logger) || p.startsWith(logger + ".") || logger.startsWith(p + "."))) { return; } assertTrue( @@ -93,8 +98,8 @@ private static Map load(org.springframework.core.io.Resource res */ private static File[] profileConfigurationFiles() throws IOException { File resources = new ClassPathResource("application.yml").getFile().getParentFile(); - File[] profiles = resources.listFiles( - (directory, name) -> name.startsWith("application-") && name.endsWith(".yml")); + File[] profiles = resources.listFiles((directory, name) -> name.startsWith("application-") + && (name.endsWith(".yml") || name.endsWith(".yaml"))); return profiles == null ? new File[0] : profiles; } } diff --git a/components/graph-rag-core/src/main/java/com/orgmemory/graphrag/observability/FailureTolerantGraphRagEventSink.java b/components/graph-rag-core/src/main/java/com/orgmemory/graphrag/observability/FailureTolerantGraphRagEventSink.java index 7e51973e..03aec555 100644 --- a/components/graph-rag-core/src/main/java/com/orgmemory/graphrag/observability/FailureTolerantGraphRagEventSink.java +++ b/components/graph-rag-core/src/main/java/com/orgmemory/graphrag/observability/FailureTolerantGraphRagEventSink.java @@ -2,6 +2,8 @@ import java.lang.System.Logger; import java.util.Objects; +import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.atomic.AtomicLong; import java.util.concurrent.atomic.AtomicReference; @@ -13,7 +15,7 @@ * retrieval or an indexing job succeeds. The cost is that a sink broken since * startup looks exactly like a sink with nothing to report. This wrapper keeps the * absorption and adds the signal that was missing: a running count, the type of the - * most recent failure, and one log line each time the failure changes kind. + * most recent failure, and one log line the first time each kind of failure appears. * *

Only class names are recorded. A telemetry backend's exception message can * quote the request it failed to send, so the message and the stack trace are @@ -28,9 +30,16 @@ public final class FailureTolerantGraphRagEventSink implements GraphRagEventSink private static final Logger LOGGER = System.getLogger(FailureTolerantGraphRagEventSink.class.getName()); + /** + * How many distinct failure types are worth a log line. A backend that produces more + * than this is malfunctioning in a way one more line will not clarify. + */ + private static final int REPORTED_FAILURE_TYPE_LIMIT = 10; + private final GraphRagEventSink delegate; private final AtomicLong swallowedFailures = new AtomicLong(); private final AtomicReference lastFailureType = new AtomicReference<>(); + private final Set reportedFailureTypes = ConcurrentHashMap.newKeySet(); FailureTolerantGraphRagEventSink(GraphRagEventSink delegate) { this.delegate = Objects.requireNonNull(delegate, "delegate"); @@ -55,17 +64,26 @@ public String lastFailureType() { return lastFailureType.get(); } + /** How many log lines this sink has produced. One per distinct failure kind. */ + int reportedFailureTypeCount() { + return reportedFailureTypes.size(); + } + private void record(RuntimeException failure) { swallowedFailures.incrementAndGet(); String failureType = failure.getClass().getName(); - // One line per change of kind: a permanently broken sink must not flood the - // log at event rate, but a sink that starts failing differently must say so. - if (!failureType.equals(lastFailureType.getAndSet(failureType))) { + lastFailureType.set(failureType); + // One line the first time each kind appears. Comparing against the previous kind + // alone would flood at event rate as soon as a broken backend alternates, which a + // timeout that retries as a connection failure does immediately. + if (reportedFailureTypes.size() < REPORTED_FAILURE_TYPE_LIMIT + && reportedFailureTypes.add(failureType)) { LOGGER.log( Logger.Level.WARNING, "GraphRAG telemetry sink {0} is failing with {1}; events are being dropped." + " Message and stack trace are withheld because a telemetry failure" - + " can quote the event it could not send.", + + " can quote the event it could not send. Later failures of this kind" + + " are counted rather than logged.", delegate.getClass().getName(), failureType); } diff --git a/components/graph-rag-core/src/test/java/com/orgmemory/graphrag/observability/FailureTolerantGraphRagEventSinkTests.java b/components/graph-rag-core/src/test/java/com/orgmemory/graphrag/observability/FailureTolerantGraphRagEventSinkTests.java index 2ec02283..9e2441ff 100644 --- a/components/graph-rag-core/src/test/java/com/orgmemory/graphrag/observability/FailureTolerantGraphRagEventSinkTests.java +++ b/components/graph-rag-core/src/test/java/com/orgmemory/graphrag/observability/FailureTolerantGraphRagEventSinkTests.java @@ -53,6 +53,28 @@ void reportsNothingWhileTheBackendIsHealthy() { assertNull(sink.lastFailureType()); } + @Test + void keepsCountingAndReportingTheLatestKindWhenTheBackendAlternatesFailures() { + var kinds = new java.util.ArrayDeque(java.util.List.of( + new IllegalStateException("timeout"), + new IllegalArgumentException("connection refused"), + new IllegalStateException("timeout again"))); + var sink = GraphRagEventSink.failureTolerant(event -> { + throw kinds.removeFirst(); + }); + + sink.emit(event()); + sink.emit(event()); + sink.emit(event()); + + // A -> B -> A is what a timeout that retries as a connection failure looks like. + // Every one is counted; the recurrence of A must not produce a second log line, which + // is why reporting is keyed on the set of kinds seen rather than on the previous one. + assertEquals(3, sink.swallowedFailureCount()); + assertEquals(2, sink.reportedFailureTypeCount(), "the recurrence of A must not log again"); + assertEquals(IllegalStateException.class.getName(), sink.lastFailureType()); + } + @Test void namesOnlyTheFailureTypeSoATelemetryErrorCannotQuoteTheEventItCouldNotSend() { var sink = GraphRagEventSink.failureTolerant(event -> { diff --git a/docs/specs/domains/secure-graph-rag.md b/docs/specs/domains/secure-graph-rag.md index 018591af..ab2a6cf3 100644 --- a/docs/specs/domains/secure-graph-rag.md +++ b/docs/specs/domains/secure-graph-rag.md @@ -158,9 +158,14 @@ Reconciled: `2026-07-29-observability-pipeline (f17256b)`. holding query, evidence or provider-response text. It applies to all spans, not only GraphRAG ones, and has no toggle. Span attributes are not filtered. - The OpenAI and Anthropic client packages are pinned above WARN in both apps, - without an environment override, because their own logging concatenates the - prompt or the response content into messages that Spring AI's - `log-prompt`/`log-completion` settings do not govern. + because their own logging concatenates the prompt or the response content into + messages that Spring AI's `log-prompt`/`log-completion` settings do not govern. + That pin is the default, not the boundary: `LOGGING_LEVEL_*` variables, system + properties, logback configuration and a level on a more specific logger all + outrank it. `ProviderLoggingBoundaryVerifier` asks each class holding such a + call site whether WARN is enabled on its own logger and fails startup if any + is, so the check is against the resolved level rather than any source of it. + It has no disable property. ## Graph Explorer diff --git a/docs/tests/domains/secure-graph-rag.md b/docs/tests/domains/secure-graph-rag.md index 58460038..5c074be5 100644 --- a/docs/tests/domains/secure-graph-rag.md +++ b/docs/tests/domains/secure-graph-rag.md @@ -67,10 +67,17 @@ Reconciled: `2026-07-29-observability-pipeline (f17256b)`. pass. Wiring tests prove the auto-configuration stays discoverable, wraps every declared exporter, and is ordered ahead of Spring Boot's unwrapped collection. - Provider-logging tests in each app read the shipped `application.yml` and fail - if the pin is absent or if any profile lowers `org.springframework.ai.openai` - or `org.springframework.ai.anthropic` back to WARN. They assert that at least - one profile file was scanned, so an unreadable resource directory cannot make - them pass vacuously. + if the pin is absent, or if any `.yml`/`.yaml` profile lowers + `org.springframework.ai.openai` or `org.springframework.ai.anthropic` — or any + ancestor or descendant of them — back to WARN. They assert that at least one + profile file was scanned, so an unreadable resource directory cannot make them + pass vacuously. They cover the shipped default only. +- Boundary-verifier tests set levels the way an operator override would, against + the real provider classes and a real logging backend, and prove startup fails + when a leak site is at WARN or below, that a class set back to WARN is caught + even under a package pinned to ERROR, that the message names both the class and + the override to look for, that the auto-configuration stays discoverable, and + that the application context itself fails rather than starting. - Storage adapter auto-configuration tests prove PostgreSQL, OpenSearch and Neo4j stay discoverable through their registration files, that PostgreSQL owns the canonical ports without an opt-in, that OpenSearch and Neo4j claim no port diff --git a/integrations/graph-rag-observability/build.gradle.kts b/integrations/graph-rag-observability/build.gradle.kts index 499bd88a..288bf878 100644 --- a/integrations/graph-rag-observability/build.gradle.kts +++ b/integrations/graph-rag-observability/build.gradle.kts @@ -6,6 +6,7 @@ dependencies { api(project(":components:graph-rag-core")) implementation("io.opentelemetry:opentelemetry-api") implementation("io.opentelemetry:opentelemetry-sdk-trace") + implementation("org.slf4j:slf4j-api") implementation("org.springframework.boot:spring-boot-autoconfigure") // Supplies the SpanExporters collection this module replaces with a sanitized copy. implementation("org.springframework.boot:spring-boot-micrometer-tracing-opentelemetry") @@ -17,5 +18,10 @@ dependencies { testImplementation("org.junit.jupiter:junit-jupiter") testImplementation("org.springframework.boot:spring-boot-test") testImplementation("org.assertj:assertj-core") + // The logging boundary is only meaningful against the real leak sites and a real logging + // backend, so the verifier is tested against both rather than against stand-ins. + testImplementation("org.springframework.ai:spring-ai-openai") + testImplementation("org.springframework.ai:spring-ai-anthropic") + testImplementation("ch.qos.logback:logback-classic") testRuntimeOnly("org.junit.platform:junit-platform-launcher") } diff --git a/integrations/graph-rag-observability/src/main/java/com/orgmemory/integrations/graphrag/observability/ProviderLoggingBoundaryAutoConfiguration.java b/integrations/graph-rag-observability/src/main/java/com/orgmemory/integrations/graphrag/observability/ProviderLoggingBoundaryAutoConfiguration.java new file mode 100644 index 00000000..f4bc88ea --- /dev/null +++ b/integrations/graph-rag-observability/src/main/java/com/orgmemory/integrations/graphrag/observability/ProviderLoggingBoundaryAutoConfiguration.java @@ -0,0 +1,22 @@ +package com.orgmemory.integrations.graphrag.observability; + +import org.springframework.beans.factory.InitializingBean; +import org.springframework.boot.autoconfigure.AutoConfiguration; +import org.springframework.context.annotation.Bean; + +/** + * Runs {@link ProviderLoggingBoundaryVerifier} during context refresh, so an application + * whose logging configuration would leak prompts fails to start instead of serving traffic. + * + *

Logging levels are applied long before refresh, and the actuator loggers endpoint is not + * exposed, so the level observed here is the one the provider libraries will see. There is no + * property to disable this: a boundary with an off switch is a default, not a boundary. + */ +@AutoConfiguration +public class ProviderLoggingBoundaryAutoConfiguration { + + @Bean + InitializingBean providerLoggingBoundaryVerification() { + return () -> ProviderLoggingBoundaryVerifier.verify(getClass().getClassLoader()); + } +} diff --git a/integrations/graph-rag-observability/src/main/java/com/orgmemory/integrations/graphrag/observability/ProviderLoggingBoundaryVerifier.java b/integrations/graph-rag-observability/src/main/java/com/orgmemory/integrations/graphrag/observability/ProviderLoggingBoundaryVerifier.java new file mode 100644 index 00000000..c42899a5 --- /dev/null +++ b/integrations/graph-rag-observability/src/main/java/com/orgmemory/integrations/graphrag/observability/ProviderLoggingBoundaryVerifier.java @@ -0,0 +1,55 @@ +package com.orgmemory.integrations.graphrag.observability; + +import java.util.List; +import org.slf4j.LoggerFactory; +import org.springframework.util.ClassUtils; + +/** + * Refuses to start an application whose configuration would let a provider library log a + * prompt. + * + *

The OpenAI and Anthropic clients build WARN messages by concatenating the prompt or the + * response content, guarded by {@code isWarnEnabled()}. Pinning those packages in + * {@code application.yml} makes the guard closed by default, but it is only a default: + * {@code LOGGING_LEVEL_ORG_SPRINGFRAMEWORK_AI_OPENAI}, a system property, a + * {@code logback.xml}, or a level set on a subordinate logger all outrank it. Configuration + * cannot enforce a boundary that configuration can undo. + * + *

So the check is made against the resolved state rather than against any source of it: + * each class that holds a payload-logging call site is asked whether WARN is enabled on its + * own logger, which is the exact question its guard will ask at runtime, with inheritance + * already applied. Startup fails rather than proceeds, because the alternative is an + * application that looks healthy while writing customer text to disk. + */ +final class ProviderLoggingBoundaryVerifier { + + /** + * Classes whose own logging concatenates prompt or response content into a WARN message. + * These are logger names, not merely packages: a level set on a child logger overrides an + * ancestor, so only the leak site's own logger answers the question its guard asks. + */ + static final List PAYLOAD_LOGGING_CLASSES = List.of( + "org.springframework.ai.openai.OpenAiChatModel", + "org.springframework.ai.openai.OpenAiAudioSpeechModel", + "org.springframework.ai.anthropic.AnthropicChatModel"); + + private ProviderLoggingBoundaryVerifier() { } + + static void verify(ClassLoader classLoader) { + List open = PAYLOAD_LOGGING_CLASSES.stream() + .filter(name -> ClassUtils.isPresent(name, classLoader)) + .filter(name -> LoggerFactory.getLogger(name).isWarnEnabled()) + .toList(); + if (open.isEmpty()) { + return; + } + throw new IllegalStateException( + "WARN logging is enabled for " + String.join(", ", open) + + ", which would write prompt and response content to the application log." + + " Those classes build the message only when WARN is enabled, so the level" + + " is the boundary. Something outranked the ERROR pin in application.yml:" + + " check LOGGING_LEVEL_* environment variables, -Dlogging.level.*, a" + + " logback configuration, and any level set on a more specific logger." + + " spring.ai.chat.observations.log-prompt does not govern these paths."); + } +} diff --git a/integrations/graph-rag-observability/src/main/resources/META-INF/spring/org.springframework.boot.autoconfigure.AutoConfiguration.imports b/integrations/graph-rag-observability/src/main/resources/META-INF/spring/org.springframework.boot.autoconfigure.AutoConfiguration.imports index 5870283c..1355e59d 100644 --- a/integrations/graph-rag-observability/src/main/resources/META-INF/spring/org.springframework.boot.autoconfigure.AutoConfiguration.imports +++ b/integrations/graph-rag-observability/src/main/resources/META-INF/spring/org.springframework.boot.autoconfigure.AutoConfiguration.imports @@ -1,2 +1,3 @@ com.orgmemory.integrations.graphrag.observability.GraphRagObservabilityAutoConfiguration +com.orgmemory.integrations.graphrag.observability.ProviderLoggingBoundaryAutoConfiguration com.orgmemory.integrations.graphrag.observability.SpanExportSanitizationAutoConfiguration diff --git a/integrations/graph-rag-observability/src/test/java/com/orgmemory/integrations/graphrag/observability/ProviderLoggingBoundaryVerifierTests.java b/integrations/graph-rag-observability/src/test/java/com/orgmemory/integrations/graphrag/observability/ProviderLoggingBoundaryVerifierTests.java new file mode 100644 index 00000000..a0e2ced4 --- /dev/null +++ b/integrations/graph-rag-observability/src/test/java/com/orgmemory/integrations/graphrag/observability/ProviderLoggingBoundaryVerifierTests.java @@ -0,0 +1,132 @@ +package com.orgmemory.integrations.graphrag.observability; + +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import ch.qos.logback.classic.Level; +import ch.qos.logback.classic.LoggerContext; +import java.util.ArrayList; +import java.util.List; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Test; +import org.slf4j.LoggerFactory; +import org.springframework.boot.autoconfigure.AutoConfiguration; +import org.springframework.boot.autoconfigure.AutoConfigurations; +import org.springframework.boot.context.annotation.ImportCandidates; +import org.springframework.boot.test.context.runner.ApplicationContextRunner; + +/** + * The YAML pin is a default, not a boundary: an environment variable, a system property, a + * logback configuration or a level on a more specific logger all outrank it. This verifier + * exists to catch that, so the tests set the level the way an operator's override would and + * check the resolved state rather than the configuration that produced it. + */ +class ProviderLoggingBoundaryVerifierTests { + + private static final String LEAK_SITE = "org.springframework.ai.openai.OpenAiChatModel"; + + private final List touched = new ArrayList<>(); + + /** + * The test JVM has no logging configuration, so every leak site starts WARN-enabled. Each + * test begins from the closed state the shipped YAML produces and then reopens exactly one, + * so a failure names the site the test is about rather than the default. + */ + @org.junit.jupiter.api.BeforeEach + void closeEveryLeakSite() { + ProviderLoggingBoundaryVerifier.PAYLOAD_LOGGING_CLASSES + .forEach(name -> setLevel(name, Level.ERROR)); + } + + @AfterEach + void restoreLevels() { + touched.forEach(name -> loggerContext().getLogger(name).setLevel(null)); + touched.clear(); + } + + @Test + void startsWhenEveryLeakSiteIsAboveWarn() { + assertDoesNotThrow(() -> ProviderLoggingBoundaryVerifier.verify(classLoader())); + } + + @Test + void refusesToStartWhenAnOverrideReopensAProviderLeakSite() { + setLevel(LEAK_SITE, Level.WARN); + + IllegalStateException failure = assertThrows( + IllegalStateException.class, + () -> ProviderLoggingBoundaryVerifier.verify(classLoader())); + + assertTrue(failure.getMessage().contains(LEAK_SITE), "the message must name what to fix"); + assertTrue( + failure.getMessage().contains("LOGGING_LEVEL"), + "the message must point at the override that outranked the YAML pin"); + } + + @Test + void guardsEveryClassKnownToConcatenatePayloadIntoAWarnMessage() { + assertTrue( + ProviderLoggingBoundaryVerifier.PAYLOAD_LOGGING_CLASSES.containsAll(List.of( + "org.springframework.ai.openai.OpenAiChatModel", + "org.springframework.ai.anthropic.AnthropicChatModel")), + "dropping a known leak site from the list silently reopens it"); + } + + @Test + void refusesToStartWhenTheLevelIsLoweredBelowWarnRatherThanToIt() { + setLevel(LEAK_SITE, Level.DEBUG); + + assertThrows( + IllegalStateException.class, + () -> ProviderLoggingBoundaryVerifier.verify(classLoader())); + } + + @Test + void checksTheLeakSiteRatherThanItsPackageSoAChildOverrideCannotHide() { + setLevel("org.springframework.ai.openai", Level.ERROR); + setLevel(LEAK_SITE, Level.WARN); + + assertThrows( + IllegalStateException.class, + () -> ProviderLoggingBoundaryVerifier.verify(classLoader()), + "a package pinned to ERROR must not excuse a class set back to WARN"); + } + + @Test + void staysDiscoverableWithoutAnApplicationNamingIt() { + List names = new ArrayList<>(); + ImportCandidates.load(AutoConfiguration.class, classLoader()).forEach(names::add); + + assertTrue( + names.contains(ProviderLoggingBoundaryAutoConfiguration.class.getName()), + "META-INF/spring/…AutoConfiguration.imports no longer names this class, so an " + + "application could start with prompt logging enabled"); + } + + @Test + void failsTheApplicationContextRatherThanLettingItServeTraffic() { + setLevel(LEAK_SITE, Level.WARN); + + new ApplicationContextRunner() + .withConfiguration( + AutoConfigurations.of(ProviderLoggingBoundaryAutoConfiguration.class)) + .run(context -> assertTrue( + context.getStartupFailure() != null, + "startup must fail; an application that looks healthy while logging " + + "prompts is the outcome this guard exists to prevent")); + } + + private void setLevel(String logger, Level level) { + touched.add(logger); + loggerContext().getLogger(logger).setLevel(level); + } + + private static LoggerContext loggerContext() { + return (LoggerContext) LoggerFactory.getILoggerFactory(); + } + + private static ClassLoader classLoader() { + return ProviderLoggingBoundaryVerifierTests.class.getClassLoader(); + } +}