refactor(observability): give the payload boundary its own module - #152
Merged
Conversation
`integrations/graph-rag-observability` opened with
`api(project(":components:graph-rag-core"))`, so taking the payload boundary
meant taking the graph domain. `apps/mcp` did not take it: its build file had
no `project(...)` dependency at all — the only Gradle application here with
none — while carrying the OpenTelemetry starter and exporting spans. So the
guarantee decision 0018 states across the process held in two deployables and
in none of the third's spans.
Seven of that module's ten files had nothing to do with GraphRAG, two of them
added the same day by the increment that found this.
The boundary moves to `integrations/observability`, which depends on no
OrgMemory module. That absence is load-bearing: it is what keeps the boundary
adoptable by a deployable with no domain dependency, which is the condition
that failed. `graph-rag-observability` keeps what implements a core port — the
two event sinks and the task decorator — and depends on the new module only in
its test source set, where the whole-export allowlist gate drives a GraphRAG
span through the sanitizer in the position it occupies in production.
`orgmemory.spring-boot-app-conventions` adds the new module, so taking the
convention is taking the boundary. `apps/mcp` gains it without its build file
changing, and a fourth application cannot omit it by not knowing to ask.
No behaviour changed and no test was rewritten: the four boundary test classes
moved unedited, and 32 + 28 is the 60 the single module ran before.
`ARCHITECTURE.md` and decision 0019 both record that this module is a
deliberate exception to `integrations -> core ports` — it implements no core
port — so the exception is one that was seen and chosen rather than one that
was never noticed.
Gates: :integrations:observability:test (32),
:integrations:graph-rag-observability:test (28), :apps:api:test (152),
:apps:worker:test (47), :apps:mcp:test (55). 314 tests, 0 skipped, 0 failures.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (5)
📒 Files selected for processing (20)
💤 Files with no reviewable changes (1)
📜 Recent review details🧰 Additional context used📓 Path-based instructions (5)**/*📄 CodeRabbit inference engine (CLAUDE.md)
Files:
**/*.{java,gradle,gradle.kts,properties,yml,yaml}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
**/*.{java,sql}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
**/*.{java,gradle,gradle.kts}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
ARCHITECTURE.md📄 CodeRabbit inference engine (CLAUDE.md)
Files:
🔇 Additional comments (19)
📝 WalkthroughWalkthroughThe telemetry boundary is extracted into ChangesObservability Boundary Extraction
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes the last open decision of the observability increment — and investigating
it found that one deployable had no payload boundary at all.
What was wrong
integrations/graph-rag-observabilityopens withapi(project(":components:graph-rag-core")). Taking the payload boundary meanttaking the graph domain.
apps/mcpdid not take it. Its build file had noproject(...)dependency atall — the only Gradle application here with none — while carrying
spring-boot-starter-opentelemetryand exporting spans.So the guarantee decision 0018
states across the process held in two deployables and in none of the third's
spans.
KnowledgeSearchApiClientchains aRestClientExceptionas the cause ofits own constant-message failure, and
exception.stacktraceexports unfilteredwithout the sanitizer. Whether any particular exception carried payload was not
established; that nothing would have stopped it was.
Seven of that module's ten files had nothing to do with GraphRAG — two of them
added the same day, by the increment that found this.
The split
integrations/observability(new)integrations/graph-rag-observabilityGraphRagEventSinkadapters, theGraphRagTaskDecoratorimplementationgraph-rag-coreThe absence of a
project(...)line in the new module is load-bearing. It iswhat keeps the boundary adoptable by a deployable with no domain dependency —
the condition that failed here.
graph-rag-observabilitydepends on the new module only in its test sourceset, where
WholeExportAllowlistTestsdrives a GraphRAG span through thesanitizer in the position the sanitizer occupies in production. That test
belongs across the seam.
Enforcement, not a name
orgmemory.spring-boot-app-conventionsnow adds the new module. Taking theconvention is taking the boundary. Every Gradle application already applies that
plugin, so:
apps/mcpgains the boundary without its own build file changing, still doesnot carry
graph-rag-core, and a fourth application cannot omit it by notknowing to ask.
An exception recorded rather than slipped in
ARCHITECTURE.mdstates the adapter rule asintegrations -> core ports. Thenew module implements no core port — it implements OpenTelemetry's
SpanExporterand two startup checks that implement nothing. By that rule it is not an
integration.
Placing it in
integrations/anyway is the project owner's decision, taken afterthe alternatives were laid out, including a fifth top-level directory that was
rejected as more structure than ~250 lines earns against a module count already
at seventeen. Both
ARCHITECTURE.mdanddecision 0019
name it as an exception that was seen and chosen.
No behaviour changed
The four boundary test classes moved unedited. 32 + 28 is the 60 the single
module ran before.
Not done here
Decision 0019's challenge — proposal, strongest counterargument, repository
evidence, choice, rejected alternatives — is recorded, but I wrote the
counterargument myself. It has not been through an independent reviewer.
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation
Tests