Skip to content

[1/5] VSA: Merge Elastic.Documentation.Api.* into single project#3333

Merged
Mpdreamz merged 6 commits into
mainfrom
refactor/merge-api-projects
May 19, 2026
Merged

[1/5] VSA: Merge Elastic.Documentation.Api.* into single project#3333
Mpdreamz merged 6 commits into
mainfrom
refactor/merge-api-projects

Conversation

@Mpdreamz
Copy link
Copy Markdown
Member

@Mpdreamz Mpdreamz commented May 18, 2026

Why

The three-project split (Api.Core / Api.Infrastructure / Api.App) implemented a hexagonal layering that provided no external reuse benefit — all consumers are internal to this repo. The same split caused Mcp.Remote to accumulate a dependency on the full Api stack just to call one OTel helper and use two search interfaces, making it harder to reason about and maintain independently.

What

  • Elastic.Documentation.Api.Core, Api.Infrastructure, and Api.App are collapsed into a single Elastic.Documentation.Api project (SDK.Web, AOT-publishable, same AssemblyName=docs-builder-api).
  • EuidSpanProcessor, EuidLogProcessor, and a new AddEuidEnrichment helper move to Elastic.Documentation.ServiceDefaults so any web app in the repo can enrich spans/logs with the user euid without pulling in the full Api project.
  • The three search/changes gateway interfaces (IFullSearchGateway, INavigationSearchGateway, IChangesGateway) and their DTOs move to services/Elastic.Documentation.Search alongside their implementations — the Search service no longer references the Api project at all.
  • Mcp.Remote drops both Api.Core and Api.Infrastructure project references entirely; its deps are now ServiceDefaults + services/Search + services/Assembler only.

How

AddDocsApiOpenTelemetry in the merged project becomes a thin wrapper: it calls the new AddEuidEnrichment (which owns the Elastic OTel SDK init, service.version wiring, euid cookie enrichment, and processors) and then adds the five API-specific activity sources on top. Mcp.Remote calls AddEuidEnrichment directly instead.


Part of a move to vertical slice architecture to be able to cleanly lift search contract:

  1. [1/5] VSA: Merge Elastic.Documentation.Api.* into single project #3333 — Merge Elastic.Documentation.Api.* into single project
  2. [2/5] VSA: Collapse Gateway+Usecase into *Service per feature #3335 — Collapse Gateway+Usecase into *Service per feature
  3. [3/5] VSA: Slim Elastic.Documentation, extract heavy deps to Tooling #3345 — Slim Elastic.Documentation, extract heavy deps to Tooling
  4. [4/5] VSA: Slim Configuration to YAML schema; organize root namespaces #3347 — Slim Configuration to YAML schema; organize root namespaces
  5. [5/5] VSA: Extract Elastic.Documentation.Search.Contract as zero-dep library #3350 — Extract Elastic.Documentation.Search.Contract as zero-dep library

Mpdreamz and others added 2 commits May 18, 2026 12:01
Three projects (Api.Core, Api.Infrastructure, Api.App) collapsed into
Elastic.Documentation.Api. Mcp.Remote no longer references any Api.*
project — its only deps are ServiceDefaults, services/Search, and
services/Assembler.

Key moves:
- EuidSpanProcessor + EuidLogProcessor → ServiceDefaults (shared)
- AddEuidEnrichment helper added to ServiceDefaults
- Search gateway interfaces (IFullSearchGateway, INavigationSearchGateway,
  IChangesGateway) → services/Elastic.Documentation.Search
- McpToolSourceName constant inlined into Mcp.Remote

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
@Mpdreamz Mpdreamz requested a review from a team as a code owner May 18, 2026 10:17
@Mpdreamz Mpdreamz added the chore label May 18, 2026
@Mpdreamz Mpdreamz requested a review from reakaleek May 18, 2026 10:17
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 18, 2026

Review Change Stack

📝 Walkthrough

Walkthrough

This PR consolidates the documentation API from a three-project structure (Core, Infrastructure, App) into a single unified Elastic.Documentation.Api project while reorganizing namespaces and extracting shared infrastructure. Telemetry configuration moves to a shared Elastic.Documentation.ServiceDefaults package for reuse across services. Search gateway contracts relocate to the Elastic.Documentation.Search library, while use case implementations remain in the API but reorganized under non-Core namespaces. All downstream projects update their references and imports to align with the new structure.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title accurately describes the main change: merging three API projects into a single project.
Description check ✅ Passed The description comprehensively explains the motivation, what changed, and how the refactoring works. It clearly relates to the changeset.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
✨ Simplify code
  • Create PR with simplified code
  • Commit simplified code in branch refactor/merge-api-projects

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

coderabbitai[bot]
coderabbitai Bot previously requested changes May 18, 2026
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/api/Elastic.Documentation.Api/Telemetry/OtlpProxyOptions.cs (1)

35-45: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Fix environment variable key mismatch in ResolveEndpoint().

The documentation at lines 19–20 specifies OTEL_EXPORTER_OTLP_ENDPOINT as priority 2, but the implementation at line 36 reads OTLP_PROXY_ENDPOINT instead. The rest of the codebase (8+ locations including integration tests) uses OTEL_EXPORTER_OTLP_ENDPOINT, so following the documented contract will silently fail and fall back to localhost:4318.

Change line 36 to read OTEL_EXPORTER_OTLP_ENDPOINT, or if backward compatibility with OTLP_PROXY_ENDPOINT is needed, check both keys in priority order.

🤖 Prompt for 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.

In `@src/api/Elastic.Documentation.Api/Telemetry/OtlpProxyOptions.cs` around lines
35 - 45, The ResolveEndpoint() logic uses the wrong environment variable key;
change the envVarKey used in OtlpProxyOptions.ResolveEndpoint (currently set to
"OTLP_PROXY_ENDPOINT") to "OTEL_EXPORTER_OTLP_ENDPOINT" or implement a fallback
that checks both "OTEL_EXPORTER_OTLP_ENDPOINT" first then "OTLP_PROXY_ENDPOINT"
to preserve backward compatibility, and ensure the
Environment.GetEnvironmentVariable calls use that corrected key(s) so the method
returns the documented OTEL_EXPORTER_OTLP_ENDPOINT value instead of falling back
to the default.
🤖 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 `@src/api/Elastic.Documentation.Api/Dockerfile`:
- Around line 34-39: The Dockerfile hardcodes linux-x64 in the publish RID and
the COPY path; change the publish and copy to use the computed runtime
identifier instead of "linux-x64". Replace the RUN dotnet publish invocation to
use a derived RID variable (e.g. use build args TARGETOS/TARGETARCH to form RID
like "${TARGETOS}-${TARGETARCH}" and pass -r ${RID}) and update the COPY
--from=base target path from release_linux-x64 to release_${RID} (ensure the
same RID logic is used in both the RUN dotnet publish and the COPY --from=base
lines so arm64 builds publish and copy the matching artifact).

In `@src/api/Elastic.Documentation.Api/OpenTelemetry/OpenTelemetryExtensions.cs`:
- Around line 85-87: The ServiceDefaults.Telemetry.TelemetryConstants qualifier
may be unresolved; update the two uses in OpenTelemetryExtensions.cs (the calls
to activity.SetTag and activity.AddBaggage referencing
ServiceDefaults.Telemetry.TelemetryConstants.UserEuidAttributeName) so the
symbol resolves: either add the correct using/using alias that imports the
namespace which declares ServiceDefaults (or a global using alias if project
style requires) or fully qualify ServiceDefaults with its declaring namespace
where ServiceDefaults is defined; ensure both references (UserEuidAttributeName
in activity.SetTag and activity.AddBaggage) are changed consistently.

In `@src/api/Elastic.Documentation.Mcp.Remote/Program.cs`:
- Line 27: The tracer provider is not configured to capture MCP activities
because AddEuidEnrichment() does not register the
"Elastic.Documentation.Api.McpTools" ActivitySource; update the tracer
configuration where builder.AddEuidEnrichment() is called (or the
TracerProviderBuilder helper) to also call
AddSource("Elastic.Documentation.Api.McpTools") so the TracerProvider will
receive MCP activities from that ActivitySource.

In
`@src/Elastic.Documentation.ServiceDefaults/Telemetry/EuidEnrichmentExtensions.cs`:
- Around line 83-84: Replace the use of Assembly.GetCallingAssembly() when
building the informationalVersion in EuidEnrichmentExtensions (the variable
named informationalVersion) with Assembly.GetEntryAssembly() so telemetry
`service.version` reflects the host service's assembly instead of the shared
library; if you want to be defensive, call Assembly.GetEntryAssembly() first and
fall back to Assembly.GetCallingAssembly() only if EntryAssembly is null, then
continue to read the AssemblyInformationalVersionAttribute from that chosen
assembly.

---

Outside diff comments:
In `@src/api/Elastic.Documentation.Api/Telemetry/OtlpProxyOptions.cs`:
- Around line 35-45: The ResolveEndpoint() logic uses the wrong environment
variable key; change the envVarKey used in OtlpProxyOptions.ResolveEndpoint
(currently set to "OTLP_PROXY_ENDPOINT") to "OTEL_EXPORTER_OTLP_ENDPOINT" or
implement a fallback that checks both "OTEL_EXPORTER_OTLP_ENDPOINT" first then
"OTLP_PROXY_ENDPOINT" to preserve backward compatibility, and ensure the
Environment.GetEnvironmentVariable calls use that corrected key(s) so the method
returns the documented OTEL_EXPORTER_OTLP_ENDPOINT value instead of falling back
to the default.
🪄 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: CHILL

Plan: Enterprise

Run ID: bb359497-c6ad-4e03-b509-e98bc2ca8e81

📥 Commits

Reviewing files that changed from the base of the PR and between 26a419f and 065710c.

📒 Files selected for processing (92)
  • aspire/AppHost.cs
  • aspire/aspire.csproj
  • docs-builder.slnx
  • src/Elastic.Documentation.ServiceDefaults/Elastic.Documentation.ServiceDefaults.csproj
  • src/Elastic.Documentation.ServiceDefaults/Logging/EuidLogProcessor.cs
  • src/Elastic.Documentation.ServiceDefaults/Telemetry/EuidEnrichmentExtensions.cs
  • src/Elastic.Documentation.ServiceDefaults/Telemetry/EuidSpanProcessor.cs
  • src/Elastic.Documentation.ServiceDefaults/Telemetry/TelemetryConstants.cs
  • src/api/Elastic.Documentation.Api.App/Elastic.Documentation.Api.App.csproj
  • src/api/Elastic.Documentation.Api.Core/Elastic.Documentation.Api.Core.csproj
  • src/api/Elastic.Documentation.Api.Infrastructure/Elastic.Documentation.Api.Infrastructure.csproj
  • src/api/Elastic.Documentation.Api/Adapters/AskAi/AgentBuilderAskAiGateway.cs
  • src/api/Elastic.Documentation.Api/Adapters/AskAi/AgentBuilderStreamTransformer.cs
  • src/api/Elastic.Documentation.Api/Adapters/AskAi/AskAiGatewayFactory.cs
  • src/api/Elastic.Documentation.Api/Adapters/AskAi/AskAiProviderResolver.cs
  • src/api/Elastic.Documentation.Api/Adapters/AskAi/ElasticsearchAskAiMessageFeedbackGateway.cs
  • src/api/Elastic.Documentation.Api/Adapters/AskAi/KibanaOptions.cs
  • src/api/Elastic.Documentation.Api/Adapters/AskAi/LlmGatewayAskAiGateway.cs
  • src/api/Elastic.Documentation.Api/Adapters/AskAi/LlmGatewayOptions.cs
  • src/api/Elastic.Documentation.Api/Adapters/AskAi/LlmGatewayStreamTransformer.cs
  • src/api/Elastic.Documentation.Api/Adapters/AskAi/SseParser.cs
  • src/api/Elastic.Documentation.Api/Adapters/AskAi/StreamTransformerBase.cs
  • src/api/Elastic.Documentation.Api/Adapters/AskAi/StreamTransformerFactory.cs
  • src/api/Elastic.Documentation.Api/Adapters/Telemetry/AdotOtlpGateway.cs
  • src/api/Elastic.Documentation.Api/AskAi/AskAiEvent.cs
  • src/api/Elastic.Documentation.Api/AskAi/AskAiMessageFeedbackRequest.cs
  • src/api/Elastic.Documentation.Api/AskAi/AskAiMessageFeedbackUsecase.cs
  • src/api/Elastic.Documentation.Api/AskAi/AskAiUsecase.cs
  • src/api/Elastic.Documentation.Api/AskAi/IAskAiGateway.cs
  • src/api/Elastic.Documentation.Api/AskAi/IAskAiMessageFeedbackGateway.cs
  • src/api/Elastic.Documentation.Api/AskAi/IStreamTransformer.cs
  • src/api/Elastic.Documentation.Api/Aws/IParameterProvider.cs
  • src/api/Elastic.Documentation.Api/Aws/LambdaExtensionParameterProvider.cs
  • src/api/Elastic.Documentation.Api/Aws/LocalParameterProvider.cs
  • src/api/Elastic.Documentation.Api/Caching/CacheKey.cs
  • src/api/Elastic.Documentation.Api/Caching/DynamoDbDistributedCache.cs
  • src/api/Elastic.Documentation.Api/Caching/IDistributedCache.cs
  • src/api/Elastic.Documentation.Api/Caching/InMemoryDistributedCache.cs
  • src/api/Elastic.Documentation.Api/Caching/MultiLayerCache.cs
  • src/api/Elastic.Documentation.Api/Changes/ChangesUsecase.cs
  • src/api/Elastic.Documentation.Api/Dockerfile
  • src/api/Elastic.Documentation.Api/Elastic.Documentation.Api.csproj
  • src/api/Elastic.Documentation.Api/Gcp/GcpIdTokenProvider.cs
  • src/api/Elastic.Documentation.Api/Gcp/IGcpIdTokenProvider.cs
  • src/api/Elastic.Documentation.Api/MappingsExtensions.cs
  • src/api/Elastic.Documentation.Api/OpenTelemetry/OpenTelemetryExtensions.cs
  • src/api/Elastic.Documentation.Api/Program.cs
  • src/api/Elastic.Documentation.Api/Properties/launchSettings.json
  • src/api/Elastic.Documentation.Api/Search/FullSearchUsecase.cs
  • src/api/Elastic.Documentation.Api/Search/NavigationSearchUsecase.cs
  • src/api/Elastic.Documentation.Api/SerializationContext.cs
  • src/api/Elastic.Documentation.Api/ServicesExtension.cs
  • src/api/Elastic.Documentation.Api/Telemetry/IOtlpGateway.cs
  • src/api/Elastic.Documentation.Api/Telemetry/OtlpForwardResult.cs
  • src/api/Elastic.Documentation.Api/Telemetry/OtlpProxyOptions.cs
  • src/api/Elastic.Documentation.Api/Telemetry/OtlpProxyRequest.cs
  • src/api/Elastic.Documentation.Api/Telemetry/OtlpProxyUsecase.cs
  • src/api/Elastic.Documentation.Api/TelemetryConstants.cs
  • src/api/Elastic.Documentation.Api/Validation/IValidator.cs
  • src/api/Elastic.Documentation.Api/appsettings.Development.json
  • src/api/Elastic.Documentation.Api/appsettings.edge.json
  • src/api/Elastic.Documentation.Api/appsettings.json
  • src/api/Elastic.Documentation.Mcp.Remote/Elastic.Documentation.Mcp.Remote.csproj
  • src/api/Elastic.Documentation.Mcp.Remote/Program.cs
  • src/api/Elastic.Documentation.Mcp.Remote/Telemetry/McpToolTelemetry.cs
  • src/api/Elastic.Documentation.Mcp.Remote/Tools/CoherenceTools.cs
  • src/api/Elastic.Documentation.Mcp.Remote/Tools/SearchTools.cs
  • src/services/Elastic.Documentation.Search/ChangesGateway.cs
  • src/services/Elastic.Documentation.Search/Elastic.Documentation.Search.csproj
  • src/services/Elastic.Documentation.Search/FullSearchGateway.cs
  • src/services/Elastic.Documentation.Search/IChangesGateway.cs
  • src/services/Elastic.Documentation.Search/IFullSearchGateway.cs
  • src/services/Elastic.Documentation.Search/INavigationSearchGateway.cs
  • src/services/Elastic.Documentation.Search/MockSearchGateway.cs
  • src/services/Elastic.Documentation.Search/NavigationSearchGateway.cs
  • src/services/Elastic.Documentation.Search/ServicesExtension.cs
  • src/tooling/docs-builder/Http/DocumentationWebHost.cs
  • src/tooling/docs-builder/Http/StaticWebHost.cs
  • src/tooling/docs-builder/docs-builder.csproj
  • tests-integration/Elastic.Assembler.IntegrationTests/Elastic.Assembler.IntegrationTests.csproj
  • tests-integration/Elastic.Assembler.IntegrationTests/Search/SearchIntegrationTests.cs
  • tests-integration/Elastic.Documentation.Api.IntegrationTests/AskAiGatewayStreamingTests.cs
  • tests-integration/Elastic.Documentation.Api.IntegrationTests/Elastic.Documentation.Api.IntegrationTests.csproj
  • tests-integration/Elastic.Documentation.Api.IntegrationTests/EuidEnrichmentIntegrationTests.cs
  • tests-integration/Elastic.Documentation.Api.IntegrationTests/Fixtures/ApiWebApplicationFactory.cs
  • tests-integration/Elastic.Documentation.Api.IntegrationTests/OtlpProxyIntegrationTests.cs
  • tests-integration/Mcp.Remote.IntegrationTests/Mcp.Remote.IntegrationTests.csproj
  • tests-integration/Mcp.Remote.IntegrationTests/McpToolsIntegrationTestsBase.cs
  • tests-integration/Search.IntegrationTests/Search.IntegrationTests.csproj
  • tests/Elastic.Documentation.Api.Infrastructure.Tests/Adapters/AskAi/StreamTransformerTests.cs
  • tests/Elastic.Documentation.Api.Infrastructure.Tests/Caching/DistributedCacheTests.cs
  • tests/Elastic.Documentation.Api.Infrastructure.Tests/Elastic.Documentation.Api.Infrastructure.Tests.csproj
💤 Files with no reviewable changes (11)
  • src/api/Elastic.Documentation.Api.Core/Elastic.Documentation.Api.Core.csproj
  • src/api/Elastic.Documentation.Api.Infrastructure/Elastic.Documentation.Api.Infrastructure.csproj
  • src/services/Elastic.Documentation.Search/FullSearchGateway.cs
  • src/services/Elastic.Documentation.Search/MockSearchGateway.cs
  • src/services/Elastic.Documentation.Search/NavigationSearchGateway.cs
  • src/services/Elastic.Documentation.Search/ChangesGateway.cs
  • tests-integration/Mcp.Remote.IntegrationTests/McpToolsIntegrationTestsBase.cs
  • tests-integration/Mcp.Remote.IntegrationTests/Mcp.Remote.IntegrationTests.csproj
  • src/api/Elastic.Documentation.Mcp.Remote/Elastic.Documentation.Mcp.Remote.csproj
  • src/services/Elastic.Documentation.Search/Elastic.Documentation.Search.csproj
  • src/api/Elastic.Documentation.Api.App/Elastic.Documentation.Api.App.csproj

Comment thread src/api/Elastic.Documentation.Api/Dockerfile Outdated
Comment thread src/api/Elastic.Documentation.Mcp.Remote/Program.cs
Comment thread src/Elastic.Documentation.ServiceDefaults/Telemetry/EuidEnrichmentExtensions.cs Outdated
Mpdreamz and others added 3 commits May 18, 2026 12:26
…x-x64

Enables arm64 image builds to publish and copy the correct architecture
artifact rather than always using the amd64 binary.

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
…r key

- Register McpTools ActivitySource in Mcp.Remote TracerProvider so MCP
  spans are captured
- Use GetEntryAssembly() over GetCallingAssembly() for service.version so
  the host service's version is reported rather than ServiceDefaults
- OtlpProxyOptions now checks OTEL_EXPORTER_OTLP_ENDPOINT first then
  OTLP_PROXY_ENDPOINT as backward-compatible fallback, matching the
  documented priority order

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Member

@reakaleek reakaleek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wanted to share a different read on the motivation before we collapse the projects.

The split was never about external reuse. None of us expected another repo to consume Api.Core. It was there to enforce the
Clean Architecture dependency rule (Ports & Adapters): source dependencies point in one direction only, App -> Infrastructure
-> Core, and the use cases never depend on low level details like Elasticsearch, AWS, HTTP or OTel exporters.

A few reasons that matters:

  1. Dependency Inversion at the boundary. Core defines ports (IAskAiGateway, IFullSearchGateway, IDistributedCache) and the
    use cases depend on those interfaces. Infrastructure provides the adapters. Core owns the contract, Infrastructure conforms to it.
    The moment Core can see Infrastructure types, the arrow flips and the use case starts depending on the vendor.

  2. Stable depends on stable. The use cases (ask AI, search navigation, record feedback) are the most stable thing we have. The
    Elasticsearch client, DynamoDB SDK, GCP token provider and OTel exporter are the least stable, they version, get swapped, get
    reconfigured. The current layering means churn in the volatile layer can't ripple back into the stable one. Merge them and any
    infra change is free to touch a use case.

  3. Framework independence. Core doesn't reference Microsoft.AspNetCore.App, the ES client, AWS SDK or any OTel package, only
    logging and config abstractions. That's what lets us reason about and unit test the use cases without a web host or a live ES.
    Once everything is Microsoft.NET.Sdk.Web in one assembly, that property is gone.

  4. The compiler is the enforcement mechanism. Project boundaries are checked by the build, folders are checked by reviewers on a
    good day. "Core must not reference Infrastructure" is currently impossible to violate. After the merge it becomes a convention
    that decays the first time someone is in a hurry, and the symptom (a use case new-ing an ElasticsearchClient) won't show up
    until it has already shipped.

On Mcp.Remote, agreed the current shape is awkward, but I'd read it as evidence for the split rather than against it. The only
thing pulling Infrastructure into Mcp.Remote is the OTel helper. The search interfaces it uses are already satisfied by the tiny
Core assembly, because the ports live in Core. Your PR already does the right thing for that piece by lifting AddEuidEnrichment
into ServiceDefaults and moving the gateway interfaces into services/Elastic.Documentation.Search. Both moves are good on
their own, and after them Mcp.Remote drops its Infrastructure reference without needing the Api projects to merge. We could ship
those two extractions and leave Core/Infrastructure/App intact: Mcp.Remote ends up exactly as clean as the PR description targets,
the Api keeps its compile time guardrail, and the dependency rule still holds.

Happy to split the OTel/search extractions out as their own PR if that's useful, those wins don't require deleting the boundary.

@Mpdreamz
Copy link
Copy Markdown
Member Author

Appreciate the deep read — and I think we're closer than it looks, just optimizing on different axes.

The shape I'm working toward is vertical slice architecture, not the merge-and-flatten you might be reading PR1 as. Concretely:

  • App (the merged Api project) IS the infrastructure layer. Composition root, web host, adapters, all the stuff that bridges HTTP into the rest of the system. That's its job.
  • Implementations live in service assemblies under src/services/. Already the case for search (services/Elastic.Documentation.Search owns interfaces and impls). Each service is a vertical slice — one cohesive feature, all its parts together.
  • Service projects pragmatically co-locate the interface with the impl. When external reuse appears, we lift just the interface into a Contracts assembly. We don't pre-split for hypothetical sharers.
  • Cross-cutting infrastructure that's actually reused ends up in Elastic.Documentation.ServiceDefaults. Telemetry, health checks, OTel plumbing — exactly the kind of thing your PR1 feedback pushed me to extract from Api.Infrastructure, and rightly so.

On testability specifically:

that's what lets us reason about and unit test the use cases without a web host or a live ES

I don't think this depends on the project split. Tests depend on services — most of which do contain implementations, that's the practical reality — but the unit-test seam is the interface. We can mock IFullSearchService without a web host or a live ES regardless of which assembly its impl ships in. Framework-independent testability is a property of the abstraction (the interface), not of an enforced assembly boundary around it.

The thing your model gives up that mine doesn't is the compiler firewall: "use case cannot accidentally new ElasticsearchClient()" goes from build error to reviewer-enforced. That's a real cost, but in a single-executable Web app with one MappingsExtensions.cs funnel, the firewall hasn't been catching anything that PR review wouldn't. And the price was three types per feature (IGateway + Gateway + Usecase) with one impl in sight everywhere — the Usecase layer was a second interface dressed as a class.

I think both our shapes honor the load-bearing tenets of Clean Architecture — dependency inversion at the abstraction, framework-independent contracts, testable units, technology-free domain. We just optimize for different things. Your model optimizes for intra-app vendor isolation with the compiler as the enforcement mechanism. Mine optimizes for cross-repo sharing with the abstraction (interface or domain record) as the enforcement mechanism, and Contracts / services/* as the deployment boundary where reuse actually happens.

The concrete goal driving this is sharing our domain and service interfaces with elastic/website-search-data in a way that does not bleed implementations across. Contracts ships DocumentationDocument, IndexedProduct, slimmed ApplicableTo POCOs — no YAML, no diagnostics, no Elasticsearch client, no AspNetCore. The Search service interface can travel the same way when that repo needs it. The implementations stay home in services/Elastic.Documentation.Search and the App. That's the boundary that earns its keep, and it's the one the Core/Infrastructure/App split wasn't placing.

@Mpdreamz
Copy link
Copy Markdown
Member Author

One quick factual clarification on the Mcp.Remote framing:

The search interfaces it uses are already satisfied by the tiny Core assembly, because the ports live in Core.

IFullSearchGateway lived in Api.Core, but Mcp.Remote was only ever reaching through it to get at the search service. The actual implementation has been in services/Elastic.Documentation.Search the whole time, and Mcp.Remote already referenced that project to instantiate it. The Api.Core dependency wasn't satisfying a real abstraction need — it was an artifact of the port being placed in Core under the horizontal-layering model when it logically belonged with its implementation.

PR1 step 2 just relocated the interface to live next to its impl (services/Search), so Mcp.Remote consumes one project for one vertical slice — the search service — instead of one project for the interface and another for the impl. That's the vertical slice argument restated: Mcp.Remote was already depending on the search service, not on a Core abstraction layer. Reading the Mcp.Remote shape as evidence for the split assumes the port was load-bearing for the consumer; in practice the consumer never cared.

@Mpdreamz
Copy link
Copy Markdown
Member Author

One more on this specifically:

The search interfaces it uses are already satisfied by the tiny Core assembly, because the ports live in Core.

This isn't quite accurate. Mcp.Remote never depended on the Gateway or Usecase abstractions in Api.Core — its consumption pattern was always service-shaped. It reached for the search service, and that service has always lived in services/Elastic.Documentation.Search, which Mcp.Remote had a project reference to anyway (it has to, to instantiate anything). The IFullSearchGateway port lived in Api.Core only because the horizontal-layering model placed ports there; the actual implementation has always been one project away in services/Search.

So the pre-PR1 reality: Mcp.Remote referenced both Api.Core (for the interface) and services/Search (for the impl that satisfies it). One vertical concern, two projects. PR1 step 2 just moved the interface to live next to its impl — the same place Mcp.Remote was already reaching for code.

And critically: Mcp.Remote's consumption stays identical across PR1 and PR2. It depended on a service before, it depends on a service now (IFullSearchService). The Gateway and Usecase layers it never touched got collapsed; the service it actually uses remains. That's the vertical slice paying out — the consumer's coupling tracks the concept, not the layering.

Reading Mcp.Remote as evidence for the split assumes the Core port was load-bearing for that consumer. In practice it was an extra hop with no benefit — the impl wasn't swappable, the interface wasn't satisfied by anything other than what services/Search provides, and Mcp.Remote was never going to construct its own impl.

@Mpdreamz Mpdreamz changed the title Merge Elastic.Documentation.Api.* into single project [1/3] VSA: Merge Elastic.Documentation.Api.* into single project May 18, 2026
@Mpdreamz Mpdreamz changed the title [1/3] VSA: Merge Elastic.Documentation.Api.* into single project [1/5] VSA: Merge Elastic.Documentation.Api.* into single project May 19, 2026
@Mpdreamz
Copy link
Copy Markdown
Member Author

Mpdreamz commented May 19, 2026

@coderabbitai review

Co-authored-by: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 19, 2026

@Mpdreamz I'll kick off a review of the PR now.

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@Mpdreamz
Copy link
Copy Markdown
Member Author

@reakaleek and I discussed offline. Ultimately it boils down to Simple Clean Architecture vs Vertical Slice (Clean) Architecture.

Both solve the tenants of clean architecture, the change now to VSA was mostly precipicated because we already follow it for most or our src/services and that elastic/website-search-data only needs interfaces as contracts, not a paradigm through gateways and usecases which in this particular sharing story is not strictly needed.

Copy link
Copy Markdown
Member

@reakaleek reakaleek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed in live call

@Mpdreamz Mpdreamz merged commit 286866d into main May 19, 2026
24 checks passed
@Mpdreamz Mpdreamz deleted the refactor/merge-api-projects branch May 19, 2026 09:48
groman92 pushed a commit that referenced this pull request May 20, 2026
* Merge Elastic.Documentation.Api.* into single project

Three projects (Api.Core, Api.Infrastructure, Api.App) collapsed into
Elastic.Documentation.Api. Mcp.Remote no longer references any Api.*
project — its only deps are ServiceDefaults, services/Search, and
services/Assembler.

Key moves:
- EuidSpanProcessor + EuidLogProcessor → ServiceDefaults (shared)
- AddEuidEnrichment helper added to ServiceDefaults
- Search gateway interfaces (IFullSearchGateway, INavigationSearchGateway,
  IChangesGateway) → services/Elastic.Documentation.Search
- McpToolSourceName constant inlined into Mcp.Remote

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>

* Fix C# import ordering and name simplification after Api merge

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>

* Fix Dockerfile publish RID to use TARGETARCH/TARGETOS instead of linux-x64

Enables arm64 image builds to publish and copy the correct architecture
artifact rather than always using the amd64 binary.

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>

* Address PR review: OTel source registration, assembly version, env var key

- Register McpTools ActivitySource in Mcp.Remote TracerProvider so MCP
  spans are captured
- Use GetEntryAssembly() over GetCallingAssembly() for service.version so
  the host service's version is reported rather than ServiceDefaults
- OtlpProxyOptions now checks OTEL_EXPORTER_OTLP_ENDPOINT first then
  OTLP_PROXY_ENDPOINT as backward-compatible fallback, matching the
  documented priority order

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>

* Fix import ordering in Mcp.Remote Program.cs

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>

* [2/5] VSA: Collapse Gateway+Usecase into *Service per feature (#3335)

Co-authored-by: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants