fix(graph-rag): fan telemetry to every sink and pin adapter auto-configuration - #132
Conversation
Both wiring sites resolved GraphRagEventSink with ObjectProvider.getIfAvailable(), which throws NoUniqueBeanDefinitionException as soon as a second sink bean exists. Registering any backend beside the OpenTelemetry adapter would have failed worker startup and retrieval configuration rather than emitting to both. GraphRagEventSink.composite fans one event out to every sink and isolates their failures: a broken backend no longer drops the event for the others. The first failure is rethrown with the rest suppressed, so the existing caller guards that treat emission as non-critical keep their behavior. Verified: :components:graph-rag-core:test --tests "com.orgmemory.graphrag.observability.*", plus :core:compileJava and :apps:worker:compileJava. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Every existing test in these modules constructs an adapter directly, so none of them notices when Spring stops loading the module. The registration file is plain text no compiler checks: renaming or moving an auto-configuration class silently removes every port it contributes while the suite stays green. Each adapter now asserts that it remains discoverable through META-INF/spring/…AutoConfiguration.imports, and that its ports appear under the conditions it declares. OpenSearch and Neo4j additionally assert they claim nothing until an operator enables them, because loading either one takes the storage ports away from PostgreSQL. The OpenSearch wiring assertion lives with the container-backed tests: its publication store creates an index while the bean is built, so it cannot be proven against mocks. Neo4j gained an assertion that an enabled adapter without a password fails startup rather than reaching the database unauthenticated. Verified: :integrations:graph-rag-postgres:test, :integrations:graph-rag-opensearch:test, :integrations:graph-rag-neo4j:test, compileJava, :core:test. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Telemetry fan-out and the storage adapters' auto-configuration coverage are current behavior, so the spec and its mirrored test matrix carry them and both reconciliation commits move forward. 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 (1)
📒 Files selected for processing (5)
📜 Recent review details⏰ Context from checks skipped due to timeout. (4)
🧰 Additional context used📓 Path-based instructions (4)**/*📄 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:
🔇 Additional comments (6)
📝 WalkthroughWalkthroughGraph event sinks now fan out across all ordered providers with isolated failure aggregation. New Spring context tests validate Neo4j, OpenSearch, and PostgreSQL auto-configuration discovery, conditional activation, bean wiring, and required properties. ChangesGraph event sink fan-out
Graph RAG adapter auto-configuration validation
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant GraphIndexingProcessor
participant ObjectProvider
participant GraphRagEventSink
participant EventSinkDelegates
GraphIndexingProcessor->>ObjectProvider: obtain ordered sinks
ObjectProvider-->>GraphIndexingProcessor: sink list
GraphIndexingProcessor->>GraphRagEventSink: create composite
GraphRagEventSink->>EventSinkDelegates: emit event to each sink
EventSinkDelegates-->>GraphRagEventSink: failures, if any
GraphRagEventSink-->>GraphIndexingProcessor: complete or rethrow aggregated failure
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@components/graph-rag-core/src/main/java/com/orgmemory/graphrag/observability/GraphRagEventSink.java`:
- Around line 47-51: The catch block in GraphRagEventSink’s delegate fan-out
must avoid self-suppression: only call failure.addSuppressed(sinkFailure) when
failure and sinkFailure are different instances. Add a regression test with more
than two delegates throwing the same RuntimeException instance, verifying
fan-out continues without IllegalArgumentException.
In
`@components/graph-rag-core/src/test/java/com/orgmemory/graphrag/observability/GraphRagEventSinkTests.java`:
- Around line 56-67: Strengthen reportsEveryFailureRatherThanOnlyTheFirst by
retaining references to both IllegalStateException instances, asserting the
thrown failure is the first instance, and asserting its single suppressed
exception is the second instance. Preserve the existing composite.emit
invocation and failure-count assertion as applicable.
In
`@integrations/graph-rag-neo4j/src/test/java/com/orgmemory/graphrag/neo4j/Neo4jGraphRagAutoConfigurationTests.java`:
- Around line 63-66: Update the startup-failure assertion in
Neo4jGraphRagAutoConfigurationTests to verify that
rootCause(context.getStartupFailure()) has a message containing “Neo4j password
must be configured when the adapter is enabled,” while retaining the
IllegalArgumentException type check.
In
`@integrations/graph-rag-opensearch/src/test/java/com/orgmemory/graphrag/opensearch/OpenSearchGraphRagAutoConfigurationTests.java`:
- Around line 38-44: Update claimsNoPortUntilAnOperatorAsksForIt to assert that
all six claimed OpenSearch interfaces have no beans when the enabling property
is omitted, including ProjectionPublicationStore, LexicalIndex, VectorIndex, and
ProcessingStatusIndex alongside GraphStore and ContentStore. Preserve the
existing assertion message and disabled-path test behavior.
In
`@integrations/graph-rag-postgres/src/test/java/com/orgmemory/graphrag/postgres/PostgresGraphRagAutoConfigurationTests.java`:
- Around line 69-74: Expand leavesEveryPortUnclaimedWhenTurnedOff to assert that
all canonical adapter ports are absent when disabled: GraphStore, ContentStore,
ProjectionPublicationStore, both index types, and both cache types. Keep the
existing context-based assertion and failure message while including each
corresponding bean lookup in the empty check.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 2bb02080-aef9-4050-a0ec-96fc31e68f2b
⛔ Files ignored due to path filters (2)
docs/specs/domains/secure-graph-rag.mdis excluded by!docs/**docs/tests/domains/secure-graph-rag.mdis excluded by!docs/**
📒 Files selected for processing (11)
apps/worker/src/main/java/com/orgmemory/worker/graph/GraphIndexingProcessor.javacomponents/graph-rag-core/src/main/java/com/orgmemory/graphrag/observability/GraphRagEventSink.javacomponents/graph-rag-core/src/test/java/com/orgmemory/graphrag/observability/GraphRagEventSinkTests.javacore/src/main/java/com/orgmemory/core/knowledge/GraphRagKnowledgeRetrievalConfiguration.javaintegrations/graph-rag-neo4j/build.gradle.ktsintegrations/graph-rag-neo4j/src/test/java/com/orgmemory/graphrag/neo4j/Neo4jGraphRagAutoConfigurationTests.javaintegrations/graph-rag-opensearch/build.gradle.ktsintegrations/graph-rag-opensearch/src/test/java/com/orgmemory/graphrag/opensearch/OpenSearchGraphRagAutoConfigurationTests.javaintegrations/graph-rag-opensearch/src/test/java/com/orgmemory/graphrag/opensearch/OpenSearchProjectionPublicationIntegrationTests.javaintegrations/graph-rag-postgres/build.gradle.ktsintegrations/graph-rag-postgres/src/test/java/com/orgmemory/graphrag/postgres/PostgresGraphRagAutoConfigurationTests.java
📜 Review details
⏰ Context from checks skipped due to timeout. (4)
- GitHub Check: Neo4j adapter
- GitHub Check: PostgreSQL GraphRAG
- GitHub Check: OpenSearch adapter
- GitHub Check: Backend · Java 25
🧰 Additional context used
📓 Path-based instructions (5)
**/*
📄 CodeRabbit inference engine (CLAUDE.md)
**/*: Always read the repository guidance and relevant sections ofARCHITECTURE.md; before changing a domain, read its specification, test-coverage document, and binding decision filenames.
Treat the repository as the engineering system of record; current repository and runtime evidence take precedence over chat or Northstar.
Readdocs/guidelines/agent-safety.mdbefore retrieval, AI, MCP, permission, upload, graph, or export work. Never commit secrets or customer data.
Files:
integrations/graph-rag-postgres/build.gradle.ktsintegrations/graph-rag-opensearch/build.gradle.ktscore/src/main/java/com/orgmemory/core/knowledge/GraphRagKnowledgeRetrievalConfiguration.javaintegrations/graph-rag-neo4j/build.gradle.ktsintegrations/graph-rag-opensearch/src/test/java/com/orgmemory/graphrag/opensearch/OpenSearchGraphRagAutoConfigurationTests.javaapps/worker/src/main/java/com/orgmemory/worker/graph/GraphIndexingProcessor.javaintegrations/graph-rag-postgres/src/test/java/com/orgmemory/graphrag/postgres/PostgresGraphRagAutoConfigurationTests.javaintegrations/graph-rag-neo4j/src/test/java/com/orgmemory/graphrag/neo4j/Neo4jGraphRagAutoConfigurationTests.javacomponents/graph-rag-core/src/test/java/com/orgmemory/graphrag/observability/GraphRagEventSinkTests.javaintegrations/graph-rag-opensearch/src/test/java/com/orgmemory/graphrag/opensearch/OpenSearchProjectionPublicationIntegrationTests.javacomponents/graph-rag-core/src/main/java/com/orgmemory/graphrag/observability/GraphRagEventSink.java
**/*.{java,gradle,gradle.kts,properties,yml,yaml}
📄 CodeRabbit inference engine (CLAUDE.md)
Before using unfamiliar Spring Boot 4, Spring Modulith 2, Spring AI 2, or Gradle APIs, consult current official documentation, Context7, and the relevant project verification skill.
Files:
integrations/graph-rag-postgres/build.gradle.ktsintegrations/graph-rag-opensearch/build.gradle.ktscore/src/main/java/com/orgmemory/core/knowledge/GraphRagKnowledgeRetrievalConfiguration.javaintegrations/graph-rag-neo4j/build.gradle.ktsintegrations/graph-rag-opensearch/src/test/java/com/orgmemory/graphrag/opensearch/OpenSearchGraphRagAutoConfigurationTests.javaapps/worker/src/main/java/com/orgmemory/worker/graph/GraphIndexingProcessor.javaintegrations/graph-rag-postgres/src/test/java/com/orgmemory/graphrag/postgres/PostgresGraphRagAutoConfigurationTests.javaintegrations/graph-rag-neo4j/src/test/java/com/orgmemory/graphrag/neo4j/Neo4jGraphRagAutoConfigurationTests.javacomponents/graph-rag-core/src/test/java/com/orgmemory/graphrag/observability/GraphRagEventSinkTests.javaintegrations/graph-rag-opensearch/src/test/java/com/orgmemory/graphrag/opensearch/OpenSearchProjectionPublicationIntegrationTests.javacomponents/graph-rag-core/src/main/java/com/orgmemory/graphrag/observability/GraphRagEventSink.java
**/*.{java,gradle,gradle.kts}
📄 CodeRabbit inference engine (CLAUDE.md)
Use the testing harness; a terminating clean test is the JVM context gate, and
bootRunis not verification. IDE inspection applies only to edited backend Java.
Files:
integrations/graph-rag-postgres/build.gradle.ktsintegrations/graph-rag-opensearch/build.gradle.ktscore/src/main/java/com/orgmemory/core/knowledge/GraphRagKnowledgeRetrievalConfiguration.javaintegrations/graph-rag-neo4j/build.gradle.ktsintegrations/graph-rag-opensearch/src/test/java/com/orgmemory/graphrag/opensearch/OpenSearchGraphRagAutoConfigurationTests.javaapps/worker/src/main/java/com/orgmemory/worker/graph/GraphIndexingProcessor.javaintegrations/graph-rag-postgres/src/test/java/com/orgmemory/graphrag/postgres/PostgresGraphRagAutoConfigurationTests.javaintegrations/graph-rag-neo4j/src/test/java/com/orgmemory/graphrag/neo4j/Neo4jGraphRagAutoConfigurationTests.javacomponents/graph-rag-core/src/test/java/com/orgmemory/graphrag/observability/GraphRagEventSinkTests.javaintegrations/graph-rag-opensearch/src/test/java/com/orgmemory/graphrag/opensearch/OpenSearchProjectionPublicationIntegrationTests.javacomponents/graph-rag-core/src/main/java/com/orgmemory/graphrag/observability/GraphRagEventSink.java
**/*.{java,sql}
📄 CodeRabbit inference engine (CLAUDE.md)
Keep
ddl-auto=validateand pair every persisted-model change with a Flyway migration.
Files:
core/src/main/java/com/orgmemory/core/knowledge/GraphRagKnowledgeRetrievalConfiguration.javaintegrations/graph-rag-opensearch/src/test/java/com/orgmemory/graphrag/opensearch/OpenSearchGraphRagAutoConfigurationTests.javaapps/worker/src/main/java/com/orgmemory/worker/graph/GraphIndexingProcessor.javaintegrations/graph-rag-postgres/src/test/java/com/orgmemory/graphrag/postgres/PostgresGraphRagAutoConfigurationTests.javaintegrations/graph-rag-neo4j/src/test/java/com/orgmemory/graphrag/neo4j/Neo4jGraphRagAutoConfigurationTests.javacomponents/graph-rag-core/src/test/java/com/orgmemory/graphrag/observability/GraphRagEventSinkTests.javaintegrations/graph-rag-opensearch/src/test/java/com/orgmemory/graphrag/opensearch/OpenSearchProjectionPublicationIntegrationTests.javacomponents/graph-rag-core/src/main/java/com/orgmemory/graphrag/observability/GraphRagEventSink.java
core/src/main/java/com/orgmemory/core/{authorization,knowledge,permission}/**/*.java
⚙️ CodeRabbit configuration file
core/src/main/java/com/orgmemory/core/{authorization,knowledge,permission}/**/*.java: Treat PostgreSQL ACL evidence as canonical and OpenFGA as the relationship
authorization decision point. Authorization must fail closed. Filtering
must happen before ranking, LIMIT, graph traversal, answer generation,
export, and citation rendering. Flag metadata or timing leak paths.
Files:
core/src/main/java/com/orgmemory/core/knowledge/GraphRagKnowledgeRetrievalConfiguration.java
🔇 Additional comments (13)
components/graph-rag-core/src/main/java/com/orgmemory/graphrag/observability/GraphRagEventSink.java (1)
24-46: LGTM!Also applies to: 52-60
apps/worker/src/main/java/com/orgmemory/worker/graph/GraphIndexingProcessor.java (1)
82-82: LGTM!core/src/main/java/com/orgmemory/core/knowledge/GraphRagKnowledgeRetrievalConfiguration.java (1)
43-43: LGTM!components/graph-rag-core/src/test/java/com/orgmemory/graphrag/observability/GraphRagEventSinkTests.java (1)
3-10: LGTM!Also applies to: 32-54, 69-100
integrations/graph-rag-neo4j/build.gradle.kts (1)
13-15: LGTM!integrations/graph-rag-neo4j/src/test/java/com/orgmemory/graphrag/neo4j/Neo4jGraphRagAutoConfigurationTests.java (1)
1-55: LGTM!Also applies to: 69-115
integrations/graph-rag-postgres/build.gradle.kts (1)
16-18: LGTM!integrations/graph-rag-postgres/src/test/java/com/orgmemory/graphrag/postgres/PostgresGraphRagAutoConfigurationTests.java (2)
1-66: LGTM!
77-99: LGTM!integrations/graph-rag-opensearch/build.gradle.kts (1)
15-16: LGTM!integrations/graph-rag-opensearch/src/test/java/com/orgmemory/graphrag/opensearch/OpenSearchGraphRagAutoConfigurationTests.java (1)
1-37: LGTM!Also applies to: 46-54
integrations/graph-rag-opensearch/src/test/java/com/orgmemory/graphrag/opensearch/OpenSearchProjectionPublicationIntegrationTests.java (2)
4-4: LGTM!Also applies to: 21-21, 41-44, 617-622, 654-665
623-653: 🎯 Functional CorrectnessProvide a terminating clean
:integrations:graph-rag-opensearch:testresult.This is required for the
java/gradleJVM context gate before merge.
Review caught a real defect in the composite: Throwable.addSuppressed rejects self-suppression, so two sinks raising one shared exception instance made the fan-out throw IllegalArgumentException from inside telemetry rather than reporting the sink failure. An identity guard skips the redundant suppression. The assertions the review flagged as too weak now hold their claims: the composite test keeps both exception references and proves which one propagates and which is suppressed; Neo4j asserts the password branch by message rather than accepting any validation failure; and both disabled-path tests assert every port the adapter would claim rather than a sample of two. Verified: the graph-rag-core, postgres, neo4j and opensearch auto-configuration and event-sink tests. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Review findings addressed (
|
1 participant
Why
Two latent failures in the GraphRAG observability wiring, found while scoping the
observability work:
A second event sink would have failed startup. Both wiring sites resolved
GraphRagEventSinkwithObjectProvider.getIfAvailable(), which throwsNoUniqueBeanDefinitionExceptionas soon as two sink beans exist. Registering anybackend beside the OpenTelemetry adapter — a Micrometer sink for
/actuator/prometheus, for instance — would have broken worker startup and theretrieval configuration rather than emitting to both.
Nothing proved the storage adapters still auto-configure. Every existing test in
graph-rag-{postgres,opensearch,neo4j}constructs an adapter directly, so the suitestays green even if Spring stops loading the module.
META-INF/spring/…AutoConfiguration.importsis plain text that no compiler checks: renaming or moving an auto-configuration class
silently removes every port it contributes, and only production notices.
What changed
GraphRagEventSink.composite(List<GraphRagEventSink>)ingraph-rag-corefans oneevent out to every sink. Sinks fail independently — a broken backend no longer drops the
event for the others. The first failure is rethrown with the rest suppressed, so the
existing caller guards that already treat emission as non-critical keep their behavior.
Both call sites now use it:
apps/worker/…/GraphIndexingProcessor(indexing stages)core/…/GraphRagKnowledgeRetrievalConfiguration(retrieval stages)Auto-configuration tests for all three storage adapters. Each asserts it stays
discoverable through
ImportCandidates.load(AutoConfiguration.class, …)— note thatAutoConfigurations.of(X.class)bypasses the registration file, so that check is the onlyone that catches a broken
.imports. Each also asserts its ports appear under theconditions it declares: PostgreSQL owns the canonical ports with no opt-in, while
OpenSearch and Neo4j claim nothing until enabled, because loading either takes the storage
ports away from PostgreSQL.
Two behaviors surfaced while writing them and are now pinned:
database unauthenticated.
OpenSearchProjectionPublicationStorecreates an index while the bean is built, so theOpenSearch wiring assertion cannot run against mocks. It lives with the container-backed
tests instead of being dropped.
Verification
clean testis running locally; result posted as a comment before merge.JetBrains MCP inspection was unavailable, so the fallback gates from
orgmemory-static-analysiswere used.Consolidation
docs/specs/domains/secure-graph-rag.mdanddocs/tests/domains/secure-graph-rag.mdcarry the fan-out contract and the new coverage; both
Reconciled:lines move tofd495d0.git log 7acda3a..HEADover the specs' source paths showed only the twocommits in this PR.
Not in scope
Adding a Micrometer sink and
/actuator/prometheusis the follow-on this unblocks — nometrics registry is wired today, so Spring AI's
gen_ai.client.token.usageis recordedinto a
SimpleMeterRegistryand discarded.🤖 Generated with Claude Code
Summary by CodeRabbit