Skip to content

add skeleton for aws-opentelemetry-extensions package#820

Open
liustve wants to merge 1 commit into
mainfrom
add-components-package-skeleton
Open

add skeleton for aws-opentelemetry-extensions package#820
liustve wants to merge 1 commit into
mainfrom
add-components-package-skeleton

Conversation

@liustve

@liustve liustve commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

PACKAGE NAME IS SUBJECT TO CHANGE

Adds the skeleton for a new standalone aws-opentelemetry-extensions package as the first step toward migrating ServiceEvents out of aws-opentelemetry-distro into its own package.

This PR is scaffolding only — pyproject.toml, README.rst, .coveragerc, version.py, and the amazon.opentelemetry.serviceevents namespace. No ServiceEvents code is moved yet, and nothing outside the new folder changes. Follow-up PRs will move the code over.

Scaffold the new standalone package (pyproject.toml, README.rst, .coveragerc,
version.py, and the amazon.opentelemetry.serviceevents namespace) with no code
moved yet. Nothing outside the new folder changes.
@liustve
liustve requested a review from a team as a code owner July 16, 2026 20:29
@liustve liustve added the skip changelog doesn't need a CHANGELOG entry label Jul 16, 2026
@liustve liustve changed the title feat(extensions): add skeleton for aws-opentelemetry-extensions package add skeleton for aws-opentelemetry-extensions package Jul 16, 2026
@github-actions

github-actions Bot commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Claude finished @liustve's task in 1m 46s —— View job


PR Review — Scaffolding for aws-opentelemetry-extensions

Overall the scaffolding is straightforward and self-contained. There is one clearly load-bearing issue (package-name inconsistency) plus a few smaller items worth resolving before the follow-up PRs move real code over.


🔴 Blocking: package name is inconsistent — folder says extensions, everything inside says components

The PR title/body and folder name are aws-opentelemetry-extensions, but every internal reference uses aws-opentelemetry-components. This means:

  • pip install aws-opentelemetry-components in the README will not match the folder name / any downstream wiring.
  • Coverage data will not normalize into the source tree (see below).
  • The Homepage URL points to a directory that does not exist.

Occurrences (pr-head/-relative paths):

aws-opentelemetry-extensions/pyproject.toml

[project]
name = "aws-opentelemetry-components"   # L5 — should be aws-opentelemetry-extensions?
...
description = "AWS OpenTelemetry Components (ServiceEvents)"   # L7

[project.urls]
Homepage = "https://github.com/aws-observability/aws-otel-python-instrumentation/tree/main/aws-opentelemetry-components"   # L34

aws-opentelemetry-extensions/README.rst

AWS OpenTelemetry Components         # L1 —
...
    pip install aws-opentelemetry-components   # L9

aws-opentelemetry-extensions/.coveragerc (this one has real functional impact — the [paths] remap won't match the on-disk layout, so coverage from tox envs will not combine correctly with source paths):

[paths]
source =
    aws-opentelemetry-components/src/amazon   # L6 — path does not exist on disk
    .tox/3.*/lib/python*/site-packages/amazon
    */site-packages/amazon

Please pick one name and align every file. Given the PR title, I'd expect aws-opentelemetry-extensions throughout. Fix this →


🟡 Dependency-pinning strategy differs from distro

aws-opentelemetry-distro/pyproject.toml pins its OTel deps to exact versions (e.g. opentelemetry-api == 1.42.1). The new aws-opentelemetry-extensions/pyproject.toml uses ranges:

"opentelemetry-api >= 1.39, < 2.0",
"opentelemetry-sdk >= 1.39, < 2.0",
"opentelemetry-exporter-otlp-proto-http >= 1.39, < 2.0",
"opentelemetry-semantic-conventions >= 0.60b0",

Since distro will consume this package, pip's resolver has to reconcile the two. With distro's exact pins winning, this is likely fine day-to-day, but the range in extensions is misleading: it implies compatibility with 1.39.x1.41.x even though distro only ships with 1.42.1. Two options:

  • Pin extensions to the same versions as distro (safest — mirrors the shipped combination).
  • Or explicitly document that these ranges represent the tested floor when the package is consumed standalone, not by distro.

The current inconsistency will bite when a future distro bump wants to move OTel forward and extensions silently allows an older pin.


🟡 Coverage threshold: 70% vs distro's 95%

# .coveragerc
fail_under = 70.00

Distro's root .coveragerc uses fail_under = 95.00. If ServiceEvents is moving over, presumably it already has (or should carry over) high coverage. Starting at 70% invites drift. Consider matching 95% (or documenting why extensions is intentionally lower — e.g. because upcoming ServiceEvents code has integration-only paths).


🟢 Minor / nits

  • README.rst L11–L13: The intro paragraph appears below the "Installation" section (pip install … on L9), which reads oddly. Typical layout is a one-line description → Installation → Components. Consider moving the "Shared components for the AWS Distro…" sentence above the install block.
  • Missing LICENSE in the package root: pyproject.toml declares license = {text = "Apache-2.0"} but there's no LICENSE file inside aws-opentelemetry-extensions/. Hatchling won't include one in the built sdist/wheel unless present. The repo-root LICENSE covers the source tree, but distributed artifacts typically ship their own copy. Not a blocker for a scaffold PR, but worth resolving before the first release.
  • No [tool.hatch.build.targets.sdist] tests inclusion: distro's sdist target includes /tests — you'll want the same once tests land, but it's fine to defer.

Todo

  • Read prior review comments to dedupe (none present)
  • Review PR diff
  • Analyze scaffolding files
  • Post review findings

Summary: One blocking naming inconsistency to fix; a couple of policy questions (dep pinning, coverage floor) that are worth discussing before this becomes a load-bearing package.
Branch: add-components-package-skeleton

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

Labels

skip changelog doesn't need a CHANGELOG entry

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant