Repository navigation
Closed
Conversation
ocelotl
force-pushed
the
docs_sdk
branch
2 times, most recently
from
October 31, 2026 14:27
594efcb to
a3058bc
Compare
Owner
Author
|
Superseded by the upstream pull request open-telemetry#5869 |
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.
Documents every hand-written module, class, function and method in
opentelemetry-sdk, so that each one carries a docstring, every parameter is documented, and every callable with a return value has aReturns:section.Base branch is
docs-base, a snapshot ofopen-telemetry/opentelemetry-python@3462000f3, so the diff shows only this work. This pull request is against theocelotlfork and is not proposed upstream.Nothing in
opentelemetry-sdk/srcis generated, so all 61 source files were in scope.What changed
61 files under
opentelemetry-sdk/src/opentelemetry/sdk/plusdocs/conf.py, +10466/-142, in 65 commits.The first 61 commits add the documentation, one file each, in reading order: the version module first, then each subpackage kept together, from the outside in. One file per commit means any single file can be reviewed on its own without reading around it.
The last four commits are repairs that the verification tools and a review pass found rather than the coverage measurement:
93c54f2d0keeps the nitpicky documentation build at its existing warning count.a54707dadcorrects eight docstring claims that the code contradicts, each checked against the line of code that disagrees with it. They are listed at the end of this description.e26068947restores fivetyping.Protocolmethod bodies that pyright requires, explained below.5a1bfa821documents eleven callables that an earlier revision of the coverage script could not see, because it did not descend into functions nested insideif,tryandwithblocks.Conventions followed
docs/conf.pydoes not setspecial-members, so an__init__docstring never renders. Constructor parameters therefore go on the class docstring, which is what the API reference shows, and__init__carries a short docstring saying so. That keeps one parameter list that cannot drift out of step with a second copy.New text names things as inline literals in double backticks rather than as Sphinx cross-reference roles.
docs/conf.py:102setsnitpicky = True, most of what these docstrings need to name is private, module-level data or a type alias, and none of those have a target to resolve to. Cross-reference roles that were already in the files are left as they were. Three new single-backtick names inopentelemetry-sdk/src/opentelemetry/sdk/resources/__init__.py, at lines 177, 178 and 326, are the exception: they nameResource,Resource.createandResourceDetector, the module docstring above them already links all three, and the documentation build confirms they resolve.Verification
opentelemetry-sdk/src, measured by walking the AST: 0 remaining gaps, down from 773 across 61 files.ruff checkandruff format --checkwith ruff 0.16.1, the version pinned indev-requirements.txt:23: pass on all 64 files underopentelemetry-sdk/src.pytest opentelemetry-sdk/tests: 973 passed, 1 skipped, 133 subtests passed.tox -e typecheck: 2 errors, both in the Python 3.14 standard library filestring/templatelib.py, outside this repository. The base commit reports the same two.tox -e lint-opentelemetry-sdk: 10.00/10, no messages.tox -e docs, which runssphinx-build -Wwithnitpicky = Trueset atdocs/conf.py:102: 16 warnings, the same 16 as the base commit, of the same four kinds (6_http.Compression, 6 ambiguousMeterProvider, 2types.Attributes, 2Attributes). This branch adds none.Things to check
Four decisions reach beyond adding a docstring.
docs/conf.pyis the one file outsideopentelemetry-sdk/srcthat changed.sphinx_autodoc_typehintsemits a type cross-reference for a parameter only once that parameter has a:param:entry, which napoleon generates from theArgs:section, so documenting the parameters turned seven private or incubating annotations into unresolvable references. They are now innitpick_ignore, next to the two private instrument classes already listed there for exactly the same reason. The seven areopentelemetry.sdk.trace._TracerConfig,opentelemetry.sdk.metrics._internal._MeterConfig,opentelemetry.sdk.metrics._internal._ProxyMeterConfig,opentelemetry.sdk.metrics._internal.measurement_consumer.MeasurementConsumer,opentelemetry.sdk._logs._internal._LoggerConfig,opentelemetry.sdk._logs._internal._logger_metrics.LoggerMetricsTandopentelemetry.semconv._incubating.attributes.otel_attributes.OtelComponentTypeValues. The alternative is to leave those parameters undocumented.Four modules gain
# pylint: disable=too-many-lines, because the docstrings take them past themax-module-lines=1000set at.pylintrc:268:opentelemetry-sdk/src/opentelemetry/sdk/resources/__init__.py:4,opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/__init__.py:4,opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/export/__init__.py:4andopentelemetry-sdk/src/opentelemetry/sdk/_configuration/__init__.py:5. Three other SDK modules already carry the same line, so this follows the repository's own answer rather than inventing one. The alternatives are raising the limit repository-wide or splitting the modules.Forty bodies whose only statement was
passor...lose that statement, because a docstring is already a body and both ruff rule PIE790 and pylintW0107/W2301reject the leftover once a docstring is there. Behaviour is identical in every case, and nopassthat was part of a larger body was touched.Five more were removed and then restored by
e26068947. They are methods of atyping.Protocol, where pyright requires the body: with the docstring as the whole body, the method is declared to return a value and returns on no code path, whichtox -e typecheckreports asreportReturnType. PIE790 does not fire inside aProtocolat all, so ruff never asked for those five to go. Each now carries... # pylint: disable=unnecessary-ellipsis, the form the repository already uses for this exact situation atopentelemetry-sdk/src/opentelemetry/sdk/trace/_sampling_experimental/_composable.py:45and:49. The five are in_configuration/__init__.py,trace/_sampling_experimental/_rule_based.pyandtrace/_tracer_metrics.py.opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/exponential_histogram/__init__.pywas a zero-byte file. It gains the Apache license header that every other file in the package carries, alongside its new module docstring.Documentation that was wrong and is now fixed
SpanLimitswrote six of itsDefault:lines as{_DEFAULT_...}inside a plain string, so the braces reached the rendered page verbatim, and three of the names in them do not exist in the module at all.get_aggregated_resourcessaidinitial_resource"has highest priority". It is merged first, so every detector overrides it, andResource.createrelies on that to let a detector override the defaulttelemetry.sdk.*attributes.MetricExporter.exportdocumented a parameter namedmetrics, which the signature does not have.should_sampledocstrings inexemplar_filter.pydocumented a parameter namedtimestamp. The signature calls ittime_unix_nano.Mapping.get_lower_boundarypointed its twoRaises:entries atopentelemetry.sdk.metrics.MappingUnderflowErrorandMappingOverflowError. Neither name exists; both classes live in themapping.errorsmodule and are not re-exported.Measurementdocumented its fields under anAttributes:heading whose continuation lines were not indented, so napoleon attached them to nothing.AggregationTemporalitysaid "Can be one of the following values:" and then listed none.ReadWriteLogRecord.dropped_attributesopened itsReturns:section with a line containing a colon, which napoleon reads as a type annotation, so "How many attributes were discarded" was emitted as a class reference.Docstring claims that a review pass found the code contradicting
Each was fixed in
a54707dadafter reading the line of code that disagrees with it.BatchProcessorhands the exporter the oldest item first, not the newest.emitappends on the left and the export loop pops from the right.BatchProcessorschedule delay of 0 removes the timeout rather than the wait. The worker blocks onEvent.wait(None)until anemitwakes it.BoundedDictmoves a rewritten key to the end, so eviction drops the key written longest ago rather than the key set first.GlobalErrorHandlerselects a handler when the handler class is a subclass of the class of the raised exception, the reverse of the relation the docstring stated._ViewInstrumentMatchstarts a new aggregation at the current time unless the stream resets on delta collection and has been collected before. The default preferences are cumulative, so no stream resets.PeriodicExportingMetricReader.shutdowncollects nothing of its own, and there is no exporting thread at all when the export interval is infinite.PeriodicExportingMetricReader.shutdownpasses the remaining budget under the keywordtimeoutwhileMetricExporter.shutdowndeclarestimeout_millis, so the value is swallowed by**kwargs._BaseConfigurator.configureholds no guard, so calling it twice runs_configuretwice._import_opampcatchesStopIterationandAttributeErroronly, so anImportErrorfrom loading the entry point propagates.