diff --git a/.changelog/5704.fixed b/.changelog/5704.fixed new file mode 100644 index 00000000000..aef00dd8750 --- /dev/null +++ b/.changelog/5704.fixed @@ -0,0 +1 @@ +`opentelemetry-exporter-zipkin-json`: warn and fall back to the default instead of raising `ValueError` when `OTEL_EXPORTER_ZIPKIN_TIMEOUT` holds a non-integer value; the same fix is applied to `opentelemetry-exporter-zipkin-proto-http`. diff --git a/.pylintrc b/.pylintrc index bda4148beb5..6ee96abff4d 100644 --- a/.pylintrc +++ b/.pylintrc @@ -36,10 +36,6 @@ persistent=yes # Specify a configuration file. #rcfile= -# When enabled, pylint would attempt to guess common misconfiguration and emit -# user-friendly hints instead of false-positive error messages. -suggestion-mode=yes - # Allow loading of arbitrary C extensions. Extensions are imported into the # active Python interpreter and may run arbitrary code. unsafe-load-any-extension=no diff --git a/codegen/opentelemetry-codegen-json/src/opentelemetry/codegen/__init__.py b/codegen/opentelemetry-codegen-json/src/opentelemetry/codegen/__init__.py index e69de29bb2d..c86a31b86c6 100644 --- a/codegen/opentelemetry-codegen-json/src/opentelemetry/codegen/__init__.py +++ b/codegen/opentelemetry-codegen-json/src/opentelemetry/codegen/__init__.py @@ -0,0 +1,14 @@ +# Copyright The OpenTelemetry Authors +# SPDX-License-Identifier: Apache-2.0 + +"""Build time code generators for OpenTelemetry Python. + +Nothing under ``opentelemetry.codegen`` runs while an application is emitting telemetry. These +modules are invoked by ``protoc`` during development and read protocol buffer descriptors, then +hand the Python source they build back to ``protoc``, which is the program that writes the files. +Those files are then committed to the repository and shipped as ordinary modules. + +``opentelemetry.codegen.json`` is the only generator that currently lives here. It produces the +generated Python modules of the ``opentelemetry-proto-json`` distribution, under its ``src`` +directory. The packaging files and the tests of that distribution are hand-written. +""" diff --git a/codegen/opentelemetry-codegen-json/src/opentelemetry/codegen/json/__init__.py b/codegen/opentelemetry-codegen-json/src/opentelemetry/codegen/json/__init__.py index e69de29bb2d..ccc8d348de0 100644 --- a/codegen/opentelemetry-codegen-json/src/opentelemetry/codegen/json/__init__.py +++ b/codegen/opentelemetry-codegen-json/src/opentelemetry/codegen/json/__init__.py @@ -0,0 +1,27 @@ +# Copyright The OpenTelemetry Authors +# SPDX-License-Identifier: Apache-2.0 + +"""Protocol buffer plugin that generates OTLP/JSON dataclasses. + +``protoc`` discovers this generator as the plugin named ``otlp_json``, through the +``protoc-gen-otlp_json`` console script declared in the distribution metadata of +``opentelemetry-codegen-json``. Passing ``--otlp_json_out=`` to ``protoc`` makes it run +``opentelemetry.codegen.json.plugin.main``, which hands the request to +``opentelemetry.codegen.json.generator``. + +For each ``.proto`` file named in ``file_to_generate`` on the request, and not for the descriptors +that are only dependencies, the generator emits one Python module holding a ``slots=True`` +dataclass per protobuf message and an ``enum.IntEnum`` per protobuf enum. Every generated message +class carries ``to_dict`` and ``from_dict`` methods that implement the Proto3 JSON mapping, with +the OTLP deviation that ``trace_id``, ``span_id`` and ``parent_span_id`` are hex encoded rather +than base64 encoded. Alongside those modules it copies +``opentelemetry.codegen.json.runtime.json_codec`` into the output tree as ``_json_codec``. The +generated code imports that copy, which belongs to the generated distribution, so no part of +``opentelemetry-codegen-json`` is imported at runtime. + +The modules here are hand-written. Everything emitted into the ``opentelemetry-proto-json`` +distribution is generated and must never be edited by hand, though only the per-message modules +say so in an ``AUTO-GENERATED`` header. The copied ``_json_codec.py``, the generated +``version/__init__.py`` and the empty ``__init__.py`` files carry no header and are regenerated +just the same. +""" diff --git a/codegen/opentelemetry-codegen-json/src/opentelemetry/codegen/json/generator.py b/codegen/opentelemetry-codegen-json/src/opentelemetry/codegen/json/generator.py index 4cf99a3e8c9..ac38e418917 100644 --- a/codegen/opentelemetry-codegen-json/src/opentelemetry/codegen/json/generator.py +++ b/codegen/opentelemetry-codegen-json/src/opentelemetry/codegen/json/generator.py @@ -3,6 +3,31 @@ # pylint: disable=no-member,invalid-name,too-many-lines +"""Turns protobuf descriptors into OTLP/JSON dataclass modules. + +This module is hand-written and runs at build time. Everything it produces is a string of Python +source destined for the ``opentelemetry-proto-json`` distribution, where it is committed and must +not be edited by hand. Only the per-message modules say so in an ``AUTO-GENERATED`` header; the +copied ``_json_codec.py``, the generated ``version/__init__.py`` and the empty ``__init__.py`` +files carry no header and are overwritten by a regeneration just the same. Changing the shape of +the generated code means changing the code here and regenerating, which +``scripts/proto_codegen_json.sh`` does. + +``OtlpJsonGenerator`` holds the whole translation. ``generate_plugin_response`` is what +``opentelemetry.codegen.json.plugin`` calls, and it turns the generator's output into the +``CodeGeneratorResponse`` that ``protoc`` reads back. ``generate_code`` is the inner form it is +built from, returning a plain mapping from output path to source text. + +The output layout mirrors the input layout. A ``package_transform`` callable rewrites each +``.proto`` path, by default replacing the substring ``opentelemetry/proto/`` with +``opentelemetry/proto_json/``, and the longest directory prefix shared by every transformed path +becomes the common root. Two things are written at that common root rather than per proto file: +``_json_codec.py``, copied verbatim from +``opentelemetry.codegen.json.runtime.json_codec``, and ``version/__init__.py``, holding +``__version__``. An empty ``__init__.py`` is then added to every ancestor directory of a +generated file, apart from the ``opentelemetry`` namespace directory itself. +""" + from __future__ import annotations import logging @@ -34,6 +59,24 @@ class OtlpJsonGenerator: """ Generates Python dataclasses and JSON serialization/deserialization code from protobuf descriptors. + + Construction indexes every message and enum in every descriptor of ``request``, including the + descriptors that are only dependencies and are not themselves being generated, because a field + in a generated file may name a type defined in one of those. Generation itself happens in + ``generate_all``. + + An instance produces source text only, and nothing here touches the filesystem except to read + the codec module. ``protoc`` is what writes the files, from the response that + ``opentelemetry.codegen.json.plugin`` hands back to it. + + Args: + request: The ``CodeGeneratorRequest`` that ``protoc`` wrote to the plugin's standard input. + Its ``proto_file`` field carries every descriptor, and its ``file_to_generate`` field + names the subset to emit modules for. + package_transform: Rewrites a ``.proto`` path into the path of the Python file to generate + from it. The ``.proto`` suffix is replaced with ``.py`` afterwards, so this callable + only has to deal with the directory part. + version: The value written as ``__version__`` into the generated ``version/__init__.py``. """ def __init__( @@ -43,12 +86,7 @@ def __init__( version: str, ) -> None: """ - Initialize the generator. - - Args: - request: Protobuf compiler plugin request - package_transform: A callable that transforms the proto file path. - version: Version string for the generated code. + Initialize the generator. The parameters are documented on the class. """ self._request = request self._package_transform = package_transform @@ -616,9 +654,25 @@ def _get_serialization_expr(self, field_desc: descriptor.FieldDescriptorProto, v """ Get the Python expression to serialize a value of a given type for JSON output. + The expression is a fragment of generated source, so ``var_name`` is spliced into it as + written rather than evaluated here. For a repeated field the caller passes the loop + variable of a lambda, and for a singular field it passes an attribute access on ``self``. + + A field named ``trace_id``, ``span_id`` or ``parent_span_id`` is hex encoded in preference + to the base64 encoding that Proto3 JSON gives bytes, which is the OTLP deviation. The type + is consulted first: a message-typed or enum-typed field is handled by its type whatever it + is called. Among the remaining types the name alone decides, so a field with one of those + three names is hex encoded even when it is declared as something other than ``bytes``. + Args: field_desc: Field descriptor for the value being serialized var_name: Variable name representing the value to serialize + + Returns: + A Python expression that evaluates to the JSON form of the value. For a type that + needs no conversion, such as a string or a 32-bit integer, this is ``var_name`` + unchanged. On the repeated branch the caller compares the result against ``var_name`` + to decide whether the list needs a mapping call at all. """ codec = self._get_codec_module_path() if field_desc.type == descriptor.FieldDescriptorProto.TYPE_MESSAGE: @@ -858,8 +912,15 @@ def _load_codec_source() -> str: """ Load the source code for the codec module from its source file. + The text is copied into the generated package unchanged, as ``_json_codec.py``, so that the + generated code carries its own runtime helpers and does not import this distribution. + Returns: Source code as a string + + Raises: + RuntimeError: If ``runtime/json_codec.py`` cannot be read. Generation cannot continue, + because every generated module imports the codec module by name. """ codec_src_path = Path(__file__).parent / "runtime" / "json_codec.py" try: @@ -908,12 +969,22 @@ def generate_code( """ Main entry point for code generation. + ``generate_plugin_response`` is the only caller. This form exists because a mapping from path + to source is easier to read and to assert against than a protobuf response message. + Args: request: Protobuf compiler plugin request - package_transform: Package transformation string or callable + package_transform: Rewrites a ``.proto`` path into the path of the Python file to generate + from it. The default substitutes ``opentelemetry/proto_json/`` for + ``opentelemetry/proto/`` wherever that substring occurs, and leaves a path that does + not contain it alone. Returns: - Dictionary mapping output file paths to generated code + Dictionary mapping output file paths to generated code. Besides one entry per file in + ``request.file_to_generate``, it holds ``_json_codec.py`` and ``version/__init__.py`` at + the common root, and an ``__init__.py`` for every ancestor directory of a generated file + apart from the ``opentelemetry`` namespace directory. It is empty when + ``file_to_generate`` is. """ generator = OtlpJsonGenerator(request, package_transform, version="0.0.0") return generator.generate_all() @@ -926,12 +997,20 @@ def generate_plugin_response( """ Generate plugin response with all generated files. + Besides the files, the response declares which ``protoc`` features this plugin understands. + Without ``FEATURE_PROTO3_OPTIONAL`` declared, ``protoc`` refuses to pass a descriptor + containing an ``optional`` proto3 field, and the edition range states which editions of the + language the plugin accepts. + Args: request: Protobuf compiler plugin request - package_transform: Package transformation string + package_transform: Rewrites a ``.proto`` path into the path of the Python file to generate + from it. The default substitutes ``opentelemetry/proto_json/`` for + ``opentelemetry/proto/`` wherever that substring occurs, and leaves a path that does + not contain it alone. Returns: - Plugin response with generated files + Plugin response with generated files, ready for ``protoc`` to write to disk. """ response = plugin.CodeGeneratorResponse() diff --git a/codegen/opentelemetry-codegen-json/src/opentelemetry/codegen/json/plugin.py b/codegen/opentelemetry-codegen-json/src/opentelemetry/codegen/json/plugin.py index 7cd2466c22a..3c4ebbf6606 100644 --- a/codegen/opentelemetry-codegen-json/src/opentelemetry/codegen/json/plugin.py +++ b/codegen/opentelemetry-codegen-json/src/opentelemetry/codegen/json/plugin.py @@ -3,6 +3,27 @@ # pylint: disable=no-member +"""The ``protoc`` plugin entry point for the OTLP/JSON generator. + +``protoc`` invokes a plugin as a separate process, with an empty argument vector. It writes a +serialized ``CodeGeneratorRequest`` to the standard input of that process and reads a serialized +``CodeGeneratorResponse`` back from its standard output, so a plugin must keep both streams free +of anything else. Diagnostics therefore go to standard error. + +Nothing configures logging on the path ``protoc`` takes. The ``logging.basicConfig`` call below +runs only when this module is executed as ``__main__``, and ``protoc`` reaches ``main`` through +the console script instead, which imports the module under its own name. A record of ``WARNING`` +or above therefore arrives at standard error through ``logging.lastResort``, carrying its message +alone and none of the formatting set up below, and a record under ``WARNING`` is dropped. + +``protoc`` finds this module through the ``protoc-gen-otlp_json`` console script, which the +``opentelemetry-codegen-json`` distribution declares. The name after ``protoc-gen-`` is the plugin +name, so ``--otlp_json_out=`` on a ``protoc`` command line is what runs ``main``. + +The actual generation lives in ``opentelemetry.codegen.json.generator``. This module only handles +the process protocol around it. +""" + import logging import sys from collections.abc import Iterator @@ -18,7 +39,25 @@ @contextmanager def code_generation() -> Iterator[tuple[plugin.CodeGeneratorRequest, plugin.CodeGeneratorResponse],]: """ - Context manager for handling the code generation process. + Read the ``protoc`` request from standard input and write the response to standard output. + + Unless the version branch below is taken, on entry this reads every byte of standard input, + parses it as a ``CodeGeneratorRequest``, and yields that request together with an empty + ``CodeGeneratorResponse`` for the body of the ``with`` statement to fill in. On exit it + serializes the response and writes it to standard output. + + Two cases write no response to standard output at all. A first command line argument of + ``-V`` or ``--version`` prints the version and calls ``sys.exit`` before anything is read from + standard input, so the body never runs and the call does not block on a stream that may never + close. That branch answers a version query typed directly at the console script; + ``protoc`` never reaches it, because ``protoc`` runs a plugin with an empty argument vector. + An exception raised inside the body propagates instead, and because the serialization is + placed after the ``yield`` rather than in a ``finally`` block, it is skipped, so ``protoc`` + reports the failure instead of accepting a partial result. + + Returns: + A context manager yielding the parsed ``CodeGeneratorRequest`` and the empty + ``CodeGeneratorResponse`` that the caller is expected to populate. """ if len(sys.argv) > 1 and sys.argv[1] in ("-V", "--version"): print("opentelemetry-codegen-json 0.0.0") @@ -39,7 +78,18 @@ def code_generation() -> Iterator[tuple[plugin.CodeGeneratorRequest, plugin.Code def main() -> None: """ - Main entry point for the protoc plugin. + Run one ``protoc`` plugin invocation. + + This is the callable behind the ``protoc-gen-otlp_json`` console script. It passes the request + read from standard input to + ``opentelemetry.codegen.json.generator.generate_plugin_response``, then merges the feature + flags of that result into the response that ``code_generation`` writes out and copies every + generated file across one at a time. + + Both steps are needed because ``code_generation`` owns the response object that gets + serialized. The flags are merged with ``|=`` rather than assigned, so a flag the response + already carries survives. The files are copied one at a time because a repeated protobuf + message field cannot be rebound to another message, only added to and merged into. """ with code_generation() as (request, response): generated_response = generate_plugin_response(request) diff --git a/codegen/opentelemetry-codegen-json/src/opentelemetry/codegen/json/runtime/__init__.py b/codegen/opentelemetry-codegen-json/src/opentelemetry/codegen/json/runtime/__init__.py index e69de29bb2d..f78962b66d3 100644 --- a/codegen/opentelemetry-codegen-json/src/opentelemetry/codegen/json/runtime/__init__.py +++ b/codegen/opentelemetry-codegen-json/src/opentelemetry/codegen/json/runtime/__init__.py @@ -0,0 +1,15 @@ +# Copyright The OpenTelemetry Authors +# SPDX-License-Identifier: Apache-2.0 + +"""Source modules that the generator copies verbatim into its output. + +The generator does not import a module under ``opentelemetry.codegen.json.runtime`` in order to +call it. It reads the text of the module off disk and writes that text into the generated package, +so that the generated code never requires ``opentelemetry-codegen-json`` to be installed. + +``opentelemetry.codegen.json.runtime.json_codec`` is the only such module. It lands in the output +tree as ``_json_codec``, next to the generated message modules. + +Changing a module here changes every package generated afterwards, so the copies already committed +under ``opentelemetry-proto-json`` stay as they are until the next regeneration. +""" diff --git a/codegen/opentelemetry-codegen-json/src/opentelemetry/codegen/json/runtime/json_codec.py b/codegen/opentelemetry-codegen-json/src/opentelemetry/codegen/json/runtime/json_codec.py index e528f4f0eb9..61e6b6bc954 100644 --- a/codegen/opentelemetry-codegen-json/src/opentelemetry/codegen/json/runtime/json_codec.py +++ b/codegen/opentelemetry-codegen-json/src/opentelemetry/codegen/json/runtime/json_codec.py @@ -1,6 +1,39 @@ # Copyright The OpenTelemetry Authors # SPDX-License-Identifier: Apache-2.0 +"""Encoding and decoding helpers that every generated message module calls. + +This module is hand-written, and it is also the one module of +``opentelemetry-codegen-json`` that ends up inside the generated package. +``opentelemetry.codegen.json.generator`` reads its text off disk and writes it out as +``_json_codec.py`` at the root of the generated tree, so the generated code calls these functions +without ``opentelemetry-codegen-json`` being installed. Nothing here is itself generated. + +Two consequences follow from being copied rather than imported. A change here reaches a generated +package only at its next regeneration, so the copy committed under ``opentelemetry-proto-json`` +can be older than this file. And every import written here becomes a runtime requirement of the +generated distribution, since it is the generated distribution that will run this code. + +The functions implement the Proto3 JSON mapping described at +https://protobuf.dev/programming-guides/json/, with one OTLP deviation: ``encode_hex`` and +``decode_hex`` carry ``trace_id``, ``span_id`` and ``parent_span_id`` as hex strings where Proto3 +JSON would use base64. + +Every ``decode_`` function takes the protobuf field name as its last argument and uses it only to +build the message of the exception it raises, so a failure names the field that caused it rather +than only the position in the payload. A ``decode_`` function raises ``TypeError`` when the JSON +value is of the wrong Python type and ``ValueError`` when the type is right but the content is +not, and it chains neither, so the traceback stops at the field rather than exposing the internals +of ``base64`` or ``int``. + +The type checking is not total, and each gap is written out on the function that has it. +``decode_hex`` and ``decode_base64`` return empty bytes for any falsy value before they check its +type. ``decode_repeated`` checks an element only as far as the ``item_parser`` it is given does, +which for a repeated field of an already valid JSON type is not at all. ``validate_type`` checks +with ``isinstance``, so a JSON ``true`` passes as an integer everywhere an integer is accepted +except in ``decode_enum``, which rejects it explicitly. +""" + from __future__ import annotations import abc @@ -21,12 +54,28 @@ class JsonMessage(abc.ABC): """ Abstract base class for protobuf messages with JSON serialization. + + Every generated message class is a dataclass that inherits from this one. The two abstract + methods are what the generator writes out, field by field, for each message; the two concrete + methods are the same thing with ``json.dumps`` and ``json.loads`` wrapped around them, and are + inherited unchanged. + + Instances are not validated on construction. A generated class accepts whatever the caller + assigns to its fields, and the type checking happens on the way in from JSON, inside + ``from_dict``. """ @abc.abstractmethod def to_dict(self) -> dict[str, typing.Any]: """ Convert this message to a dictionary. + + Returns: + A dictionary keyed by the lowerCamelCase JSON names of the fields, holding only the + fields that are set. A field left at its proto3 default is omitted. The members of a + oneof group are tested in reverse declaration order, and the first one that is not + ``None`` is the only one written, so a caller that has set several of them loses the + rest without an error. """ @classmethod @@ -34,11 +83,30 @@ def to_dict(self) -> dict[str, typing.Any]: def from_dict(cls: type[M], data: dict[str, typing.Any]) -> M: """ Create an instance from a dictionary. + + Args: + data: A dictionary keyed by the lowerCamelCase JSON names of the fields. A key that + no field of this message claims is ignored rather than rejected, and a field + whose key is absent or ``None`` keeps its default. + + Returns: + An instance of the message class this was called on. + + Raises: + TypeError: If ``data`` is not a dictionary, or if a value in it is of a Python type + that its field cannot accept. Not every wrong type is caught: see the gaps + described on this module. + ValueError: If a value is of an acceptable type but cannot be decoded, such as a + malformed hex string or an integer that no member of the field's enum carries. """ def to_json(self) -> str: """ Serialize this message to a JSON string. + + Returns: + The output of ``to_dict`` run through ``json.dumps`` with its default settings, so + the keys appear in insertion order and the result carries no indentation. """ return json.dumps(self.to_dict()) @@ -46,6 +114,19 @@ def to_json(self) -> str: def from_json(cls, data: str | bytes) -> Self: """ Deserialize from a JSON string or bytes. + + Args: + data: A JSON document holding an object. Bytes are decoded by ``json.loads``, which + accepts UTF-8, UTF-16 and UTF-32. + + Returns: + An instance of the message class this was called on. + + Raises: + json.JSONDecodeError: If ``data`` is not well-formed JSON. + TypeError: If the document is well-formed but is not an object, or if a value in it + is of a Python type that its field cannot accept. + ValueError: If a value is of an acceptable type but cannot be decoded. """ return cls.from_dict(json.loads(data)) @@ -54,10 +135,14 @@ def encode_hex(value: bytes | None) -> str: """ Encode bytes as hex string. + This is the OTLP form for ``trace_id``, ``span_id`` and ``parent_span_id``, where Proto3 JSON + would use base64. The digits are lowercase, which is what ``bytes.hex`` produces. + Args: value: The bytes to encode. Returns: - Hex string representation of the input bytes. + Hex string representation of the input bytes. ``None`` and empty bytes both give the + empty string, and a generated ``to_dict`` drops the field rather than writing that. """ return value.hex() if value else "" @@ -70,7 +155,8 @@ def encode_base64(value: bytes | None) -> str: Args: value: The bytes to encode. Returns: - Base64 string representation of the input bytes. + Base64 string representation of the input bytes, with the standard alphabet and padding. + ``None`` and empty bytes both give the empty string. """ return base64.b64encode(value).decode("utf-8") if value else "" @@ -80,10 +166,14 @@ def encode_int64(value: int) -> str: Encode 64 bit integers as strings. Required for int64, uint64, fixed64, sfixed64 and sint64 per Proto3 JSON spec. + A JSON number is a double in many parsers, which cannot hold every 64-bit integer exactly, so + the string form is what keeps the value intact across a round trip. + Args: value: The integer to encode. Returns: - String representation of the input integer. + String representation of the input integer, in base ten, with a leading minus sign when + negative. """ return str(value) @@ -92,10 +182,14 @@ def encode_float(value: float) -> float | str: """ Encode float/double values. + JSON has no literal for a non-finite number, so Proto3 JSON spells the three of them as + strings. ``decode_float`` reads those same three spellings back. + Args: value: The float to encode. Returns: - The input value, or a string for special float values (NaN, Infinity). + The input value, or one of the strings ``"NaN"``, ``"Infinity"`` and ``"-Infinity"`` when + it is not finite. A NaN carries no sign here, so a negative NaN comes out as ``"NaN"``. """ if math.isnan(value): return "NaN" @@ -111,9 +205,14 @@ def encode_repeated( """ Helper to serialize repeated fields with a mapping function. + The generated code calls this only when the element type needs conversion. A repeated field + of a type that is already valid JSON, such as ``string``, is assigned straight across with no + call here. + Args: values: The list of values to encode. - map_fn: A function that takes a single value and returns its encoded form. + map_fn: A function that takes a single value and returns its encoded form. It is applied + to every element, including ``None``, so it has to tolerate whatever the field holds. Returns: A list of encoded values, or an empty list if input is None or empty. """ @@ -124,11 +223,24 @@ def decode_hex(value: str | None, field_name: str) -> bytes: """ Decode hex string to bytes. + This is the OTLP form for ``trace_id``, ``span_id`` and ``parent_span_id``. Every other bytes + field goes through ``decode_base64`` instead. + + The length is not checked against what the field means, so a trace id of the wrong number of + bytes decodes without complaint and is caught, if at all, further along. + Args: value: The hex string to decode. field_name: The name of the field being decoded (for error messages). Returns: The decoded bytes, or empty bytes if input is None or empty. + + Raises: + TypeError: If ``value`` is truthy and not a ``str``. The emptiness check runs first, so a + falsy value of any type, ``0`` and ``[]`` included, returns ``b""`` rather than + raising. + ValueError: If ``value`` is a string of odd length or holds a character that is not a hex + digit. """ if not value: return b"" @@ -143,11 +255,21 @@ def decode_base64(value: str | None, field_name: str) -> bytes: """ Decode base64 string to bytes. + ``base64.b64decode`` is called without ``validate=True``, so a character outside the base64 + alphabet is discarded rather than rejected, and only a length or padding problem that survives + that produces an error. + Args: value: The base64 string to decode. field_name: The name of the field being decoded (for error messages). Returns: The decoded bytes, or empty bytes if input is None or empty. + + Raises: + TypeError: If ``value`` is truthy and not a ``str``. As in ``decode_hex``, the emptiness + check runs first, so a falsy value of any type returns ``b""`` rather than raising. + ValueError: If ``base64.b64decode`` rejects the string. Any exception it raises is + reported as a ``ValueError`` naming the field. """ if not value: return b"" @@ -162,11 +284,22 @@ def decode_int64(value: int | str | None, field_name: str) -> int: """ Parse int64 from number or string. + Both spellings are accepted because Proto3 JSON writes a 64-bit integer as a string but + requires parsers to take a number as well. + + No range check is applied, so a value outside the range of the protobuf type is kept as the + arbitrary precision Python integer it parsed to. + Args: value: The value to decode, which can be an int, a string, or None field_name: The name of the field being decoded (for error messages). Returns: The decoded integer, or 0 if input is None. + + Raises: + TypeError: If ``value`` is none of ``None``, ``int`` or ``str``. A ``bool`` passes this + check, for the reason given on ``validate_type``. + ValueError: If ``value`` is a string that does not spell an integer. """ if value is None: return 0 @@ -181,11 +314,20 @@ def decode_float(value: float | str | None, field_name: str) -> float: """ Parse float/double from number or string, handling special values. + Proto3 JSON has no literals for the three non-finite values, so it spells them as the strings + ``"NaN"``, ``"Infinity"`` and ``"-Infinity"``, which are matched here before any numeric + conversion is attempted. Those three spellings are exact and case sensitive; ``"nan"`` and + ``"inf"`` are left to ``float``, which happens to accept them too. + Args: value: The value to decode, which can be a float, int, string, or None field_name: The name of the field being decoded (for error messages). Returns: The decoded float, or 0.0 if input is None. + + Raises: + TypeError: If ``value`` is none of ``None``, ``float``, ``int`` or ``str``. + ValueError: If ``value`` is a string that ``float`` cannot parse. """ if value is None: return 0.0 @@ -210,12 +352,28 @@ def decode_repeated( """ Parse a list of values using the provided item parser function. + The list itself is always type checked here. Whether the elements are checked depends on what + ``item_parser`` does, and for a repeated field whose element type needs no conversion, such as + ``string`` or ``bool``, the generator passes the identity lambda ``lambda _v: _v``. Those + elements are therefore not checked at all: a repeated ``string`` field given ``[1, {"a": 2}]`` + decodes to that same list, despite the generated type hint saying + ``builtins.list[builtins.str]``. + + An element that ``item_parser`` does reject stops the whole call, so a partially decoded list + is never returned. + Args: values: The list of values to decode, or None. item_parser: A function that takes a single value and returns the parsed form. - field_name: The name of the field being decoded (for error messages). + field_name: The name of the field being decoded (for error messages). The index of the + offending element is not included, so an error names the field only. Returns: A list of parsed values, or an empty list if input is None. + + Raises: + TypeError: If ``values`` is neither ``None`` nor a ``list``, or if ``item_parser`` rejects + the type of an element. + ValueError: If ``item_parser`` rejects the content of an element. """ if values is None: return [] @@ -232,10 +390,19 @@ def validate_type( Validate that a value is of the expected type(s). Raises TypeError if validation fails. + The check is ``isinstance``, so a subclass passes as its base. That matters for one case in + practice: ``bool`` is a subclass of ``int`` in Python, so a JSON ``true`` passes a check + against ``int`` and is then decoded as the integer 1. ``decode_enum`` rejects ``bool`` + explicitly before calling this; the other integer paths do not. + Args: value: The value to validate. expected_types: A type or tuple of types that the value is expected to be. field_name: The name of the field being validated (for error messages). + + Raises: + TypeError: If ``value`` is an instance of none of ``expected_types``. The message names + the field, the expected types and the actual type. """ if not isinstance(value, expected_types): raise TypeError(f"Field '{field_name}' expected {expected_types}, got {type(value).__name__}") @@ -248,12 +415,24 @@ def decode_enum(value: int | str, enum_type: type[EnumT], field_name: str) -> En Per the ProtoJSON spec, parsers must accept both enum names (str) and integer values (int). + A name is matched exactly and case sensitively against the members of ``enum_type``, which are + generated with the names the ``.proto`` file gives them. An integer has to equal the value of + a member: an unknown number is rejected rather than kept, which is stricter than protobuf's + own rule that an unrecognised enum value in proto3 is preserved. + Args: value: The enum name or integer value to decode. enum_type: The enum class to decode into. field_name: The name of the field being decoded (for error messages). Returns: The corresponding enum member. + + Raises: + TypeError: If ``value`` is a ``bool``, which is rejected before the type check so that a + JSON ``true`` cannot silently become the member whose value is 1, or if it is neither + an ``int`` nor a ``str``. + ValueError: If ``value`` names no member of ``enum_type``, or is a number that no member + carries. """ if isinstance(value, bool): raise TypeError(f"Field '{field_name}' expected int or str, got bool") diff --git a/codegen/opentelemetry-codegen-json/src/opentelemetry/codegen/json/types.py b/codegen/opentelemetry-codegen-json/src/opentelemetry/codegen/json/types.py index bc2da8d99ef..9099288b845 100644 --- a/codegen/opentelemetry-codegen-json/src/opentelemetry/codegen/json/types.py +++ b/codegen/opentelemetry-codegen-json/src/opentelemetry/codegen/json/types.py @@ -3,6 +3,27 @@ # pylint: disable=no-member +"""Maps protobuf field types onto the Python the generator writes. + +Every value this module returns is a fragment of Python source for +``opentelemetry.codegen.json.generator`` to splice into a generated file, not a Python object. The +string ``"builtins.int"`` is a type hint to be written out, and the string ``'""'`` is the source +text of an empty string literal. + +Names are fully qualified against ``builtins`` throughout. Qualifying keeps a type hint from +resolving to a generated class that happens to carry a builtin's name, which a bare ``str`` or +``int`` in a generated module would. + +The mappings follow the Proto3 JSON specification at +https://protobuf.dev/programming-guides/json/, with one OTLP deviation that +``is_hex_encoded_field`` carries: ``trace_id``, ``span_id`` and ``parent_span_id`` are hex strings +rather than the base64 strings Proto3 JSON gives bytes fields. + +Three protobuf types are absent from every table here: ``TYPE_MESSAGE``, ``TYPE_ENUM`` and +``TYPE_GROUP``. The first two are resolved against the descriptor index rather than a fixed table, +which the generator does itself, and ``TYPE_GROUP`` is the proto2 construct that OTLP does not use. +""" + from typing import Final from google.protobuf import descriptor_pb2 as descriptor @@ -73,10 +94,18 @@ def get_python_type(proto_type: int) -> str: """ Get the Python type corresponding to a protobuf field type. + This covers the scalar types only. A message, an enum or a proto2 group has to be resolved + against the descriptor index instead, which the generator does before it gets here. + Args: proto_type: The protobuf field type as an integer (from FieldDescriptorProto). Returns: - A string representing the Python type + A string representing the Python type, qualified against ``builtins``. + + Raises: + ValueError: If the type is not a scalar type. Failing here rather than guessing is + deliberate: a wrong type hint would be written into a generated file and only + surface later, far from its cause. """ if proto_type not in PROTO_TO_PYTHON: raise ValueError(f"Unknown protobuf type: {proto_type}") @@ -87,10 +116,15 @@ def get_default_value(proto_type: int) -> str: """ Get the default value for a protobuf field type as a string. + These are the proto3 zero values, written as Python source: ``0`` for every integer type, + ``0.0`` for the floating point types, ``False``, ``""`` and ``b""``. + Args: proto_type: The protobuf field type as an integer (from FieldDescriptorProto). Returns: - A string representing the default value for that type + A string representing the default value for that type. A type with no entry, which means + a message, an enum or a proto2 group, gives the string ``"None"``. The generator does not + rely on that fallback: it decides the default for those cases before calling this. """ return PROTO_DEFAULTS.get(proto_type, "None") @@ -99,6 +133,9 @@ def is_int64_type(proto_type: int) -> bool: """ Check if type is a 64-bit integer type (int64, uint64, fixed64, sfixed64, sint64) + These are the types Proto3 JSON encodes as strings, because a 64-bit integer does not survive + a round trip through the IEEE 754 double that a JSON number becomes in many parsers. + Args: proto_type: The protobuf field type as an integer (from FieldDescriptorProto). Returns: @@ -123,6 +160,11 @@ def is_hex_encoded_field(field_name: str) -> bool: """ Check if field is a hex-encoded field (trace_id, span_id, parent_span_id). + OTLP/JSON departs from Proto3 JSON for these three, carrying them as lowercase hex strings + instead of base64. This function looks at the name alone and never at the type. The generator + does check the type first, so a message-typed or enum-typed field never reaches this test, + but among the remaining types the name is the whole decision. + Args: field_name: The name of the protobuf field. Returns: @@ -135,6 +177,16 @@ def to_json_field_name(snake_name: str) -> str: """ Convert snake_case field name to camelCase for JSON representation. + This is the fallback for a descriptor that carries no ``json_name``. ``protoc`` normally fills + that field in, and the generator prefers it when present. + + The conversion uses ``str.title`` on each component after the first. ``str.title`` uppercases + the first letter of the string and the first letter after every character that is not a + letter, and lowercases every other letter, so ``trace_ID`` gives ``traceId`` and a digit + inside a component starts a second uppercase letter: ``foo_bar1baz`` gives ``fooBar1Baz``. + A leading underscore produces an empty first component, so the result begins with whatever + ``str.title`` makes of the second one: ``_foo`` gives ``Foo``, and ``_1abc`` gives ``1Abc``. + Args: snake_name: The field name in snake_case. Returns: @@ -146,7 +198,11 @@ def to_json_field_name(snake_name: str) -> str: def is_numeric_type(proto_type: int) -> bool: """ - Check if the protobuf field type is a numeric type (int32, int64, uint32, uint64, float, double). + Check if the protobuf field type is a numeric type. + + This is every integer type and both floating point types, twelve in all: the five members of + ``INT64_TYPES``, the 32-bit integers ``int32``, ``uint32``, ``fixed32``, ``sfixed32`` and + ``sint32``, and ``float`` and ``double``. ``bool`` is not one of them. Args: proto_type: The protobuf field type as an integer (from FieldDescriptorProto). @@ -160,12 +216,25 @@ def get_json_allowed_types(proto_type: int, field_name: str = "") -> str: """ Get the Python type(s) allowed for the JSON representation of a field. + The result is written into a generated ``from_dict`` as the second argument of a + ``validate_type`` call, so it names what a decoder will accept off the wire rather than what + the dataclass attribute ends up holding. Those differ wherever Proto3 JSON permits more than + one spelling: a 64-bit integer arrives as either a number or a string, a float as a number or + as one of the strings ``"NaN"``, ``"Infinity"`` and ``"-Infinity"``, and a bytes field as a + base64 string. + Args: proto_type: The protobuf field type as an integer (from FieldDescriptorProto). field_name: The name of the protobuf field (used to determine if it's hex-encoded) Returns: - A string representing the allowed Python type(s) for the JSON representation of the field. + A string representing the allowed Python type(s) for the JSON representation of the + field, either a single qualified name or a parenthesized tuple of them, ready to be + written into generated source. + + Raises: + ValueError: If the type is neither hex encoded by name nor one of the scalar types, since + the remaining cases fall through to ``get_python_type``. """ if is_hex_encoded_field(field_name): return "builtins.str" diff --git a/codegen/opentelemetry-codegen-json/src/opentelemetry/codegen/json/writer.py b/codegen/opentelemetry-codegen-json/src/opentelemetry/codegen/json/writer.py index 65a3cb5ebc4..36b4e3415f2 100644 --- a/codegen/opentelemetry-codegen-json/src/opentelemetry/codegen/json/writer.py +++ b/codegen/opentelemetry-codegen-json/src/opentelemetry/codegen/json/writer.py @@ -1,6 +1,23 @@ # Copyright The OpenTelemetry Authors # SPDX-License-Identifier: Apache-2.0 +"""Accumulates lines of Python source and tracks indentation. + +``CodeWriter`` is the only thing in this module. +``opentelemetry.codegen.json.generator`` uses it to assemble every file it emits. + +The writer has no model of the code it is building. It appends strings to a list and prefixes each +non-empty one with the current indentation, and the methods named after Python constructs only +save the caller from writing the keyword, the punctuation and the indentation bookkeeping by hand. +Nothing here parses or validates what it is given, so a syntactically invalid fragment passed in +comes out unchanged. + +A method named after a block construct, such as ``if_`` or ``class_``, is a context manager that +writes the header line and indents the body for the duration of the ``with`` statement. A method +named after a statement writes one line and returns the writer, so calls can be chained. Names +that would collide with a Python keyword or a builtin carry a trailing underscore. +""" + from __future__ import annotations from collections.abc import Generator, Iterable @@ -10,12 +27,25 @@ # pylint: disable-next=too-many-public-methods class CodeWriter: + """ + Builds a Python source file line by line, keeping track of the current indentation level. + + A new instance holds no lines and sits at indentation level zero. ``writeln`` appends one + line, prefixed with the current level's worth of spaces unless the line is empty, and the + methods built on it append one or more such lines. The block context managers raise the level + for the duration of a ``with`` statement and lower it again on exit, including when the body + raises. ``to_string`` and ``to_lines`` read the result + out; neither clears the instance, and no method removes or rewrites a line once it has been + appended. + + Args: + indent_size: Number of spaces that one indentation level is worth. Four matches the + formatting the repository applies to the generated files. + """ + def __init__(self, indent_size: int = 4) -> None: """ - Initializes a new CodeWriter instance. - - Args: - indent_size: Number of spaces for each indentation level (default: 4) + Initializes a new CodeWriter instance. The parameters are documented on the class. """ self._lines: list[str] = [] self._indent_level: int = 0 @@ -25,6 +55,14 @@ def __init__(self, indent_size: int = 4) -> None: def indent(self) -> Generator[CodeWriter, None, None]: """ Context manager to increase indentation level for a block of code. + + Use this for a block whose header has already been written, or for one that no other + method covers. The level is lowered again in a ``finally`` clause, so an exception raised + inside the ``with`` statement leaves the writer at the level it started from. + + Returns: + A context manager yielding this ``CodeWriter``, so the body can be written through the + name bound by ``as``. """ self._indent_level += 1 try: @@ -36,6 +74,11 @@ def writeln(self, line: str = "") -> CodeWriter: """ Writes a line of code with proper indentation. If the line is empty, it writes a blank line. + A blank line is written as the empty string rather than as the current indentation's worth + of spaces, which keeps trailing whitespace out of the generated file. Every other method + that writes a single line goes through this one, except ``blank_line``, which appends + directly. + Args: line: The line of code to write (default: empty string for a blank line) Returns: @@ -65,6 +108,10 @@ def comment(self, content: str | Iterable[str]) -> CodeWriter: """ Writes a comment line or block. If content is a string, it writes a single comment line. + An empty string, whether passed on its own or found in the iterable, becomes a bare ``#`` + rather than ``#`` followed by a space, so the generated file carries no trailing + whitespace. + Args: content: A string for a single comment line or an iterable of strings for multiple comment lines Returns: @@ -81,6 +128,10 @@ def docstring(self, content: str | Iterable[str]) -> CodeWriter: """ Writes a docstring. If content is a string, it writes a single-line docstring. If content is an iterable of strings, it writes a multi-line docstring. + Nothing in the content is escaped, so a triple quote inside it closes the docstring early + and breaks the generated file. The content the generator passes is fixed literal text and + protobuf message and enum names, neither of which can contain one. + Args: content: A string for a single-line docstring or an iterable of strings for a multi line docstring Returns: @@ -100,6 +151,10 @@ def import_(self, module: str, *items: str) -> CodeWriter: Writes an import statement. If items are provided, it writes a from-import statement. Otherwise, it writes a regular import statement. + The import is written wherever the writer currently is, with no sorting and no grouping, + so the caller decides the order. The generator sorts its own import lists before calling + this. + Args: module: The module to import *items: Optional items to import from the module (if empty, imports the whole module) @@ -117,8 +172,15 @@ def block(self, header: str) -> Generator[CodeWriter, None, None]: """ Create a generic code block with a header (e.g. if, for, while, try, etc.) + The header is written exactly as given, trailing colon included, so this is the escape + hatch for a construct that has no method of its own, such as ``try`` or ``with``. + Args: header: The header line for the code block + + Returns: + A context manager yielding this ``CodeWriter``, with the indentation raised for the + body of the block. """ self.writeln(header) with self.indent(): @@ -134,10 +196,21 @@ def class_( """ Generate a class definition with optional base classes and decorators. + The body is not given a ``pass`` statement when nothing is written inside the ``with`` + statement, so an empty body produces a syntax error in the generated file. The caller is + responsible for writing at least one statement. + Args: name: The name of the class - bases: Optional iterable of base class names - decorators: Optional iterable of decorator names + bases: Optional iterable of base class names. ``None`` and an empty list or tuple + both produce a class statement with no parenthesized base list at all, since the + test is on the truthiness of ``bases`` rather than on what it yields. + decorators: Optional iterable of decorator names, written in the order given, each + already without its leading ``@`` + + Returns: + A context manager yielding this ``CodeWriter``, with the indentation raised for the + class body. """ if decorators is not None: for dec in decorators: @@ -162,13 +235,23 @@ def dataclass( """ Generate a dataclass definition with optional base classes, decorators and dataclass parameters. + The dataclass decorator is written last, immediately above the class statement, so any + decorator passed in ``decorators`` wraps it. With neither ``frozen`` nor ``slots`` set the + decorator is written bare, without parentheses. + Args: name: The name of the dataclass bases: Optional iterable of base class names - decorators: Optional iterable of additional decorator names + decorators: Optional iterable of additional decorator names, written above the + dataclass decorator frozen: Whether to set frozen=True in the dataclass decorator slots: Whether to set slots=True in the dataclass decorator - decorator_name: The full name of the dataclass decorator to use + decorator_name: The full name of the dataclass decorator to use. The default assumes + the generated module imports ``dataclasses`` rather than ``dataclass`` itself. + + Returns: + A context manager yielding this ``CodeWriter``, with the indentation raised for the + class body. """ dc_params = [] if frozen: @@ -203,11 +286,20 @@ def enum( """ Generate an enum definition with optional base classes and decorators. + ``enum_type`` is written as the first base and anything in ``bases`` follows it. Python + requires the enum class to come last, so a non-empty ``bases`` produces a class statement + that raises ``TypeError`` when the generated module is imported. The generated OTLP enums + pass ``enum.IntEnum`` and nothing else, so the case does not arise there. + Args: name: The name of the enum enum_type: The base enum type to inherit from (default: "enum.Enum") bases: Optional iterable of additional base class names decorators: Optional iterable of decorator names + + Returns: + A context manager yielding this ``CodeWriter``, with the indentation raised for the + enum body. """ if decorators is not None: for dec in decorators: @@ -233,11 +325,20 @@ def field( """ Write a dataclass field with optional default value or default factory. + ``default`` is interpolated into the line as it is, so a string passed there is written + without quotes and has to arrive already in Python source form: ``'""'`` for an empty + string default, not ``""``. A ``default`` of ``None`` means no default at all rather than + a default of ``None``, so to write ``= None`` pass the string ``"None"``. + Args: name: The name of the field type_hint: The type hint for the field - default: Optional default value for the field - default_factory: Optional default factory for the field + default: Optional default value for the field, already written as Python source + default_factory: Optional default factory for the field, which takes precedence over + ``default`` when both are given + + Returns: + The CodeWriter instance """ if default_factory: self.writeln(f"{name}: {type_hint} = dataclasses.field(default_factory={default_factory})") @@ -251,9 +352,15 @@ def enum_member(self, name: str, value: Any) -> CodeWriter: """ Write an enum member with a specific value. + ``value`` is interpolated with ``str``, so an integer comes out as a numeric literal and a + string comes out unquoted. Pass a string member value already quoted. + Args: name: The name of the enum member - value: The value of the enum member + value: The value of the enum member, already written as Python source + + Returns: + The CodeWriter instance """ self.writeln(f"{name} = {value}") return self @@ -269,11 +376,22 @@ def function( """ Create a function definition with optional decorators and return type. + As with ``class_``, an empty body produces a syntax error, so write at least a ``pass`` + statement inside the ``with`` statement. + Args: name: The name of the function - params: An iterable of parameter strings or a single string for the parameter list + params: An iterable of parameter strings or a single string for the parameter list. + An iterable is joined with ``", "``; a string is written as it stands, which is + the way to pass a parameter list that is already assembled, with the caller + responsible for the separators. decorators: Optional iterable of decorator names - return_type: Optional return type hint for the function + return_type: Optional return type hint for the function. ``None`` and the empty string + both leave the arrow off entirely. + + Returns: + A context manager yielding this ``CodeWriter``, with the indentation raised for the + function body. """ if decorators is not None: for dec in decorators: @@ -297,11 +415,20 @@ def method( """ Create a method definition within a class with optional decorators and return type. + This delegates to ``function`` and adds nothing. It exists so that generator code reads as + what it is building, and it does not supply ``self``: the caller lists it in ``params`` + like any other parameter, or omits it for a ``staticmethod``. + Args: name: The name of the method - params: An iterable of parameter strings or a single string for the parameter list + params: An iterable of parameter strings or a single string for the parameter list, + including the receiver when there is one decorators: Optional iterable of decorator names return_type: Optional return type hint for the method + + Returns: + A context manager yielding this ``CodeWriter``, with the indentation raised for the + method body. """ with self.function(name, params, decorators=decorators, return_type=return_type): yield self @@ -312,7 +439,12 @@ def if_(self, condition: str) -> Generator[CodeWriter, None, None]: Create an if block Args: - condition: The condition for the if statement + condition: The condition for the if statement, written as Python source without the + ``if`` keyword and without the trailing colon + + Returns: + A context manager yielding this ``CodeWriter``, with the indentation raised for the + body of the branch. """ self.writeln(f"if {condition}:") with self.indent(): @@ -323,8 +455,17 @@ def elif_(self, condition: str) -> Generator[CodeWriter, None, None]: """ Create an elif block + Nothing checks that an ``if`` block precedes this one at the same indentation level. The + caller has to get that ordering right, which is why the generator decides between ``if_`` + and ``elif_`` from the position of a field within its oneof group. + Args: - condition: The condition for the elif statement + condition: The condition for the elif statement, written as Python source without the + ``elif`` keyword and without the trailing colon + + Returns: + A context manager yielding this ``CodeWriter``, with the indentation raised for the + body of the branch. """ self.writeln(f"elif {condition}:") with self.indent(): @@ -334,6 +475,12 @@ def elif_(self, condition: str) -> Generator[CodeWriter, None, None]: def else_(self) -> Generator[CodeWriter, None, None]: """ Create an else block + + As with ``elif_``, the preceding ``if`` block is the caller's responsibility. + + Returns: + A context manager yielding this ``CodeWriter``, with the indentation raised for the + body of the branch. """ self.writeln("else:") with self.indent(): @@ -345,8 +492,12 @@ def for_(self, var: str, iterable: str) -> Generator[CodeWriter, None, None]: Create a for loop Args: - var: The loop variable - iterable: The iterable to loop over + var: The loop variable, or several separated by commas for tuple unpacking + iterable: The iterable to loop over, written as a Python expression + + Returns: + A context manager yielding this ``CodeWriter``, with the indentation raised for the + loop body. """ self.writeln(f"for {var} in {iterable}:") with self.indent(): @@ -358,7 +509,12 @@ def while_(self, condition: str) -> Generator[CodeWriter, None, None]: Create a while loop Args: - condition: The condition for the while loop + condition: The condition for the while loop, written as Python source without the + ``while`` keyword and without the trailing colon + + Returns: + A context manager yielding this ``CodeWriter``, with the indentation raised for the + loop body. """ self.writeln(f"while {condition}:") with self.indent(): @@ -386,7 +542,8 @@ def return_(self, value: str | None = None) -> CodeWriter: Write a return statement with an optional return value Args: - value: The value to return (if None, writes a bare return statement) + value: The value to return, written as a Python expression. ``None`` writes a bare + return statement, and so does the empty string, since the check is on truthiness. Returns: The CodeWriter instance """ @@ -410,8 +567,11 @@ def blank_line(self, count: int = 1) -> CodeWriter: """ Write one or more blank lines + These bypass ``writeln`` and are appended as empty strings, so the current indentation + level has no effect on them. + Args: - count: The number of blank lines to write + count: The number of blank lines to write. Zero and any negative number write none. Returns: The CodeWriter instance """ @@ -422,6 +582,13 @@ def to_string(self) -> str: """ Get the generated code as a single string with newline characters separating lines. + The lines are joined rather than terminated, so the result has no trailing newline unless + the last line written was blank. ``_generate_file`` in + ``opentelemetry.codegen.json.generator`` relies on that for the per-proto message modules, + ending each one with ``blank_line``. The other files the generator produces end another + way: ``version/__init__.py`` through ``writemany``, and ``_json_codec.py`` and the empty + ``__init__.py`` files without a ``CodeWriter`` at all. + Returns: A string containing the entire generated code. """ @@ -432,6 +599,8 @@ def to_lines(self) -> list[str]: Get the generated code as a list of lines Returns: - A list of strings, where each string is a line of generated code. + A list of strings, where each string is a line of generated code. This is the + writer's own list and not a copy, so mutating it changes what the writer will produce + and what a later call returns. """ return self._lines diff --git a/dev-requirements.txt b/dev-requirements.txt index caa46ac0c4a..c96dbd33898 100644 --- a/dev-requirements.txt +++ b/dev-requirements.txt @@ -1,4 +1,4 @@ -pylint==3.3.4 +pylint==4.0.9 griffe==1.15.0 httpretty==1.1.4 inline-snapshot==0.34.2 diff --git a/exporter/opentelemetry-exporter-zipkin-json/src/opentelemetry/exporter/zipkin/json/__init__.py b/exporter/opentelemetry-exporter-zipkin-json/src/opentelemetry/exporter/zipkin/json/__init__.py index e976a07b387..426230615bc 100644 --- a/exporter/opentelemetry-exporter-zipkin-json/src/opentelemetry/exporter/zipkin/json/__init__.py +++ b/exporter/opentelemetry-exporter-zipkin-json/src/opentelemetry/exporter/zipkin/json/__init__.py @@ -81,10 +81,27 @@ DEFAULT_ENDPOINT = "http://localhost:9411/api/v2/spans" REQUESTS_SUCCESS_STATUS_CODES = (200, 202) +_DEFAULT_TIMEOUT = 10 logger = logging.getLogger(__name__) +def _timeout_from_env() -> int: + value = environ.get(OTEL_EXPORTER_ZIPKIN_TIMEOUT) + if value is None or not value.strip(): + return _DEFAULT_TIMEOUT + try: + return int(value) + except ValueError: + logger.warning( + "Invalid value %r for %s, using default of %s seconds", + value, + OTEL_EXPORTER_ZIPKIN_TIMEOUT, + _DEFAULT_TIMEOUT, + ) + return _DEFAULT_TIMEOUT + + class ZipkinExporter(SpanExporter): def __init__( self, @@ -128,7 +145,7 @@ def __init__( self.session = session or requests.Session() self.session.headers.update({"Content-Type": self.encoder.content_type()}) self._closed = False - self.timeout = timeout or int(environ.get(OTEL_EXPORTER_ZIPKIN_TIMEOUT, 10)) + self.timeout = timeout or _timeout_from_env() def export(self, spans: Sequence[Span]) -> SpanExportResult: # After the call to Shutdown subsequent calls to Export are diff --git a/exporter/opentelemetry-exporter-zipkin-json/tests/test_zipkin_exporter.py b/exporter/opentelemetry-exporter-zipkin-json/tests/test_zipkin_exporter.py index ced0bd27d81..0e49d1d81f5 100644 --- a/exporter/opentelemetry-exporter-zipkin-json/tests/test_zipkin_exporter.py +++ b/exporter/opentelemetry-exporter-zipkin-json/tests/test_zipkin_exporter.py @@ -63,6 +63,52 @@ def test_constructor_env_vars(self): self.assertEqual(exporter.local_node.port, None) self.assertEqual(exporter.timeout, 15) + def test_constructor_invalid_timeout_env_var_falls_back(self): + with patch.dict(os.environ, {OTEL_EXPORTER_ZIPKIN_TIMEOUT: "10s"}): + with self.assertLogs("opentelemetry.exporter.zipkin.json", level="WARNING") as cm: + exporter = ZipkinExporter() + + self.assertEqual(exporter.timeout, 10) + self.assertEqual(len(cm.records), 1) + self.assertTrue( + any("Invalid value" in message for message in cm.output), + cm.output, + ) + + def test_constructor_empty_timeout_env_var_falls_back(self): + for value in ("", " ", "\t\n"): + with self.subTest(value=value), patch.dict(os.environ, {OTEL_EXPORTER_ZIPKIN_TIMEOUT: value}): + with self.assertNoLogs("opentelemetry.exporter.zipkin.json", level="WARNING"): + exporter = ZipkinExporter() + self.assertEqual(exporter.timeout, 10) + + def test_constructor_unset_timeout_env_var_uses_default(self): + with patch.dict(os.environ): + os.environ.pop(OTEL_EXPORTER_ZIPKIN_TIMEOUT, None) + with self.assertNoLogs("opentelemetry.exporter.zipkin.json", level="WARNING"): + exporter = ZipkinExporter() + self.assertEqual(exporter.timeout, 10) + + def test_constructor_numeric_timeout_env_var_preserves_values(self): + for value, expected in (("15", 15), (" 15 ", 15), ("0", 0), ("-1", -1)): + with self.subTest(value=value), patch.dict(os.environ, {OTEL_EXPORTER_ZIPKIN_TIMEOUT: value}): + with self.assertNoLogs("opentelemetry.exporter.zipkin.json", level="WARNING"): + exporter = ZipkinExporter() + self.assertEqual(exporter.timeout, expected) + + def test_constructor_explicit_timeout_ignores_invalid_env_var(self): + for value in ("10s", "", "\t\n"): + with self.subTest(value=value), patch.dict(os.environ, {OTEL_EXPORTER_ZIPKIN_TIMEOUT: value}): + with self.assertNoLogs("opentelemetry.exporter.zipkin.json", level="WARNING"): + exporter = ZipkinExporter(timeout=7) + self.assertEqual(exporter.timeout, 7) + + def test_constructor_explicit_timeout_preserves_truthiness(self): + with patch.dict(os.environ, {OTEL_EXPORTER_ZIPKIN_TIMEOUT: "15"}): + with self.assertNoLogs("opentelemetry.exporter.zipkin.json", level="WARNING"): + self.assertEqual(ZipkinExporter(timeout=0).timeout, 15) + self.assertEqual(ZipkinExporter(timeout=-1).timeout, -1) + def test_constructor_protocol_endpoint(self): """Test the constructor for the common usage of providing the protocol and endpoint arguments.""" diff --git a/exporter/opentelemetry-exporter-zipkin-proto-http/src/opentelemetry/exporter/zipkin/proto/http/__init__.py b/exporter/opentelemetry-exporter-zipkin-proto-http/src/opentelemetry/exporter/zipkin/proto/http/__init__.py index e664f20624b..28dc8f2c85c 100644 --- a/exporter/opentelemetry-exporter-zipkin-proto-http/src/opentelemetry/exporter/zipkin/proto/http/__init__.py +++ b/exporter/opentelemetry-exporter-zipkin-proto-http/src/opentelemetry/exporter/zipkin/proto/http/__init__.py @@ -78,10 +78,27 @@ DEFAULT_ENDPOINT = "http://localhost:9411/api/v2/spans" REQUESTS_SUCCESS_STATUS_CODES = (200, 202) +_DEFAULT_TIMEOUT = 10 logger = logging.getLogger(__name__) +def _timeout_from_env() -> int: + value = environ.get(OTEL_EXPORTER_ZIPKIN_TIMEOUT) + if value is None or not value.strip(): + return _DEFAULT_TIMEOUT + try: + return int(value) + except ValueError: + logger.warning( + "Invalid value %r for %s, using default of %s seconds", + value, + OTEL_EXPORTER_ZIPKIN_TIMEOUT, + _DEFAULT_TIMEOUT, + ) + return _DEFAULT_TIMEOUT + + class ZipkinExporter(SpanExporter): def __init__( self, @@ -121,7 +138,7 @@ def __init__( self.session = session or requests.Session() self.session.headers.update({"Content-Type": self.encoder.content_type()}) self._closed = False - self.timeout = timeout or int(environ.get(OTEL_EXPORTER_ZIPKIN_TIMEOUT, 10)) + self.timeout = timeout or _timeout_from_env() def export(self, spans: Sequence[Span]) -> SpanExportResult: # After the call to Shutdown subsequent calls to Export are diff --git a/exporter/opentelemetry-exporter-zipkin-proto-http/tests/test_zipkin_exporter.py b/exporter/opentelemetry-exporter-zipkin-proto-http/tests/test_zipkin_exporter.py index 78e30b0b310..522d4fb43cb 100644 --- a/exporter/opentelemetry-exporter-zipkin-proto-http/tests/test_zipkin_exporter.py +++ b/exporter/opentelemetry-exporter-zipkin-proto-http/tests/test_zipkin_exporter.py @@ -65,6 +65,52 @@ def test_constructor_env_vars(self): self.assertEqual(exporter.local_node.port, None) self.assertEqual(exporter.timeout, 15) + def test_constructor_invalid_timeout_env_var_falls_back(self): + with patch.dict(os.environ, {OTEL_EXPORTER_ZIPKIN_TIMEOUT: "10s"}): + with self.assertLogs("opentelemetry.exporter.zipkin.proto.http", level="WARNING") as cm: + exporter = ZipkinExporter() + + self.assertEqual(exporter.timeout, 10) + self.assertEqual(len(cm.records), 1) + self.assertTrue( + any("Invalid value" in message for message in cm.output), + cm.output, + ) + + def test_constructor_empty_timeout_env_var_falls_back(self): + for value in ("", " ", "\t\n"): + with self.subTest(value=value), patch.dict(os.environ, {OTEL_EXPORTER_ZIPKIN_TIMEOUT: value}): + with self.assertNoLogs("opentelemetry.exporter.zipkin.proto.http", level="WARNING"): + exporter = ZipkinExporter() + self.assertEqual(exporter.timeout, 10) + + def test_constructor_unset_timeout_env_var_uses_default(self): + with patch.dict(os.environ): + os.environ.pop(OTEL_EXPORTER_ZIPKIN_TIMEOUT, None) + with self.assertNoLogs("opentelemetry.exporter.zipkin.proto.http", level="WARNING"): + exporter = ZipkinExporter() + self.assertEqual(exporter.timeout, 10) + + def test_constructor_numeric_timeout_env_var_preserves_values(self): + for value, expected in (("15", 15), (" 15 ", 15), ("0", 0), ("-1", -1)): + with self.subTest(value=value), patch.dict(os.environ, {OTEL_EXPORTER_ZIPKIN_TIMEOUT: value}): + with self.assertNoLogs("opentelemetry.exporter.zipkin.proto.http", level="WARNING"): + exporter = ZipkinExporter() + self.assertEqual(exporter.timeout, expected) + + def test_constructor_explicit_timeout_ignores_invalid_env_var(self): + for value in ("10s", "", "\t\n"): + with self.subTest(value=value), patch.dict(os.environ, {OTEL_EXPORTER_ZIPKIN_TIMEOUT: value}): + with self.assertNoLogs("opentelemetry.exporter.zipkin.proto.http", level="WARNING"): + exporter = ZipkinExporter(timeout=7) + self.assertEqual(exporter.timeout, 7) + + def test_constructor_explicit_timeout_preserves_truthiness(self): + with patch.dict(os.environ, {OTEL_EXPORTER_ZIPKIN_TIMEOUT: "15"}): + with self.assertNoLogs("opentelemetry.exporter.zipkin.proto.http", level="WARNING"): + self.assertEqual(ZipkinExporter(timeout=0).timeout, 15) + self.assertEqual(ZipkinExporter(timeout=-1).timeout, -1) + def test_constructor_protocol_endpoint(self): """Test the constructor for the common usage of providing the protocol and endpoint arguments.""" diff --git a/opentelemetry-configuration/src/opentelemetry/configuration/_conversion.py b/opentelemetry-configuration/src/opentelemetry/configuration/_conversion.py index e5ceb0f59f5..dca1e14c805 100644 --- a/opentelemetry-configuration/src/opentelemetry/configuration/_conversion.py +++ b/opentelemetry-configuration/src/opentelemetry/configuration/_conversion.py @@ -14,7 +14,7 @@ from collections.abc import Mapping from dataclasses import MISSING, fields, is_dataclass from enum import Enum -from types import UnionType +from types import NoneType, UnionType from typing import Any, TypeVar, Union, get_args, get_origin, get_type_hints _T = TypeVar("_T") @@ -27,7 +27,7 @@ def _unwrap_optional(type_hint: Any) -> Any: """ origin = get_origin(type_hint) if origin is UnionType or origin is Union: - non_none = [t for t in get_args(type_hint) if t is not type(None)] + non_none = [t for t in get_args(type_hint) if t is not NoneType] if len(non_none) == 1: return non_none[0] return type_hint diff --git a/opentelemetry-semantic-conventions/.pylintrc b/opentelemetry-semantic-conventions/.pylintrc index b034d8c75ad..621216e432e 100644 --- a/opentelemetry-semantic-conventions/.pylintrc +++ b/opentelemetry-semantic-conventions/.pylintrc @@ -36,10 +36,6 @@ persistent=yes # Specify a configuration file. #rcfile= -# When enabled, pylint would attempt to guess common misconfiguration and emit -# user-friendly hints instead of false-positive error messages. -suggestion-mode=yes - # Allow loading of arbitrary C extensions. Extensions are imported into the # active Python interpreter and may run arbitrary code. unsafe-load-any-extension=no