Skip to content

Document all hand-written source in exporter/opentelemetry-exporter-zipkin-json - #57

Draft
ocelotl wants to merge 9 commits into
docs-basefrom
docs_exporter_zipkin_json
Draft

ocelotl wants to merge 9 commits into
docs-basefrom
docs_exporter_zipkin_json

Conversation

@ocelotl

@ocelotl ocelotl commented Oct 30, 2026 •

Copy link
Copy Markdown
Owner

Documents every hand-written module, class, function and method in exporter/opentelemetry-exporter-zipkin-json, so that each one carries a docstring, every parameter is documented, and every callable with a return value has a Returns: section.

Base branch is docs-base, a snapshot of open-telemetry/opentelemetry-python@3462000f3, so the diff shows only this work. This pull request is against the ocelotl fork and is not proposed upstream.

What changed

7 files, +592/-22, in 17 commits. The first six document one module each, so each can be read on its own:

  • 27ede05fe Document opentelemetry.exporter.zipkin.json.version
  • 714fd2071 Document opentelemetry.exporter.zipkin.json
  • e9b272afe Document opentelemetry.exporter.zipkin.json.v1
  • d107a8ee9 Document opentelemetry.exporter.zipkin.json.v2
  • adfea94df Document opentelemetry.exporter.zipkin.encoder
  • 5e957ca2b Document opentelemetry.exporter.zipkin.node_endpoint
  • fed63e6d8 Render the Zipkin encoder and node endpoint modules in the API reference
  • a56b69bdb Correct the Zipkin encoder docstrings that misdescribed truncation and endpoint placement
  • 68d746303 Correct the documented conditions for dropping a tag and replacing the service name
  • ab1d10890 Document the AttributeError in the parent id helper and the per-batch service name replacement
  • f4a60241c State the truncation condition, the parent id None case and the v1 endpoint placement
  • 86ebf33ef Record the KeyError on a resource with no service name, and fix two scope claims
  • b8cbc671d Bound the scope bypass claim to v2 and document the timeout parsing failure
  • fcb6da40d Bound the integer address spelling and two Raises conditions
  • dc0f676a5 Render the v1 and v2 JSON encoder modules in the API reference
  • 1763ea8f1 Name the full escape rule, the empty endpoint variable and the missing parent attribute
  • d95b10571 Document the AttributeError an event with no attributes produces

The whole package is hand-written. None of its 6 source files matches the generated-file rules, so all 6 are in scope.

Every commit after fed63e6d8 corrects docstrings whose claims did not match the code, each one found by running the code the docstring describes.

  • a56b69bdb fixes the claim that the v1 truncation of scope, status and dropped count tags is a no-op, the claim that the endpoint always becomes localEndpoint, and the silence about the v1 TypeError on the rendered ZipkinExporter page.
  • 68d746303 fixes the condition under which a sequence tag is dropped and the claim that the service name is replaced only once.
  • ab1d10890 records the AttributeError that Encoder._get_parent_id raises for a root Span, and restates the service name replacement on ZipkinExporter.export to match.
  • f4a60241c adds the positive-limit condition to the v1 truncation sentence, drops the unreachable SpanContext case from the Encoder._get_parent_id return description, and names the binary annotations alongside the annotations as the place the v1 encoder writes the local endpoint.
  • 86ebf33ef records the KeyError both constructors raise when the globally registered tracer provider carries a resource with no service.name, replaces a scope example that TracerProvider.get_tracer cannot produce, and adds the non-empty condition to the service name overwrite.
  • b8cbc671d bounds the scope truncation bypass to v2, since V1Encoder._extract_binary_annotations cuts the same value again, and records the ValueError a non-integer OTEL_EXPORTER_ZIPKIN_TIMEOUT raises out of the constructor.
  • fcb6da40d bounds the integer spelling of an IPv6 address, since ipaddress.ip_address reads an integer below 2**32 as IPv4, and adds the missing condition to two Raises: entries.
  • 1763ea8f1 adds control characters to the list of characters that beat the running-length count, qualifies the OTEL_EXPORTER_ZIPKIN_ENDPOINT fallback for an empty value, and names NonRecordingSpan, which has no parent attribute at all.
  • d95b10571 records the AttributeError an opentelemetry.sdk.trace.Event built without attributes produces, which Span.add_event never does.

fed63e6d8 and dc0f676a5 are the only changes outside the package, both to docs/exporter/zipkin/zipkin.rst, which previously rendered opentelemetry.exporter.zipkin.json and nothing else from this distribution.

Moving the ZipkinExporter constructor parameters onto the class docstring made sphinx_autodoc_typehints emit the type of the version parameter, and opentelemetry.exporter.zipkin.encoder.Protocol had no page to resolve to, which the nitpicky build reported. fed63e6d8 adds automodule directives for opentelemetry.exporter.zipkin.encoder and opentelemetry.exporter.zipkin.node_endpoint, which documents Protocol, makes the reference resolve, and puts the new Encoder, JsonEncoder and NodeEndpoint docstrings in the API reference, where they were previously invisible.

dc0f676a5 adds the same directives for opentelemetry.exporter.zipkin.json.v1 and opentelemetry.exporter.zipkin.json.v2. The JsonEncoder docstring tells the reader that _encode_span is supplied by JsonV1Encoder and JsonV2Encoder, and neither class had a page to reach, so the new V1Encoder, JsonV1Encoder and JsonV2Encoder docstrings appeared nowhere in the built documentation. automodule skips imported names unless :imported-members: is given, so the two blocks add no duplicate object description, and the built page carries no duplicate HTML id and no dangling internal anchor.

Conventions followed

docs/conf.py does not set special-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. This moved the ZipkinExporter parameters off __init__, and added Args: sections to Encoder, JsonEncoder, V1Encoder and JsonV2Encoder, which inherit the max_tag_value_length constructor parameter.

The ZipkinExporter Args: block previously ended with an unlabelled paragraph about the (local_node_ipv4, local_node_ipv6, local_node_port) tuple, indented as though it were part of the parameter list. It is now prose on the class docstring, above the Args: section, where napoleon renders it as prose.

Cross-references are written as inline literals in double backticks rather than as :class: roles wherever the target has no autodoc page, since docs/conf.py:102 sets nitpicky = True and tox -e docs passes -W.

Verification

  • Docstring coverage over exporter/opentelemetry-exporter-zipkin-json/src, measured by walking the AST: 0 remaining gaps, down from 34. The 34 broke down as 1 missing module docstring, 3 missing class docstrings, 26 missing function or method docstrings, 2 callables with undocumented parameters and 2 callables with no Returns: section.

  • ruff check and ruff format --check with ruff 0.16.1, the version pinned in dev-requirements.txt:23: pass on all 6 files.

  • tox -e py312-test-opentelemetry-exporter-zipkin-json: 53 passed.

  • tox -e lint-opentelemetry-exporter-zipkin-json: 10.00/10, no messages. No # pylint: disable= pragma was added.

  • tox -e docs, which runs sphinx-build -W with nitpicky = True set at docs/conf.py:102: 16 warnings, the same 16 as the base, of the same four kinds. All 16 are in opentelemetry-api and opentelemetry-exporter-otlp-proto-http, neither of which this branch touches:

    6  py:class reference target not found: _http.Compression
    6  more than one target found for cross-reference 'MeterProvider'
    2  py:class reference target not found: types.Attributes
    2  py:class reference target not found: Attributes
    

    Without the fed63e6d8 repair the count was 17, the extra one being py:class reference target not found: opentelemetry.exporter.zipkin.encoder.Protocol.

Things to decide

A real bug, documented but deliberately not fixed

exporter/opentelemetry-exporter-zipkin-json/src/opentelemetry/exporter/zipkin/json/v1/__init__.py:59 reads if isinstance(tag_value, str) and self.max_tag_value_length > 0:. It does not guard against max_tag_value_length being None, which is exactly what ZipkinExporter.__init__ passes on when its own max_tag_value_length argument is left unset. Every sibling comparison in opentelemetry/exporter/zipkin/encoder/__init__.py writes self.max_tag_value_length is not None and self.max_tag_value_length > 0.

The effect is that ZipkinExporter(version=Protocol.V1) raises TypeError: '>' not supported between instances of 'NoneType' and 'int' on the first export of any span that contributes a tag. A span always contributes tags, because the resource always carries telemetry.sdk.* attributes, so the v1 protocol is unusable at its default configuration. Reproduced on this branch:

from opentelemetry.exporter.zipkin.json.v1 import JsonV1Encoder
from opentelemetry.exporter.zipkin.node_endpoint import NodeEndpoint
from opentelemetry.sdk.trace import TracerProvider

tracer = TracerProvider().get_tracer("t")
with tracer.start_as_current_span("s") as span:
    pass
JsonV1Encoder(None).serialize([span], NodeEndpoint())

This is a documentation change, so the behaviour is left alone and the documentation describes it instead: the max_tag_value_length entry on the ZipkinExporter class docstring, a Raises: block on ZipkinExporter.export, and Raises: blocks on V1Encoder._extract_binary_annotations and JsonV1Encoder._encode_span. It deserves its own issue and its own fix.

Seven further defects, also documented rather than fixed

Each one was found by running the code the docstring describes, and each one is recorded in the docstring of the callable that carries it rather than repaired. Every path and line number below was read off the head of this branch.

where what happens where it is documented
src/opentelemetry/exporter/zipkin/encoder/__init__.py:201 _get_parent_id reads span_context.parent.span_id on its Span branch without checking that parent is set, so a root Span raises AttributeError: 'NoneType' object has no attribute 'span_id' and a NonRecordingSpan, which has no parent attribute at all, raises AttributeError: 'NonRecordingSpan' object has no attribute 'parent'. Both encoders pass span.parent, so the exporter never reaches it. the Raises: block of Encoder._get_parent_id
src/opentelemetry/exporter/zipkin/node_endpoint.py:83 resource.attributes[SERVICE_NAME] is a plain subscript, so NodeEndpoint() and ZipkinExporter() both raise KeyError: 'service.name' when the globally registered tracer provider carries a resource without that attribute, which TracerProvider(resource=Resource.get_empty()) produces. the Raises: blocks of NodeEndpoint and ZipkinExporter
src/opentelemetry/exporter/zipkin/json/__init__.py:166 the value of OTEL_EXPORTER_ZIPKIN_TIMEOUT goes straight to int, so a variable holding 10s or the empty string raises ValueError out of the constructor instead of falling back to 10 seconds. the timeout entry and the Raises: block of ZipkinExporter
src/opentelemetry/exporter/zipkin/encoder/__init__.py:307 _extract_tag_value_string_from_sequence counts len(str(element)) before json.dumps escapes anything, so an element holding a non-ASCII character, a quote, a backslash or a control character makes the rendered array exceed max_tag_value_length. The caller then cuts it mid-escape: JsonV2Encoder(9) on ["\n\n\n\n\n"] yields a tag value that json.loads rejects. the escaping paragraph of _extract_tag_value_string_from_sequence
src/opentelemetry/exporter/zipkin/encoder/__init__.py:411 _extract_annotations_from_events reads event.attributes.items() without guarding, and opentelemetry.sdk.trace.Event defaults attributes to None, so an Event built directly without attributes raises AttributeError. Span.add_event stores an empty mapping, so a span recorded through the SDK never reaches it. the Raises: block of _extract_annotations_from_events
src/opentelemetry/exporter/zipkin/node_endpoint.py:153 ipaddress.ip_address reads an integer by its magnitude, so an IPv6 address given as an integer below 2**32 parses as IPv4 and the ipv6 setter rejects it. NodeEndpoint(ipv6=1), which is ::1, raises ValueError. the IpInput docstring and the ipv6 setter
src/opentelemetry/exporter/zipkin/json/__init__.py:161-164 self.encoder is assigned only for Protocol.V1 and Protocol.V2, so any other version makes the next line raise AttributeError: 'ZipkinExporter' object has no attribute 'encoder'. The parameter is annotated Protocol and Protocol has only those two members, so no well-typed call reaches it. not documented, for the reason in the previous sentence

All paths in the table are relative to exporter/opentelemetry-exporter-zipkin-json.

Bad documentation in a neighbouring package, left alone

exporter/opentelemetry-exporter-zipkin-proto-http/src/opentelemetry/exporter/zipkin/proto/http/__init__.py documents a version parameter in the Args: section of ZipkinExporter.__init__ that its signature does not have. That package is outside the scope of this pull request, so it was not touched.

@ocelotl
ocelotl force-pushed the docs_exporter_zipkin_json branch from 3bc983c to ab1d108 Compare October 30, 2026 22:50
RichardoMrMu and others added 9 commits November 2, 2026 03:44
…pen-telemetry#5704)

* fix(zipkin): tolerate invalid OTEL_EXPORTER_ZIPKIN_TIMEOUT env var

Both Zipkin exporters parsed OTEL_EXPORTER_ZIPKIN_TIMEOUT with a bare
int() in __init__, so an invalid value (e.g. "10s") raised ValueError
during exporter construction and took SDK startup down, unlike the OTLP
HTTP/gRPC exporters which warn and fall back to the default.

Add _timeout_from_env() in each exporter that skips unset/empty values
silently, logs a warning for non-integer values, and falls back to the
10 second default. Explicit timeout argument precedence is unchanged.

Assisted-by: Doubao

* style(zipkin): inline timeout assignment in both exporters

Collapse the two-line timeout resolution into a single assignment per
review nit; no behavior change.

Assisted-by: Doubao

* test(zipkin): patch os.environ in invalid-timeout fallback test

Use patch.dict to set OTEL_EXPORTER_ZIPKIN_TIMEOUT instead of assigning
os.environ directly, restoring the process environment after the test;
applied symmetrically to both exporters. No behavior change.

Assisted-by: Doubao

* test(zipkin): drop redundant environment restore assertion

patch.dict already restores the process environment, so asserting it
in the invalid-timeout fallback test adds nothing; remove it from both
exporters. No behavior change.

Assisted-by: Doubao

---------

Co-authored-by: Lukas Hering <40302054+herin049@users.noreply.github.com>
Co-authored-by: Leighton Chen <lechen@microsoft.com>
…y#5874)

* chore(deps): update dependency pylint to v4 [security]

* Fix lint compatibility with Pylint 4

Remove the obsolete suggestion-mode option from both Pylint configs and use NoneType when unwrapping optional annotations.

Assisted-by: GPT-6

---------

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: Lukas Hering <lukasheringb@gmail.com>
Co-authored-by: Lukas Hering <40302054+herin049@users.noreply.github.com>
@ocelotl
ocelotl force-pushed the docs_exporter_zipkin_json branch from d95b105 to 0c51e22 Compare November 4, 2026 16:29

This branch has not been deployed

No deployments
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.

2 participants