Skip to content

fix: honour Spanner emulator detection in outbox/inbox connections (#4162)#4225

Open
iancooper wants to merge 2 commits into
masterfrom
fix/4162-spanner-emulator
Open

fix: honour Spanner emulator detection in outbox/inbox connections (#4162)#4225
iancooper wants to merge 2 commits into
masterfrom
fix/4162-spanner-emulator

Conversation

@iancooper

Copy link
Copy Markdown
Member

Symptom

Run against the Spanner emulator, 60 Spanner Outbox/Inbox tests failed at fixture setup with:

System.InvalidOperationException : Your default credentials were not found.

The gRPC client tried to load Application Default Credentials instead of routing to SPANNER_EMULATOR_HOST. The 33 Spanner/BoxProvisioning/ tests passed because they already opt into emulator detection.

Confirmed root cause

SpannerConnectionProvider and SpannerUnitOfWork built new SpannerConnection(configuration.ConnectionString) without opting into EmulatorDetection. Because EmulatorDetection is opt-in and absent, the client falls back to ADC and throws. The passing BoxProvisioning tests work precisely because their production SpannerConnectionHelper sets EmulatorDetection.EmulatorOrProduction.

Evidence (code-trace, verified against the emulator):

  • The internal providers are exercised by the outbox/inbox test bodies (SpannerConnectionProvider.cs:22/30, SpannerUnitOfWork.cs:28/41), rebuilding from configuration.ConnectionString — so a test-object-only fix is insufficient; the fix must reach the connection string that round-trips through RelationalDatabaseConfiguration.
  • SpannerConnectionStringBuilder.EmulatorDetection serialises into .ConnectionString (Google.Cloud.Spanner.Data 5.12.0), so embedding it once propagates to every derived connection.
  • EmulatorDetection.EmulatorOrProduction is a no-op in real production — it only routes to the emulator when SPANNER_EMULATOR_HOST is set.

Fix

  • Production paritySpannerConnectionProvider and SpannerUnitOfWork now build their connection string via SpannerConnectionStringBuilder { EmulatorDetection = EmulatorDetection.EmulatorOrProduction }, matching the existing SpannerConnectionHelper.
  • Test-side — the Gcp test connection string (Const.ConnectionString) embeds EmulatorDetection so the pre-existing Spanner outbox/inbox tests reach the emulator.
  • Regression testSpannerConnectionProviderEmulatorTests builds the production provider from a bare connection string and asserts it routes to the emulator (was red with the ADC error, now green).

Verification (emulator)

  • New regression test: green.
  • --filter "FullyQualifiedName~Spanner" with collection parallelisation disabled: 94 passed / 0 failed (33 BoxProvisioning kept green — no regression; all 60 ADC failures resolved).

Out of scope — tracked separately (#4224)

Under a parallel dotnet test run, ~28 Spanner Outbox/Inbox tests still fail with FailedPrecondition: Schema change operation rejected because a concurrent schema change operation ... is already in progress. That is a different root cause — the emulator serialises DDL while those test classes run in parallel (unlike BoxProvisioning, which uses a shared [Collection]). It was previously masked by the ADC failure and only surfaced once emulator detection let the tests reach the DDL stage. Filed as #4224; each test passes in isolation / when serialised.

Fixes #4162

iancooper and others added 2 commits July 6, 2026 19:12
Root cause: SpannerConnectionProvider and SpannerUnitOfWork built a
SpannerConnection from the raw connection string without opting into
EmulatorDetection, so against the emulator the client fell back to
Application Default Credentials and threw "Your default credentials
were not found". Matching the existing SpannerConnectionHelper, they now
set EmulatorDetection.EmulatorOrProduction (a no-op in real production,
active only when SPANNER_EMULATOR_HOST is set).

Also embeds EmulatorDetection in the Gcp test connection string (Const)
so the pre-existing Spanner outbox/inbox tests reach the emulator.

Fixes #4162

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Ignore docker-compose-*.local.yaml (local-only host port remaps) and
cross-reference the separate Spanner emulator DDL-concurrency issue (#4224)
from the #4162 bugfix notes.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@claude

claude Bot commented Jul 6, 2026

Copy link
Copy Markdown

Code Review — PR #4225: Honour Spanner emulator detection in outbox/inbox connections

Thorough, well-diagnosed fix. The bugfix.md trace is excellent — the root-cause reasoning (EmulatorDetection is opt-in, serialises into the connection string, and must round-trip through RelationalDatabaseConfiguration into the internal providers) is convincing, and the fix mirrors the existing SpannerConnectionHelper pattern. A few observations, mostly minor.

Design / behaviour

  1. Unconditional override of a user-supplied EmulatorDetection (worth a deliberate decision). Both SpannerConnectionProvider and SpannerUnitOfWork now force EmulatorDetection = EmulatorOrProduction onto whatever connection string the caller provided. If a production user had explicitly set EmulatorDetection=None (the driver default) or ProductionOnly in their connection string — e.g. to guarantee the client never routes to a SPANNER_EMULATOR_HOST that happens to be set in the environment — this silently overrides that choice. Per CLAUDE.md's "do not change defaults beyond what was requested," this is a real (if small) change to production behaviour, not just a test fix.

    • It's defensible because it matches SpannerConnectionHelper (BoxProvisioning already does this) and EmulatorOrProduction is a no-op unless the env var is set. But consider only applying it when the keyword isn't already present, so an explicit caller choice is respected:
      ```csharp
      var builder = new SpannerConnectionStringBuilder(configuration.ConnectionString);
      if (!builder.ContainsKey("EmulatorDetection"))
      builder.EmulatorDetection = EmulatorDetection.EmulatorOrProduction;
      ```
      Not a blocker given the parity argument — but the override is the one behavioural decision here that deserves to be intentional rather than incidental.
  2. Null/empty connection string. Previously _connectionString = configuration.ConnectionString passed a null/empty value straight through; now it flows into new SpannerConnectionStringBuilder(...) in a field initializer. If a mis-configured caller passes null, the failure mode (and stack) changes. Low risk in practice — production always supplies a connection string — but flagging since the construction site moved earlier (field init, before GetConnection).

  3. Duplication (minor / DRY). The new SpannerConnectionStringBuilder(...) { EmulatorDetection = EmulatorOrProduction } idiom now appears in four places (provider, UoW, SpannerConnectionHelper, test Const). SpannerConnectionHelper is internal to a different project so sharing isn't free, but a small helper in Paramore.Brighter.Spanner that both the provider and UoW call would keep the two production copies from drifting.

Tests

  • The new SpannerConnectionProviderEmulatorTests is the right regression: it drives the production provider from a bare connection string (built inline rather than via Const, so it stays honest after Const is patched) and proves the emulator path end-to-end with SELECT 1. Good call documenting that it was RED with the ADC error before the fix.
  • Like the rest of the Spanner suite it requires live emulator infra ([Trait("Category", "Spanner")]) — consistent with existing conventions, so no concern, just noting it isn't a unit-level guard.

Scope / housekeeping

Summary

Correct, minimal, well-tested fix that resolves #4162. The only point I'd ask the author to consciously sign off on is #1 — the production providers now unconditionally impose EmulatorOrProduction, overriding any explicit EmulatorDetection a caller set. If that's the intended parity with BoxProvisioning, ✅ ship it; the conditional variant above is a cheap way to be a touch more conservative.

@iancooper iancooper self-assigned this Jul 6, 2026
@iancooper iancooper added 3 - Done .NET Pull requests that update .net code V10.X Agent Friendly labels Jul 6, 2026
Comment on lines +18 to 25
// Enable emulator detection so the connection routes to SPANNER_EMULATOR_HOST when it is set,
// and behaves as normal production otherwise. The keyword serialises into the connection string.
private readonly string _connectionString =
new SpannerConnectionStringBuilder(configuration.ConnectionString)
{
EmulatorDetection = EmulatorDetection.EmulatorOrProduction
}.ConnectionString;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should it be done in the IAmARelationalDatabaseConfiguration?

Like

new RelationalDatabaseConfiguration(new SpannerConnectionStringBuilder("MyConnection")
        {
            EmulatorDetection = EmulatorDetection.EmulatorOrProduction
        }.ConnectionString)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3 - Done Agent Friendly Bug .NET Pull requests that update .net code V10.X

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Spanner Outbox/Inbox tests fail against the emulator (missing EmulatorDetection on test SpannerConnection)

2 participants