Skip to content

feat(go): add diagnostic event subscriptions to TCP client - #4120

Draft
explise wants to merge 1 commit into
apache:masterfrom
explise:feat/3219-go-diagnostic-events
Draft

feat(go): add diagnostic event subscriptions to TCP client#4120
explise wants to merge 1 commit into
apache:masterfrom
explise:feat/3219-go-diagnostic-events

Conversation

@explise

@explise explise commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

Which issue does this PR address

Closes #3219

Rationale

The TCP client had // TODO publish event stubs at the connect/disconnect/
shutdown paths with no way for callers to observe connection or auth state
changes. This fills those in and is groundwork for a high-level IggyConsumer
abstraction: diagnostic events let the consumer detect disconnects and re-join
consumer groups eagerly on reconnect. They are also useful on their own for
application-level observability (logging drops, health metrics).

What changed?

  • DiagnosticEvent enum + String() in contracts/, and SubscribeEvents()
    on the Client interface.
  • A small non-blocking broadcaster in the TCP client fanning out to multiple
    independent subscribers, with an idempotent unsubscribe function.
  • Emission wired into connect (Connected), failed connect / invalidate /
    disconnect (Disconnected), shutdown (Shutdown, then channel close),
    login (SignedIn), logout (SignedOut) and leader redirection
    (Redirected).

Review feedback addressed

Rebased onto current master (adapting to the transportState/sessionState
split and the reworked register/settleOnLeader sign-in flow) and applied
the open review threads from #3621:

  • Redirected is now published after the teardown, not before.
  • Connected/Disconnected are published before releasing c.mtx.
  • Broadcaster cleanups: plain send after draining a slot, no once.Do, no
    closed-guard in unsubscribe, slices.Delete, and the broadcaster is held
    by value.
  • Integration tests use the SDK's existing env-based e2e helpers instead of a
    per-package testcontainers TestMain, removing the container-leak on
    startup failure.

Local Execution

  • Passed. gofmt, go build ./..., go vet ./... clean.
  • Unit tests pass (go test ./client/tcp/).
  • Integration tests pass against a live apache/iggy:edge server: all 5
    TestDiagnosticEvents_* cases (signed-in, signed-out, shutdown + channel
    close, multiple subscribers, full lifecycle).
  • Full module green with -race: IGGY_TCP_ADDRESS=... go test -race ./....

AI Usage

  1. Tool: Claude Code.
  2. Scope: Rebase/port onto current master, applying the prior review
    feedback, and test repair.
  3. Verification: Local go build/go vet/gofmt, unit tests, and the
    integration suite run against a real server, including -race.
  4. Can I explain every line? Yes.

Port client lifecycle events to the Go SDK, mirroring the Rust SDK's
DiagnosticEvent stream. Adds a DiagnosticEvent enum, SubscribeEvents() on
the Client interface, and a non-blocking fan-out broadcaster in the TCP
client.

Emission is wired into the transport and session transitions:
Connected on connect, Disconnected on connect failure/invalidate/disconnect,
Shutdown (and channel close) on close, SignedIn/SignedOut on login/logout,
and Redirected after a leader redirection is torn down.
@explise
explise force-pushed the feat/3219-go-diagnostic-events branch from 25dffb3 to 7054617 Compare September 10, 2026 12:17
@codecov

codecov Bot commented Sep 10, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 72.22222% with 20 lines in your changes missing coverage. Please review.
✅ Project coverage is 85.57%. Comparing base (df4c75d) to head (7054617).
⚠️ Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
foreign/go/contracts/diagnostic_event.go 0.00% 16 Missing ⚠️
foreign/go/client/tcp/tcp_core.go 85.71% 2 Missing ⚠️
foreign/go/client/tcp/tcp_events.go 94.87% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master    #4120      +/-   ##
============================================
- Coverage     86.02%   85.57%   -0.45%     
  Complexity     1451     1451              
============================================
  Files          1248     1250       +2     
  Lines        194879   194949      +70     
  Branches     160177   160177              
============================================
- Hits         167637   166829     -808     
- Misses        23141    24062     +921     
+ Partials       4101     4058      -43     
Components Coverage Δ
Rust Core 86.93% <ø> (ø)
Java SDK 67.56% <ø> (ø)
C# SDK 77.06% <ø> (ø)
Python SDK 91.33% <ø> (ø)
PHP SDK 85.65% <ø> (ø)
Node SDK 96.26% <ø> (ø)
Go SDK 56.27% <72.22%> (-13.13%) ⬇️
Files with missing lines Coverage Δ
foreign/go/client/tcp/tcp_session_management.go 86.82% <100.00%> (+0.19%) ⬆️
foreign/go/client/tcp/tcp_core.go 87.39% <85.71%> (+0.64%) ⬆️
foreign/go/client/tcp/tcp_events.go 94.87% <94.87%> (ø)
foreign/go/contracts/diagnostic_event.go 0.00% <0.00%> (ø)

... and 39 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(go): add diagnostic event subscription to client

1 participant