feat OTel: added support rpc and pool metrics#642
Open
medvezhonokok wants to merge 9 commits intoydb-platform:masterfrom
Open
feat OTel: added support rpc and pool metrics#642medvezhonokok wants to merge 9 commits intoydb-platform:masterfrom
medvezhonokok wants to merge 9 commits intoydb-platform:masterfrom
Conversation
medvezhonokok
force-pushed
the
ydb-otel-tracing
branch
2 times, most recently
from
9bc2b1a to
0ac006e
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #642 +/- ##
============================================
- Coverage 71.03% 70.83% -0.20%
- Complexity 3352 3364 +12
============================================
Files 379 381 +2
Lines 15862 16052 +190
Branches 1664 1675 +11
============================================
+ Hits 11267 11371 +104
- Misses 3950 4032 +82
- Partials 645 649 +4 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
medvezhonokok
force-pushed
the
ydb-otel-tracing
branch
from
1cc6d9f to
9bc1f71
Compare
medvezhonokok
force-pushed
the
ydb-otel-tracing
branch
from
9bc1f71 to
75a421c
Compare
Drop the standalone opentelemetry/ module and the redundant SpanFinalizer
helper, revert unrelated regressions in TableClientImpl/SimpleTableClient
and the QueryTracingTest test reduction. Move OpenTelemetryMeter alongside
OpenTelemetryTracer in core (using the optional opentelemetry-api dep) and
relocate the integration test to query/test.
Implements the agreed-on metric set: ydb.client.operation.{duration,failed},
ydb.query.session.{create_time,pending_requests,timeouts,count,min,max} with
operation.name, db.response.status_code, ydb.query.session.{pool.name,state}
attributes. Pool name comes from QueryClient.Builder#sessionPoolName, default
is the connection's database. Retry-policy metrics are intentionally out of
scope for this iteration.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Operation metrics now carry only `database`, `endpoint`, `operation.name` (+ `status_code` for failed). Session metrics carry only `ydb.query.session.pool.name` (+ `ydb.query.session.state` for count). OpenTelemetryMeter.fromOpenTelemetry takes (database, endpoint) instead of (database, host, port). Drops `db.system.name` / `db.namespace` / `server.address` / `server.port` / `db.response.status_code` keys, replaces them with `database` / `endpoint` / `status_code`. Descriptions match the C# reference table verbatim. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Roll back the C#-style short descriptions to the original verbose English ones. The C# table was a reference for attribute layout, not for the description strings themselves. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…plete Drop the recordCommitMetrics / recordRollbackMetrics helpers — they wrapped the future in an extra .whenComplete just to record duration/failed. Master already terminates both chains with a .whenComplete handling currentStatus, so the metric calls fit there directly. For early-return paths (transactionId == null) the future is already completed, so duration is recorded synchronously without a wrapper. SessionPool.Handler#create keeps its dedicated .whenComplete: master has no existing one for the createSession stage, and reusing Span.endOnResult's internal one would mean duplicating its span-finalization logic. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
KirillKurdyukov
changed the title
KirillKurdyukov
force-pushed
the
ydb-otel-tracing
branch
from
4bdf613 to
189daf7
Compare
The previous version called .join() on the second createSession future and discarded the result, then asserted that the timeouts counter is non-zero. If the waiter ever completed via the queue path (e.g., a session became available before the 100ms timeout) the test would silently fail later with a misleading "ydb.query.session.timeouts metric not found" — there'd be no timeout to record. Now we explicitly assert that the waiter completed with CLIENT_DEADLINE_EXPIRED before checking the counter, give it 500ms (still short, but well above scheduler jitter), and read both pending and timeouts metrics from a single metricReader collection so the assertion order doesn't matter. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
added metrics