PYTHON-5945 Add OpenTelemetry Simple Command Support#2946
Open
blink1073 wants to merge 21 commits into
Open
Conversation
Adds optional, opt-in OpenTelemetry tracing for server commands, conforming to the DRIVERS-719 client-side OpenTelemetry specification, by hooking into the existing consolidated command telemetry.
blink1073
changed the title
…Telemetry.failed Avoids computing isinstance(exc, OperationFailure) twice at the single call site now that the exception is already threaded through for span exception recording.
The OpenTelemetry feature isn't spec-test-complete yet (PYTHON-5947), so hold off announcing it in the changelog until that lands.
…719/APM mentions Keeps the module's comments focused on what the code does rather than referencing tickets or the separate command-monitoring API.
…t type opentelemetry-sdk was only needed for exercising the in-memory span exporter in tests, not for typing pymongo/_otel.py itself, so it no longer belongs in the shared test requirements. Instead: - opentelemetry (api + sdk) is available to the typing venv only. - test_otel.py is tagged with a new "otel" pytest marker, so it's excluded from the default test run like encryption/kms/etc. - "otel" is wired into utils.py/setup_tests.py/generate_config.py the same way other optional test suites are, installing opentelemetry-sdk only when that suite is selected.
Per spec, db.collection.name must be omitted for commands against the admin database. Several non-sensitive admin commands (dropUser, grantRolesToUser, renameCollection, setFeatureCompatibilityVersion, ...) carry a string command value that isn't a collection - a username, role name, or namespace - and were leaking into db.collection.name (and db.query.summary) instead of being omitted.
Every YAML/JSON test under specifications/open-telemetry/tests asserts an outer operation-level span with the command span only as a nested child, so none of them are runnable without operation-span support (deferred to PYTHON-5947). The spec's two prose tests are command/config-scoped only, so add them directly: env-var enable and disable, and env-var-driven db.query.text emission (a path that wasn't previously exercised - only the client-option path was).
- db.query.text truncation reused a blind full-string slice, almost
always producing invalid JSON; now truncates oversized field values
first (matching the existing log-message truncation approach), with
the blind slice only as a last-resort safety net.
- explain wraps the real command ({"explain": {"find": ...}}), the
same shape as getMore's indirection, but wasn't handled, so explain
spans silently lost db.collection.name.
- An explicit tracing.query_text_max_length=0 (meant to disable
db.query.text) was indistinguishable from "not configured" and got
silently overridden by the environment variable. The option is now
Optional[int]: unset defers to the environment variable, any
explicit value (including 0) always wins.
- server.port was set to None for Unix domain socket connections
(address[1] is None for .sock addresses), an invalid attribute type
that the OTel SDK would warn about and drop; the attribute is now
omitted instead. Also corrected _ConnectionTelemetryInfo.address's
type to match reality (_Address, not a non-optional int port).
test_span_created_for_insert_and_find and test_query_text_included_when_configured duplicate coverage the unified test format's find_without_query_text.yml/insert.yml and find.yml will provide once operation-span support and the expectTracingMessages runner land; flag them for removal at that point rather than leaving the redundancy undocumented.
…-up PR The tracing keyword argument remains fully functional (validated, threaded through to _CommandTelemetry); only its user-facing docstring documentation is deferred alongside the changelog entry.
start_span/end_span_success/end_span_failure only ever handle command spans; renaming to start_command_span/end_command_span_success/ end_command_span_failure now (a fully private, 6-call-site module) leaves unambiguous naming room for PYTHON-5947's operation and transaction span functions.
Tag otel-rhel8 with "pr" so the new test type is exercised on every pull request rather than only on mainline/manual runs, matching how other single-variant optional test suites (e.g. atlas-connect) are tagged.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
setup_tests.py only installed the coverage tooling and wrote COVERAGE to test-env.sh when --cov was passed as an explicit CLI flag, so a buildvariant-level COVERAGE expansion (the only mechanism available to generated variants like otel-rhel8) was silently a no-op: the "coverage" package was never installed, so `coverage run` would have failed had the env var actually been wired up. Fix setup_tests.py to also honor a pre-set COVERAGE env var, and tag otel-rhel8 with it so its coverage merges into the combined report instead of showing as ~0% patch coverage. Verified locally: with COVERAGE=1, setup-tests now adds `--group coverage` and writes COVERAGE=1 to test-env.sh, and running the suite reports 88% coverage for pymongo/_otel.py (vs. ~28% previously, when the otel-marked tests weren't measured with coverage at all).
Covers defaults, the enabled/query_text_max_length combination, the explicit-0-vs-unset distinction (0 must survive validation rather than being coerced away, since it's meaningful for overriding the environment variable), and each rejection path (non-mapping, unknown key, non-boolean enabled, non-integer/negative query_text_max_length). No live server or opentelemetry install required.
…tion The fallback slice appended "..." after already cutting to max_length, so the result could be up to 3 characters longer than the configured bound instead of respecting it. Carve the marker out of the budget instead of adding it on top.
…lidator tests - test_prose_1/test_prose_2 in test_otel.py: once PYTHON-5947 adds operation spans, these need to account for the outer operation span appearing alongside the command span (span counts and name-based lookups will need to disambiguate between the two). - TestValidateTracingOrNone in test_common.py: superseded once the unified test format's expectTracingMessages/observeTracingMessages tests exercise this validator indirectly through real client construction.
Colocate the temporary tracing-option validator tests with the rest of the otel-specific, PYTHON-5947-superseded test surface instead of test_common.py's general validator coverage. Note this moves them behind the "otel" pytest marker, so they now run alongside the other otel tests rather than under the default suite.
Member
Author
|
The ECS failure is unrelated, it was failing on the base commit. |
Contributor
There was a problem hiding this comment.
Pull request overview
Adds opt-in OpenTelemetry command-span tracing to PyMongo, wiring span creation into the existing command telemetry/logging pipeline and introducing an optional test/build lane to validate behavior without making OpenTelemetry a hard dependency.
Changes:
- Add
pymongo._otel(optional import) and integrate command-span start/end into_CommandTelemetryfor both sync/async command execution. - Introduce a validated
MongoClient(..., tracing=...)option (enabled,query_text_max_length) and plumb it through client options into command runners. - Add new
otel-marked tests (sync + async) plus packaging/CI plumbing for an optional OpenTelemetry dependency and an Evergreen buildvariant.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| test/test_otel.py | New synchronous integration tests for command-span creation, attributes, env var behavior, and query text truncation. |
| test/asynchronous/test_otel.py | Async mirror of the OpenTelemetry command-span tests. |
| requirements/opentelemetry.txt | Defines the optional opentelemetry extra dependency (API). |
| pyproject.toml | Registers the opentelemetry extra and the otel pytest marker. |
| pymongo/synchronous/command_runner.py | Pass tracing options into _CommandTelemetry and update failure handling signature. |
| pymongo/asynchronous/command_runner.py | Async equivalent of tracing-options plumbing and failure handling update. |
| pymongo/pool_shared.py | Adjust connection telemetry protocol type for address to _Address (supports Unix sockets). |
| pymongo/common.py | Add validate_tracing_or_none and register it in URI/client option validators. |
| pymongo/client_options.py | Store and expose validated tracing options on ClientOptions. |
| pymongo/_telemetry.py | Start/end spans alongside command logging/APM events via _CommandTelemetry. |
| pymongo/_otel.py | New optional OpenTelemetry implementation for command spans and related attributes. |
| justfile | Ensure typing env includes the OpenTelemetry extra and the SDK (for test tooling). |
| .evergreen/scripts/utils.py | Add otel to Evergreen test name mapping. |
| .evergreen/scripts/setup_tests.py | Map otel test lane to the opentelemetry extra; install SDK for tests; broaden coverage enablement. |
| .evergreen/scripts/generate_config.py | Add generated Evergreen buildvariant for the OTel test lane with coverage merging. |
| .evergreen/generated_configs/variants.yml | Generated buildvariant entry for running OTel tests in CI. |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 16 out of 16 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (4)
test/test_otel.py:39
- This test module imports
opentelemetry.sdkwheneveropentelemetry-apiis installed, sopip install .[opentelemetry](API-only) will raise ImportError during module import instead of skipping these tests. Gate the SDK imports separately and expose a boolean that the skip decorator can use.
if _otel._HAS_OPENTELEMETRY:
from opentelemetry import trace
from opentelemetry.sdk.trace import TracerProvider
from opentelemetry.sdk.trace.export import SimpleSpanProcessor
from opentelemetry.sdk.trace.export.in_memory_span_exporter import InMemorySpanExporter
test/test_otel.py:60
- The skip condition currently checks only
_otel._HAS_OPENTELEMETRY(API availability), but these tests requireopentelemetry-sdk. Use the SDK-availability boolean so the module can be imported safely when onlyopentelemetry-apiis installed.
@unittest.skipUnless(_otel._HAS_OPENTELEMETRY, "opentelemetry is not installed")
test/asynchronous/test_otel.py:39
- This test module imports
opentelemetry.sdkwheneveropentelemetry-apiis installed, sopip install .[opentelemetry](API-only) will raise ImportError during module import instead of skipping these tests. Gate the SDK imports separately and expose a boolean that the skip decorator can use.
if _otel._HAS_OPENTELEMETRY:
from opentelemetry import trace
from opentelemetry.sdk.trace import TracerProvider
from opentelemetry.sdk.trace.export import SimpleSpanProcessor
from opentelemetry.sdk.trace.export.in_memory_span_exporter import InMemorySpanExporter
test/asynchronous/test_otel.py:60
- The skip condition currently checks only
_otel._HAS_OPENTELEMETRY(API availability), but these tests requireopentelemetry-sdk. Use the SDK-availability boolean so the module can be imported safely when onlyopentelemetry-apiis installed.
@unittest.skipUnless(_otel._HAS_OPENTELEMETRY, "opentelemetry is not installed")
- network.transport: derive from address[1] is None (the same signal _Address already uses for "no port"), not a ".sock" suffix check on the host string, which can misclassify. - test_otel.py: guard opentelemetry.sdk imports with their own try/except, since opentelemetry-api can be installed without opentelemetry-sdk (that's the actual shape of the pymongo[opentelemetry] extra). Gate the test class on a separate _HAS_OTEL_TEST_DEPS flag so that case is skipped instead of crashing test collection. - validate_tracing_or_none: reject bool explicitly for query_text_max_length, since bool is an int subclass and would otherwise silently pass validation as 0/1. - _build_query_text: pass default=repr to json_util.dumps so tracing stays best-effort and doesn't raise for commands containing custom/codec-managed Python types. - start_command_span: guard lsid with isinstance(..., Mapping) before formatting it, so a malformed raw command can't crash span creation.
# Conflicts: # pymongo/_otel.py
…r tracing Now that this PR targets the shared "otel" integration branch rather than main directly, the command-span feature doesn't need to wait for PYTHON-5947 to be documented - restore the changelog entry and the MongoClient/AsyncMongoClient tracing option docstring that were previously deferred. This reverts commits 4fb56bb and 1450f05.
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
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.
PYTHON-5945
Changes in this PR
Targets a feature branch "otel".
Adds optional, opt-in OpenTelemetry tracing for server commands, conforming to the DRIVERS-719 client-side OpenTelemetry spec. Spec-driven unified test format compliance is deferred to PYTHON-5947 (every operation/transaction YAML test asserts an operation-level span, which is out of scope here); the spec's two command/config-scoped prose tests are included directly.
Test Plan
New tests in
test/test_otel.py(and async mirror) using an in-memory span exporter, including the spec's two prose tests (env-var enable/disable, env-var-drivendb.query.text), run under their ownotelpytest marker/Evergreen test type (like encryption/kms) so opentelemetry stays an optional dependency. Verified no regression in existing command logging/monitoring test suites. Manually verified span output against a live server with the OTel console exporter.Checklist
Checklist for Author
Checklist for Reviewer