Conversation
Query-service retries now emit an umbrella INTERNAL ydb.RunWithRetry span and a ydb.Try INTERNAL span per attempt. Each ydb.Try carries the ydb.retry.backoff_ms attribute (the sleep preceding the attempt — 0 for the first one, i.e. the next-attempt timeline includes the backoff). Retriable exceptions are recorded on the owning ydb.Try span, and an exception that escapes the whole retry loop (including an asyncio.CancelledError hitting a backoff sleep) is recorded on the outer ydb.RunWithRetry span. CLIENT spans (ydb.CreateSession, ydb.ExecuteQuery, ydb.Commit, ydb.Rollback) now also emit network.peer.address / network.peer.port for the concrete node serving the session, while server.address / server.port keep meaning the host from the connection string. Also fixes a "Пр" typo in docs/opentelemetry.rst and corrects span names (ydb.CommitTransaction -> ydb.Commit, ydb.RollbackTransaction -> ydb.Rollback). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Move ydb.RunWithRetry / ydb.Try span emission directly into retry_operation_sync / retry_operation_async in ydb/retries.py, and drop the short-lived ydb.query._retries shim. Tracing is still no-op by default, so there is no cost for the table-service callers that share the same retry loop; we just stop duplicating the retry logic to add spans. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…p session.id/tx.id RPC (CLIENT-kind) spans now carry the peer metadata from the discovery endpoint map, not from the grpc-target string of the request: * network.peer.address = EndpointInfo.address (the node host) * network.peer.port = EndpointInfo.port * ydb.node.dc = EndpointInfo.location To do that, EndpointOptions and Connection now also carry address/port/ location populated by resolver.endpoints_with_options(); sessions resolve their peer tuple via driver._store.connections_by_node_id after CreateSession returns, which is the right place to ask which node owns this session. Dropped the noisy ydb.session.id and ydb.tx.id attributes - they pollute every span and are recoverable from trace context if really needed. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
vgvoleg
left a comment
vgvoleg
left a comment
There was a problem hiding this comment.
Поправьте замечания и сделайте rebase на свежий main.
|
|
||
| The :doc:`coordination` page covers distributed semaphores and leader election. If you | ||
| need to limit concurrent access to a shared resource across multiple processes or hosts, | ||
| need to limit concurrent access to aЗе shared resource across multiple processes or hosts, |
There was a problem hiding this comment.
Опечатка, похоже на след ручного разрешения конфликта: aЗе вместо a — две русские буквы посреди английского слова.
| attributes=attributes or {}, | ||
| ) | ||
| ctx = trace.set_span_in_context(span) | ||
| token = context.attach(ctx) |
There was a problem hiding this comment.
context.attach(ctx) здесь активирует контекст на всё время жизни TracingSpan, а detach(token) дергается в end() — в том числе из SyncResponseContextIterator.__del__ (см. ydb/query/base.py) и из __exit__.
Две проблемы:
- OTel
context.attach/detach— per-taskContextVar. Если итератор закрывается в другой asyncio-таске (а для__del__— почти всегда в GC-потоке),detachплюнет warning'омFailed to detach contextи будет логический шум на каждый стрим. - Пока стрим не закрыт, контекст со спаном активен глобально. При параллельных стримах detach'и пойдут не в LIFO-порядке — OTel это тоже логирует предупреждением.
Чище не держать attach на всё время стрима: оборачивать start_as_current_span'ом только участок, где делается RPC (чтобы inject() в _construct_metadata увидел правильный контекст), а дальше работать со спаном без активного контекста — set_attribute/set_status/end в активности контекста не нуждаются.
| self._span = None | ||
|
|
||
| def __del__(self): | ||
| self._finish_span() |
There was a problem hiding this comment.
Финализация спана через __del__ хрупкая: GC вызывает этот метод из произвольного потока, а TracingSpan.end() делает context.detach(token) для токена, привязанного к asyncio-таске или потоку создания. В лучшем случае — warning'и в логах, в худшем — потеря атрибутов последнего стрима.
Связано с комментарием в _plugin.py::_create_span: если развести spanlifetime и context attach/detach, этот __del__ становится безопасным — span.end() без detach переживает любой поток без ворчания.
|
|
||
| if yield_sleep: | ||
| yield YdbRetryOperationSleepOpt(retriable_info.sleep_timeout_seconds) | ||
| yield YdbRetryOperationSleepOpt(retriable_info.sleep_timeout_seconds, exception=e) |
There was a problem hiding this comment.
При retriable-ошибках из skip_yield_error_types (Aborted, BadSession, NotFound, InternalError) здесь yield'а не происходит. В retry_operation_sync новый try_span стартуется только в ветке isinstance(next_opt, YdbRetryOperationSleepOpt), так что на серии, например, Aborted → Aborted → success получится один ydb.Try спан, который:
- покрывает все попытки разом,
- не имеет
set_errorдля провалившихся, - с
backoff_ms=0от первой попытки.
Тест test_retry_backoff_ms_on_each_try гоняет Unavailable, этот путь не проходит — баг не ловит. В async варианте фактически работает, потому что next_opt.set_exception(e) приводит к re-raise внутри impl, и там уже нормальный путь с yield.
Решение: либо всегда отдавать YdbRetryOperationSleepOpt(0.0, exception=e) (сна нет, но маркер для спана есть), либо закрывать/открывать try_span и в skip-yield ветке снаружи.
| map for the specific node serving the call; missing fields are skipped. | ||
| Can be used as a context manager or manually. | ||
| """ | ||
| attrs = _build_ydb_attrs(driver_config, node_id, peer) |
There was a problem hiding this comment.
create_ydb_span безусловно строит attrs через _build_ydb_attrs, даже когда _registry._create_span_func is None и результат будет отброшен в _NOOP_SPAN. На каждый ExecuteQuery/Commit/Rollback — лишняя аллокация dict и несколько getattr. Не катастрофа, но обещанный в docs/opentelemetry.rst:10 «zero-cost when disabled» нарушен.
Early-exit в _registry.create_span (или флаг enabled на реестре) до построения атрибутов — и будет честно.
|
|
||
| def _split_endpoint(endpoint): | ||
| endpoint = endpoint or "" | ||
| host, _, port = endpoint.rpartition(":") |
There was a problem hiding this comment.
rpartition(':') на "grpc://host.example.com:2136" даёт host = "grpc://host.example.com" — схема попадает в server.address. По OTel SemConv там должно быть DNS-имя / IP, без URL-схемы.
Плюс граничный кейс IPv6: "[::1]:2136" отработает случайно (последнее : перед портом), а gRPC-формат "ipv6:[::1]:2136" — уже нет.
Тест test_endpoint_parsing в tests/tracing/test_tracing_sync.py явно фиксирует текущее (некорректное) поведение со схемой — его тоже надо поправить.
| def _enable_tracing(tracer=None): | ||
| global _enabled, _tracer | ||
|
|
||
| if _enabled: |
There was a problem hiding this comment.
enable_tracing() молча идемпотентна — второй вызов (в т.ч. с другим tracer) не делает ничего. В тестах это обходится прямой правкой _plugin._enabled = False, но пользователь такой путь не найдёт.
Либо задокументировать это прямо в docstring (сейчас описание принимает tracer, но не говорит, что смысл имеет только первый вызов), либо сделать replace-семантику, либо добавить парный disable_tracing() для смены/сброса.
3 participants
Pull request type
What is the current behavior?
The YDB Python SDK does not provide built-in OpenTelemetry tracing support. There is legacy integration with OpenTracing API which uses the deprecated standard.
Issue Number: N/A
What is the new behavior?
Adds OpenTelemetry tracing support to the YDB Python SDK. When enabled via enable_tracing(), the SDK automatically creates spans for key operations:
Each span includes standard attributes: db.system.name, db.namespace, server.address, server.port, ydb.session.id, ydb.node.id, ydb.tx.id.
W3C Trace Context (traceparent) is automatically propagated in gRPC metadata, enabling end-to-end distributed tracing between client and YDB server. Execute spans cover the full operation lifecycle, including streaming result iteration — not just the initial gRPC call. Errors are recorded on spans with error.type, db.response.status_code, and exception events.
Tracing is opt-in (pip install ydb[tracing] + enable_tracing()). Without it, the SDK behavior is unchanged — all tracing code paths are no-op.
Other information