Skip to content

refactor!: move the otlp-json package to be a regular ops-tracing module#2585

Merged
tonyandrewmeyer merged 4 commits into
canonical:mainfrom
tonyandrewmeyer:de-vendor-otlp-json
Jun 26, 2026
Merged

refactor!: move the otlp-json package to be a regular ops-tracing module#2585
tonyandrewmeyer merged 4 commits into
canonical:mainfrom
tonyandrewmeyer:de-vendor-otlp-json

Conversation

@tonyandrewmeyer

@tonyandrewmeyer tonyandrewmeyer commented Jun 26, 2026

Copy link
Copy Markdown
Collaborator

This PR moves the vendor/otlp_json package 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:

$ git checkout de-vendor-otlp-json
Switched to branch 'de-vendor-otlp-json'
Your branch is up to date with 'origin/de-vendor-otlp-json'.
$ uv run --with '.[tracing]' python -c 'import ops_tracing.vendor.otlp_json'
Traceback (most recent call last):
  File "<string>", line 1, in <module>
ModuleNotFoundError: No module named 'ops_tracing.vendor.otlp_json'
$ git checkout main
Switched to branch 'main'
Your branch is up to date with 'origin/main'.
$ uv run --with '.[tracing]' python -c 'import ops_tracing.vendor.otlp_json'
$ 

(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.

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 james-garner-canonical left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@james-garner-canonical

Copy link
Copy Markdown
Contributor

WDYT about moving ops_tracing/vendor to ops_tracing/_vendor in a separate PR, while we're breaking things?

@tonyandrewmeyer

Copy link
Copy Markdown
Collaborator Author

Looks good, modulo the type checking failure.

Odd, it passes locally. Python version maybe, I will check.

@tonyandrewmeyer

Copy link
Copy Markdown
Collaborator Author

Looks good, modulo the type checking failure.

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 tromai left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you. It looks good.

@tonyandrewmeyer

Copy link
Copy Markdown
Collaborator Author

WDYT about moving ops_tracing/vendor to ops_tracing/_vendor in a separate PR, while we're breaking things?

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.

@tonyandrewmeyer tonyandrewmeyer merged commit 7f7759f into canonical:main Jun 26, 2026
59 checks passed
@tonyandrewmeyer tonyandrewmeyer deleted the de-vendor-otlp-json branch June 26, 2026 04:39
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.

3 participants