Skip to content

Add two Topic Reader state gauges - #734

Open
KirillKurdyukov wants to merge 9 commits into
masterfrom
codex/topic-reader-state-gauges
Open

KirillKurdyukov wants to merge 9 commits into
masterfrom
codex/topic-reader-state-gauges

Conversation

@KirillKurdyukov

@KirillKurdyukov KirillKurdyukov commented Oct 1, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Add two Topic Reader observable gauges, using the closeable API merged in #735:

  • ydb.topic.reader.partition_session.count, unit {session}.
  • ydb.topic.reader.credit_balance_bytes, unit By.

After successful stream initialization, ReaderMetrics.registerStream(stream)
registers callbacks reading that concrete stream. Each replacement closes both
previous registrations. Retry/end unregister the matching stream; Reader close
removes registrations permanently.
Registration takes a nonnull stream. A private closeGauges() performs shared cleanup;
there is no registerStream(null) command. Final close() also rejects future registrations.

No changes to TopicRetryableStream and no metric getters in ReaderImpl.
ReaderMetrics owns two registration fields, without a composite lambda or handles
on ReadSession. No additional accounting maps/counters or reading/retry changes.

Names, units and descriptions match the C# SDK. The two existing received-counter
descriptions are also aligned. Common attributes remain consumer and reader.name.

No Core, table/query pool or public settings changes.

Validation

Java21 clean package passed46 tests, no failures/errors/skips:

JAVA_HOME=$(/usr/libexec/java_home -v 21) mvn -B -pl topic -am \
  '-Dtest=OpenTelemetryMeterTest,ReaderMetricsTest,ReaderImplTest,SyncReaderImplTest,AsyncReaderImplTest,BufferManagerTest' \
  -Dsurefire.failIfNoSpecifiedTests=false clean package

All Reader metric tests are in ReaderMetricsTest: the existing counter case,
one gauge happy-path case, and a stale-source/closed-owner regression, with a shared Meter.
After extracting cleanup, all three ReaderMetricsTest cases and the SDK/real OTel
smoke were rerun successfully, including late init and concurrent shutdown.

Public TopicClient/SyncReader smoke with a real OpenTelemetry collector:

  • No gauge observations until successful stream initialization.
  • Partition count1; credit100->80->100 on receive/delivery.
  • Repeated init does not add callbacks.
  • Retry removes the old observations; reconnect registers the initialized replacement.
  • Closing one reader leaves the other; closing both leaves no gauge observations.
  • No sleeps/polling: the captured scheduler retry is invoked directly.
  • Shutdown from the init completion callback and a later InitResponse do not recreate gauges.
  • Native credit registration paused by an event while shutdown closes the RPC stream:
    after releasing/joining both workers, the real collector has no gauge observations.

LSP diagnostics and javac/checkstyle/package checks pass. Expected deprecated-API
warnings from compatibility tests remain.
Database-backed integration tests were not run locally: the Docker daemon is unreachable.
The SDK smoke uses fake network streams and the real native OpenTelemetry backend.

Lifecycle tradeoffs

  • A metric-only synchronized operation serializes the two native registrations and
    their close calls; no lock is added to message processing, counters or Reader state.
    Lifecycle callers can wait for a concurrent registration/backend operation.
  • One active stream per ReaderMetrics. Independent Readers use independent instances;
    a future federated reader must not share this single slot across concurrent streams.
  • Identity-checked unregister prevents old stream completion removing a newer source.
    The existing ReadSession closed flag and a terminal metrics flag reject late init.
  • Observations are absent before init/in retry gaps, rather than zero.
  • Gauge replacement is not a transactional two-instrument collector snapshot.
    Actual unregister requires a backend implementing closeable registrations;
    legacy Meter's default NOOP registration remains the existing compatibility limitation.

Tracker: https://st.yandex-team.ru/YDBAPPTEAM-2094.

@codecov

codecov Bot commented Oct 1, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 75.07%. Comparing base (01a0b6b) to head (f235c2a).
⚠️ Report is 4 commits behind head on master.

Additional details and impacted files
@@             Coverage Diff              @@
##             master     #734      +/-   ##
============================================
+ Coverage     74.70%   75.07%   +0.36%     
- Complexity     3669     3689      +20     
============================================
  Files           394      394              
  Lines         16633    16639       +6     
  Branches       1757     1755       -2     
============================================
+ Hits          12426    12491      +65     
+ Misses         3593     3533      -60     
- Partials        614      615       +1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@KirillKurdyukov
KirillKurdyukov force-pushed the codex/topic-reader-state-gauges branch from e909180 to cb1e0bb Compare October 1, 2026 13:46
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.

1 participant