Skip to content

Split OpenTelemetryInstrument.bootstrap(); its exporter branch reads wrong #198

Description

@lesnik512

OpenTelemetryInstrument.bootstrap() is lite_bootstrap/instruments/opentelemetry_instrument.py:169-230 — about sixty lines doing five separable jobs: silencing two OTel loggers, assembling resource attributes, creating and registering the provider, adding up to three kinds of span processor, and applying the user's instrumentors.

The part that actually misleads

if self.bootstrap_config.opentelemetry_endpoint:
    if self.bootstrap_config.opentelemetry_exporter_protocol == "grpc":
        if import_checker.is_otlp_grpc_exporter_installed:
            tracer_provider.add_span_processor(BatchSpanProcessor(OTLPGrpcSpanExporter(...)))
        else:
            warnings.warn("…the gRPC OTLP exporter is not installed…")
    elif import_checker.is_otlp_http_exporter_installed:
        tracer_provider.add_span_processor(BatchSpanProcessor(OTLPHttpSpanExporter(...)))
    else:
        warnings.warn("…the HTTP OTLP exporter is not installed…")

The two transports do symmetric work — pick an exporter, or warn that its package is missing — but they are written at different nesting levels and in different shapes. The outer elif tests a dependency flag, not the protocol, so the trailing else reads as the else of the protocol check when it is really the HTTP-exporter-missing branch. A reader has to hold "which else belongs to which question" in their head to see that the HTTP path is even reachable.

Proposal

Extract _build_span_exporter() returning the exporter or None (emitting the missing-dependency warning itself), leaving one symmetric call site:

if exporter := self._build_span_exporter():
    tracer_provider.add_span_processor(BatchSpanProcessor(exporter))

Inside it, the two transports become two branches of the same shape. Then split the rest along its existing seams: _silence_otel_loggers(), _build_resource(), _apply_instrumentors(tracer_provider). bootstrap() becomes a readable sequence of named steps and each piece is separately testable.

No behaviour change intended: same processors, same warnings, same categories and messages. ADR-0006 fixes the policy (HTTP carries no insecure warning, otl-http is a sibling extra) and nothing here touches it — this is only the shape of the code expressing it.

Note

teardown() (line 232) collects errors with the same hand-rolled idiom that appears in two other modules; that duplication is its own issue, and whoever takes this one should not fold it in.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions