Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 12 additions & 7 deletions deploy/lineage-attach/DESIGN.md
Original file line number Diff line number Diff line change
Expand Up @@ -184,13 +184,18 @@ the shim's interlock lets the image through.

### Baking is not purely additive

The shim installs one pinned OpenTelemetry contrib release
(`OTEL_CONTRIB_VERSION` in `Dockerfile.otel-shim`), and the resolver brings
the SDK that release requires — on an image that already carries an older SDK,
the bake *upgrades* it (observed: `opentelemetry-sdk 1.42.1 → 1.44.0`, semconv
`0.63b1 → 0.65b0`). Harmless for an app that does not pin, but an app pinning
an older SDK can be affected by being wrapped. Check the build output if your
app is sensitive to those versions.
The install resolves into the app's own environment, so it can move a package
the OpenTelemetry set depends on (`wrapt`, `typing-extensions`); `uv pip check`
fails the build when the app pins one. The OpenTelemetry core itself does not
move when the app already carries it: each contrib release requires one exact
core (`0.63b1` requires `opentelemetry-api==1.42.1`), so `Dockerfile.otel-shim`
pins the install to the app's `opentelemetry-api` and the resolver lands on the
paired release. An app with no OpenTelemetry, or with `opentelemetry-api` below
1.34.0, gets `OTEL_CONTRIB_VERSION`; for the older app that moves its core, and
`uv pip check` decides. (The releases paired with older cores instrument only
`starlette<0.15`, and before `0.52b0` they lack the `initialize()` the hook
calls. The releases paired with 1.34.0 to 1.40.x require `wrapt<2`, so a
`wrapt` 2.x the app holds loosely is moved down.)

The static `uv` used to install the shim is copied into the image and left
there — a `rm` in a later layer would not reclaim the copy layer's space, so
Expand Down
44 changes: 29 additions & 15 deletions deploy/lineage-attach/Dockerfile.otel-shim
Original file line number Diff line number Diff line change
Expand Up @@ -42,11 +42,20 @@ USER root
# Our own static uv, pinned by digest: a builder-stage image may have neither
# uv nor pip in its runtime layer.
COPY --from=ghcr.io/astral-sh/uv:0.9.24@sha256:816fdce3387ed2142e37d2e56e1b1b97ccc1ea87731ba199dc8a25c04e4997c5 /uv /usr/local/bin/uv
# One contrib release for everything, so a bake never silently upgrades the
# OTel set under an app. -distro is the SDK; the eight others — server
# (starlette/asgi/fastapi), client (httpx/requests/aiohttp/urllib3) and
# -threading — are the instrumentors build-otel-shim.sh's interlock probes
# for; keep the two lists in sync.
# One contrib release for everything, and the app chooses it. Each release
# requires one exact OpenTelemetry core (0.63b1 requires
# opentelemetry-api==1.42.1, through opentelemetry-semantic-conventions), so
# the install is pinned to the opentelemetry-api the app already carries and
# the resolver lands on the one release paired with it: the bake does not move
# the api/sdk under the app. An app that carries none, or one older than
# 1.34.0, is pinned to OTEL_CONTRIB_VERSION instead; there the bake may move
# the core, and the check below decides. (1.34.0 pairs with contrib 0.55b0:
# earlier releases instrument only starlette<0.15, and before 0.52b0 they lack
# the initialize() the hook calls.)
# -distro is the SDK; the eight others — server (starlette/asgi/fastapi),
# client (httpx/requests/aiohttp/urllib3) and -threading — are the
# instrumentors build-otel-shim.sh's interlock probes for; keep the two lists
# in sync.
# `-urllib3` covers clients that bypass `requests`: boto3/botocore (S3 and
# every AWS-style store) drive urllib3 directly. Without it an object read
# lands in a trace of its own (seen live: a MinIO GetObject escaping into a
Expand All @@ -58,16 +67,21 @@ COPY --from=ghcr.io/astral-sh/uv:0.9.24@sha256:816fdce3387ed2142e37d2e56e1b1b97c
# `--system --break-system-packages` lets the install land in a non-venv
# python too; both flags are inert when --python is a venv.
ARG OTEL_CONTRIB_VERSION=0.65b0
RUN uv pip install --no-cache --python "${VENV_PYTHON}" --system --break-system-packages \
"opentelemetry-distro==${OTEL_CONTRIB_VERSION}" \
"opentelemetry-instrumentation-starlette==${OTEL_CONTRIB_VERSION}" \
"opentelemetry-instrumentation-asgi==${OTEL_CONTRIB_VERSION}" \
"opentelemetry-instrumentation-fastapi==${OTEL_CONTRIB_VERSION}" \
"opentelemetry-instrumentation-httpx==${OTEL_CONTRIB_VERSION}" \
"opentelemetry-instrumentation-requests==${OTEL_CONTRIB_VERSION}" \
"opentelemetry-instrumentation-aiohttp-client==${OTEL_CONTRIB_VERSION}" \
"opentelemetry-instrumentation-urllib3==${OTEL_CONTRIB_VERSION}" \
"opentelemetry-instrumentation-threading==${OTEL_CONTRIB_VERSION}"
RUN pin="$("${VENV_PYTHON}" -c 'from importlib.metadata import version; v = version("opentelemetry-api"); \
assert tuple(map(int, v.split(".")[:2])) >= (1, 34); print("opentelemetry-api==" + v)' 2>/dev/null)" \
Comment on lines +70 to +71

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

suggestion: the floor disappears when the image sets PYTHONOPTIMIZE. RUN inherits the base image's ENV, and with PYTHONOPTIMIZE set Python skips the assert, so a core from 1.31 to 1.33 is pinned to itself instead of falling back to OTEL_CONTRIB_VERSION.

I ran this RUN's shell (lines joined the way the Dockerfile joins them) against a venv holding opentelemetry-api==1.32.1:

  • without the variable: the bake is pinned to opentelemetry-distro==0.65b0
  • with PYTHONOPTIMIZE=1: the bake is pinned to opentelemetry-api==1.32.1, and uv pip install --dry-run resolves opentelemetry-instrumentation-starlette==0.53b1. That is the starlette<0.15 release this floor exists to keep out.

Nothing after this catches it. With that set installed next to starlette 1.7.0:

  • uv pip check passes;
  • verify_propagates's snippet passes, because 0.53b1 already has initialize();
  • StarletteInstrumentor().is_instrumented_by_opentelemetry is False. The only sign is a DependencyConflict: requested: "starlette >= 0.13, <0.15" but found: "starlette 1.7.0" log line.

So a pure-Starlette app bakes and attests clean, then splits into separate traces at runtime. (verify_propagates also uses assert, so under the same env it would pass even for a core below 1.31, where initialize() is missing. That's older code outside this diff.)

The fix can't be an if: the Dockerfile joins the continued lines into one, and a compound statement can't follow ;. So the check has to stay an expression. I tested this with PYTHONOPTIMIZE unset, 1 and 2, on cores 1.32.1, 1.43.0 and none:

Suggested change
RUN pin="$("${VENV_PYTHON}" -c 'from importlib.metadata import version; v = version("opentelemetry-api"); \
assert tuple(map(int, v.split(".")[:2])) >= (1, 34); print("opentelemetry-api==" + v)' 2>/dev/null)" \
RUN pin="$("${VENV_PYTHON}" -c 'from importlib.metadata import version; import sys; v = version("opentelemetry-api"); \
tuple(map(int, v.split(".")[:2])) >= (1, 34) or sys.exit(1); print("opentelemetry-api==" + v)' 2>/dev/null)" \

|| pin="opentelemetry-distro==${OTEL_CONTRIB_VERSION}"; \
echo "the bake is pinned to ${pin}"; \
uv pip install --no-cache --python "${VENV_PYTHON}" --system --break-system-packages "${pin}" \
opentelemetry-distro \
opentelemetry-instrumentation-starlette \
opentelemetry-instrumentation-asgi \
opentelemetry-instrumentation-fastapi \
opentelemetry-instrumentation-httpx \
opentelemetry-instrumentation-requests \
opentelemetry-instrumentation-aiohttp-client \
opentelemetry-instrumentation-urllib3 \
opentelemetry-instrumentation-threading \
|| { echo "REFUSING: the shim could not be installed alongside ${pin} (uv's error is above; if it found no solution, no contrib release is paired with that version)" >&2; exit 1; }

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nit: when pin is the fallback, this hint blames the wrong thing. Against a Python 3.9 venv with no OpenTelemetry:

  ╰─▶ Because the current Python version (3.9.23) does not satisfy
      Python>=3.10 and opentelemetry-distro==0.65b0 depends on Python>=3.10, ...
REFUSING: the shim could not be installed alongside opentelemetry-distro==0.65b0 (uv's error is above; if it found no solution, no contrib release is paired with that version)

"That version" is opentelemetry-distro==0.65b0 there. Either drop the hint and leave the explanation in RECIPE, or print it only when pin starts with opentelemetry-api==.

# The install resolves into the app's OWN environment, so it may move a package
# an app dependency pins (wrapt, deprecated, typing-extensions are the usual
# suspects) — and the attestations import only OpenTelemetry, never the app.
Expand Down
2 changes: 1 addition & 1 deletion deploy/lineage-attach/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -318,7 +318,7 @@ BAKE — once per app image ATTACH — once per Deployment
| `RECIPE.md` · `DESIGN.md` | the step-by-step; the reasoning, envelope and limits |
| `attach-lineage.sh` | **the one generator** — every YAML byte of the attachment, `EMIT=patch` / `EMIT=cm`, env-driven, stdout only, every input validated or refused |
| `sidecar-patch.sh` | the live applier: preconditions, then ConfigMap + patch + rollout wait; owns no YAML |
| `Dockerfile.otel-shim` | the propagate-only layer, one recipe for every in-envelope app, instrumentors pinned to one contrib release |
| `Dockerfile.otel-shim` | the propagate-only layer, one recipe for every in-envelope app, instrumentors on the contrib release paired with the app's own `opentelemetry-api` (a pinned default when it carries none, or one older than 1.34.0) |
| `build-otel-shim.sh` | bakes, attests (gate off: nothing OTel-shaped loads; gate on: a `traceparent` is injected), kind-loads; refuses images it cannot safely wrap |
| `lineage-propagate-hook.py` | the env-gated site hook the Dockerfile installs (`.pth` + module); read its docstring for the contract |
| `container-runtime.sh` | sourced helper: docker vs podman, kind load either way |
Expand Down
1 change: 1 addition & 0 deletions deploy/lineage-attach/RECIPE.md
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ attestation runs before the load and prints nothing when it passes).
Fail `REFUSING to bake … already instruments …` (exit 3): the app instruments itself — go to step 3 **without** `APP_CONTAINER`/`APP_IMAGE` (capture only).
Fail `REFUSING to bake … no runnable Python found` (exit 3): outside the shim's envelope (DESIGN "The envelope") — same, capture only; or pass the interpreter as arg 3 if you know it.
Fail `REFUSING to bake … is not present locally` (exit 3): wrong `IMAGE` — see Inputs; nothing was built.
Fail `REFUSING: the shim could not be installed alongside …` (the build fails): no contrib release is paired with the app's `opentelemetry-api` (a core newer than its contrib release, or a package index that lags) — uv's own error is above the line.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

suggestion: every other Fail line here tells the reader what to do next, and this one only explains the cause. Maybe end it with a step, e.g. "— capture only (step 3 without APP_CONTAINER/APP_IMAGE) until the paired contrib release reaches the index, then re-bake."

This line can also be reached with no paired-release problem at all, on the fallback pin. A Python 3.9 app with no OpenTelemetry lands here because uv reports opentelemetry-distro==0.65b0 depends on Python>=3.10. One clause for that case would keep the reader from looking for a pairing that doesn't matter.

Fail `ATTESTATION FAILED` (exit 4): the bake itself is broken (the image was not loaded) — read the assertion it prints; not an app property.

## 3. Attach (once per Deployment)
Expand Down
Loading