Skip to content

fix(opentelemetry): give inject_core_spans its own tracer cache key#13633

Open
shreemaan-abhishek wants to merge 1 commit into
apache:masterfrom
shreemaan-abhishek:fix/opentelemetry-namespace-inject-tracer-cache-key
Open

fix(opentelemetry): give inject_core_spans its own tracer cache key#13633
shreemaan-abhishek wants to merge 1 commit into
apache:masterfrom
shreemaan-abhishek:fix/opentelemetry-namespace-inject-tracer-cache-key

Conversation

@shreemaan-abhishek

Copy link
Copy Markdown
Contributor

Description

Follow-up to #13618.

_M.rewrite() and inject_core_spans() build different tracers: rewrite uses the route's configured sampler, while inject_core_spans deliberately forces an always_on sampler so that, once a trace's root span is recording, the full internal core-span tree is kept unconditionally regardless of the route sampler.

After #13618 both paths called core.lrucache.plugin_ctx() with the same extra key (metadata.modifiedIndex) on the same api_ctx, so they shared a single lrucache slot. On a warm worker, rewrite (rewrite phase) populates the slot first, and inject_core_spans (log phase) then reuses that route-sampler tracer instead of the always_on one it built, silently dropping the always_on guarantee for any sampler that is not trace_id-deterministic.

This change namespaces the inject cache key (inject_core_spans#<modifiedIndex>) so the two tracers no longer collide, while preserving the rebuild-on-metadata-change behavior from #13618.

It also hardens the regression test setup added in #13618 to fail fast when the admin update is rejected or the metadata never propagates to the worker, so the test cannot return ok on a broken setup and mask the real assertion.

Checklist

  • I have explained the need for this PR and the problem it solves
  • I have explained the changes or the new features added to this PR
  • I have added tests corresponding to this change
  • I have updated the documentation to reflect this change (no doc change needed; internal behavior)
  • I have verified that the changes are backward compatible

rewrite() and inject_core_spans() build different tracers (route sampler
vs forced always_on) but shared one lrucache slot keyed only on
metadata.modifiedIndex, so a warm worker could hand inject_core_spans the
route-sampler tracer and silently drop the always_on guarantee. Namespace
the inject cache key so the always_on tracer is preserved, while keeping
the rebuild-on-metadata-change behavior.

Also harden the regression test setup to fail fast when the admin update
is rejected or the metadata never propagates to the worker, instead of
returning "ok" on a broken setup and masking the real assertion.
@dosubot dosubot Bot added the size:M This PR changes 30-99 lines, ignoring generated files. label Jun 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:M This PR changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants