refactor!: move the otlp-json package to be a regular ops-tracing module#2585
Conversation
Moves tracing/ops_tracing/vendor/otlp_json/__init__.py to tracing/ops_tracing/otlp_json.py and updates the import in _export.py. Lint compliance for the de-vendored module is addressed in a follow-up commit.
Adds copyright, module docstring, public-function docstring; switches to single quotes and sorted imports; replaces try/except/pass with contextlib.suppress; annotates SDK-invariant asserts with the project's standard noqa rationale comment. Also fixes the import order in _export.py exposed by the de-vendor move.
It was never re-exported through ops_tracing's public namespace (it lived under vendor/), so make that explicit with a leading underscore.
james-garner-canonical
left a comment
There was a problem hiding this comment.
Looks good, modulo the type checking failure.
The breaking change seems fine to me, I don't find any hits in our org other than megademo-pebble-everywhere which just has a copy of Ops.
|
WDYT about moving |
Odd, it passes locally. Python version maybe, I will check. |
Ah, you need to lint separately not from the top. We need to clean that up at some point. |
The _VALUE alias only covered leaves and sequences, but upstream attribute values are typed as LabelValue | AnyValue, where AnyValue includes Mapping[str, AnyValue]. The runtime already handles Mapping, only the annotation was too narrow. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
tromai
left a comment
There was a problem hiding this comment.
Thank you. It looks good.
My preference would be to find time to get #2557 in a good shape and get that approved and merged. But I'm not opposed to the simple small change now if you're keen. |
This PR moves the
vendor/otlp_jsonpackage to be a single-file private module in ops-tracing.There is an archived repository with the otlp-json package, and that fed the PyPI package, but we didn't use those and had our own copy, but in a vendor directory. The package was developed by Canonical. This cleans up the situation, and also avoids a vendor package, which we are trying to stop having in general.
This is a breaking change:
(If you do this, make sure you don't have a
__pycache__lying around.)However, I think the name "vendor" should have been a clear enough signal that no-one should have been doing that.
Possibly easiest to review commit-by-commit, as two are simple moves. The rest is automated ruff cleanup and import changes.