Skip to content

LCORE-1823: Add integration tests for OpenTelemetry trace context propagation - #2422

Open
anik120 wants to merge 1 commit into
lightspeed-core:mainfrom
anik120:trace-ctx-prop-integration-tests
Open

LCORE-1823: Add integration tests for OpenTelemetry trace context propagation#2422
anik120 wants to merge 1 commit into
lightspeed-core:mainfrom
anik120:trace-ctx-prop-integration-tests

Conversation

@anik120

@anik120 anik120 commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Description

  • Add integration tests verifying W3C trace context propagation through the query endpoint
  • Tests cover: trace ID continuation from incoming traceparent header, parent-child span relationships, trace ID consistency across components, and expected child span emission (quota.check, shield.moderate, llm.inference)
  • Uses module-scoped InMemorySpanExporter fixture to capture spans without external collectors

Type of change

  • Refactor
  • New feature
  • Bug fix
  • CVE fix
  • Optimization
  • Documentation Update
  • Configuration Update
  • Bump-up service version
  • Bump-up dependent library [pyproject.toml + uv.lock]
  • Bump-up dependent library [requirements.*.txt for Konflux]
  • Bump-up library or tool used for development (does not change the final image)
  • CI configuration change
  • Konflux configuration change
  • Unit tests improvement
  • Integration tests improvement
  • End to end tests improvement
  • Benchmarks improvement

Tools used to create PR

Identify any AI code assistants used in this PR (for transparency and review context)

  • Assisted-by: (e.g., Claude, CodeRabbit, Ollama, etc., N/A if not used)
  • Generated by: (e.g., tool name and version; N/A if not used)

Related Tickets & Documents

  • Related Issue #
  • Closes #

Checklist before requesting a review

  • I have performed a self-review of my code.
  • PR has passed all pre-merge test jobs.
  • If it is a core feature, I have added thorough tests.

Testing

  • Please provide detailed steps to perform tests related to this code change.
  • How were the fix/results from this change verified? Please provide relevant screenshots or results.

Summary by CodeRabbit

  • Tests
    • Added integration coverage for OpenTelemetry trace propagation through the query endpoint.
    • Verified trace continuity, incoming parent relationships, cross-component trace sharing, and parent-child span relationships.
    • Confirmed expected query, quota, shield, and inference spans are emitted.

…pagation

- Add integration tests verifying W3C trace context propagation through the query endpoint
- Tests cover: trace ID continuation from incoming `traceparent` header, parent-child span relationships,
trace ID consistency across components, and expected child span emission (quota.check, shield.moderate, llm.inference)
- Uses module-scoped `InMemorySpanExporter` fixture to capture spans without external collectors
@coderabbitai

coderabbitai Bot commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Walkthrough

Changes

OpenTelemetry trace propagation

Layer / File(s) Summary
Span collection and test context
tests/integration/test_otel_trace_propagation.py
Adds known W3C trace context values, an in-memory span collector, span reset handling, and a helper for attaching incoming trace context.
Query span propagation assertions
tests/integration/test_otel_trace_propagation.py
Adds integration tests for trace ID continuity, incoming parentage, shared trace IDs, parent-child relationships, and expected query-flow spans.

Estimated code review effort: 2 (Simple) | ~10 minutes

🚥 Pre-merge checks | ✅ 7
✅ Passed checks (7 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the added integration tests for OpenTelemetry trace context propagation.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Performance And Algorithmic Complexity ✅ Passed The pull request adds only integration tests. It introduces no production handlers, API list operations, per-item queries, unbounded buffers, or new algorithmic work.
Security And Secret Handling ✅ Passed The commit adds only an integration-test file. Its fixed values are W3C trace IDs, and it adds no credentials, logging, routes, response data, or injection paths.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
✨ Simplify code
  • Create PR with simplified code

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot 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.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@tests/integration/test_otel_trace_propagation.py`:
- Around line 72-82: Update the integration tests to propagate W3C context
through the mounted ASGI/HTTP route instead of calling _inject_w3c_context
before the handler. Send the TRACEPARENT header on the Request, remove direct
context attachment from these scenarios, and assert the emitted root span
context retains the incoming parent trace ID and span ID.
- Around line 55-63: Update the fixture around the provider installation and
yield to save the original values of trace._TRACER_PROVIDER and
trace._TRACER_PROVIDER_SET_ONCE._done, then use a finally block to shut down
only the test provider and restore both saved values. Keep the test provider
setup intact while ensuring cleanup runs even when the test fails.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: da29147a-99a2-4a92-8468-f80cbb73226a

📥 Commits

Reviewing files that changed from the base of the PR and between 482c7b7 and e0c1f9a.

📒 Files selected for processing (1)
  • tests/integration/test_otel_trace_propagation.py
📜 Review details
🧰 Additional context used
📓 Path-based instructions (2)
**/*

📄 CodeRabbit inference engine (Custom checks)

**/*: Flag meaningful O(n^2)+ algorithms on non-trivial inputs, including handlers and Kubernetes list operations.
Flag N+1 patterns that list items and then query once per item, including Kubernetes API and database access.
Flag expensive work inside loops, including API calls, JSON parsing, and regex compilation.
Flag unbounded growth in caches, watchers, or buffers when eviction or limits are missing.
Flag missing pagination or limits on list operations and API endpoints.
Flag secrets or tokens logged in plaintext or hardcoded in source.
Flag API endpoints missing authentication or authorization.
Flag injection vulnerabilities, including SQL injection, command injection, and path traversal.
Flag sensitive data leaked in API responses, WebSocket messages, or logs.
Flag Kubernetes Secrets and Red Hat secrets missing OwnerReferences.

Files:

  • tests/integration/test_otel_trace_propagation.py
tests/integration/**/*.py

📄 CodeRabbit inference engine (AGENTS.md)

Use pytest for integration tests and maintain at least 10% integration-test coverage.

Files:

  • tests/integration/test_otel_trace_propagation.py
🧠 Learnings (1)
📚 Learning: 2026-06-24T13:45:37.249Z
Learnt from: Jdubrick
Repo: lightspeed-core/lightspeed-stack PR: 1971
File: src/utils/markdown_repair.py:31-36
Timestamp: 2026-06-24T13:45:37.249Z
Learning: In the lightspeed-stack repository, docstrings must use the section header name "Parameters:" (not "Args:") for function arguments, even if the project references Google Python docstring conventions. Ensure docstrings follow the project’s established "Parameters:" header format for any documented function parameters.

Applied to files:

  • tests/integration/test_otel_trace_propagation.py

Comment on lines +55 to +63
trace._TRACER_PROVIDER_SET_ONCE._done = False
trace._TRACER_PROVIDER = None
trace.set_tracer_provider(provider)

yield exporter

provider.shutdown()
trace._TRACER_PROVIDER_SET_ONCE._done = False
trace._TRACER_PROVIDER = None

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.

🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

ast-grep outline tests/integration/test_otel_trace_propagation.py --items all --type function

rg -n -C 4 \
  '(_TRACER_PROVIDER|_TRACER_PROVIDER_SET_ONCE|set_tracer_provider|get_tracer\()' \
  --glob '*.py'

Repository: lightspeed-core/lightspeed-stack

Length of output: 716


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- tests/integration/test_otel_trace_propagation.py ---'
cat -n tests/integration/test_otel_trace_propagation.py

printf '%s\n' '--- OpenTelemetry provider-state references ---'
rg -n -C 3 \
  '_TRACER_PROVIDER|_TRACER_PROVIDER_SET_ONCE|set_tracer_provider|get_tracer\(' \
  . \
  -g '*.py' \
  -g 'pyproject.toml' \
  -g 'requirements*.txt' \
  -g 'poetry.lock' \
  -g 'uv.lock' \
  -g 'Pipfile*' || true

printf '%s\n' '--- project dependency declarations ---'
rg -n -C 2 'opentelemetry' pyproject.toml requirements*.txt poetry.lock uv.lock Pipfile* 2>/dev/null || true

Repository: lightspeed-core/lightspeed-stack

Length of output: 35579


🏁 Script executed:

#!/bin/bash
set -euo pipefail

cat -n tests/integration/test_otel_trace_propagation.py

rg -n -C 3 \
  '_TRACER_PROVIDER|_TRACER_PROVIDER_SET_ONCE|set_tracer_provider|get_tracer\(' \
  . -g '*.py' -g 'pyproject.toml' -g 'requirements*.txt' -g 'poetry.lock' -g 'uv.lock' -g 'Pipfile*' || true

rg -n -C 2 'opentelemetry' \
  pyproject.toml requirements*.txt poetry.lock uv.lock Pipfile* 2>/dev/null || true

Repository: lightspeed-core/lightspeed-stack

Length of output: 35434


🏁 Script executed:

#!/bin/bash
set -euo pipefail

python3 - <<'PY'
import importlib.util
import inspect

spec = importlib.util.find_spec("opentelemetry.trace")
print("opentelemetry.trace available:", spec is not None)
if spec is not None:
    from opentelemetry import trace
    from opentelemetry.trace import ProxyTracer

    print("trace module:", trace.__file__)
    print("ProxyTracer source:")
    print(inspect.getsource(ProxyTracer))
    print("set_tracer_provider source:")
    print(inspect.getsource(trace.set_tracer_provider))
    print("get_tracer_provider source:")
    print(inspect.getsource(trace.get_tracer_provider))
PY

printf '%s\n' '--- test ordering and global fixture usage ---'
rg -n -C 3 \
  'test_otel_trace_propagation|otel_collector|TracerProvider\(|set_tracer_provider\(|shutdown\(' \
  tests conftest.py src 2>/dev/null || true

Repository: lightspeed-core/lightspeed-stack

Length of output: 427


🏁 Script executed:

#!/bin/bash
set -euo pipefail

python3 - <<'PY'
import importlib.util
import inspect

spec = importlib.util.find_spec("opentelemetry.trace")
print("opentelemetry.trace available:", spec is not None)

if spec is not None:
    from opentelemetry import trace
    from opentelemetry.trace import ProxyTracer

    print("trace module:", trace.__file__)
    print(inspect.getsource(ProxyTracer))
    print(inspect.getsource(trace.set_tracer_provider))
    print(inspect.getsource(trace.get_tracer_provider))
PY

rg -n -C 3 \
  'test_otel_trace_propagation|otel_collector|TracerProvider\(|set_tracer_provider\(|shutdown\(' \
  tests conftest.py src 2>/dev/null || true

Repository: lightspeed-core/lightspeed-stack

Length of output: 427


🌐 Web query:

OpenTelemetry Python 1.44.0 trace.py ProxyTracer _TRACER_PROVIDER _TRACER_PROVIDER_SET_ONCE set_tracer_provider source

💡 Result:

In OpenTelemetry Python (including version 1.44.0), the components you mentioned are part of the global tracing API defined in the opentelemetry.trace module [1][2]. These mechanisms work together to manage the global TracerProvider and ensure that tracers can be requested even before a provider is fully initialized [2][3]. ProxyTracer and ProxyTracerProvider The ProxyTracerProvider acts as a placeholder when no global TracerProvider has been configured [2]. When an application calls get_tracer, it initially returns a ProxyTracer [2]. This ProxyTracer holds a reference to the eventual "real" tracer [1][3]. When a span is started through a ProxyTracer, the tracer lazily retrieves or initializes the real tracer from the global _TRACER_PROVIDER once it is set [1][2]. _TRACER_PROVIDER and _TRACER_PROVIDER_SET_ONCE _TRACER_PROVIDER is a module-level variable that holds the actual, user-configured TracerProvider [1][2]. The _TRACER_PROVIDER_SET_ONCE object is typically a synchronization primitive (often a threading lock or a similar mechanism) used to ensure that the global provider can only be configured once, preventing multiple conflicting initializations [2][4]. set_tracer_provider The function set_tracer_provider is the public API used to assign a concrete TracerProvider to the global _TRACER_PROVIDER [2][5]. When this function is called: 1. It acquires the lock (controlled by _TRACER_PROVIDER_SET_ONCE) to verify if a provider has already been set [2][4]. 2. If already set, it may raise a warning or ignore subsequent calls, ensuring the global state remains consistent [6]. 3. Once set, future requests to get_tracer will return tracers directly from the newly configured global provider rather than returning new ProxyTracers [2]. This design allows for decoupling: libraries can request tracers at import time (receiving a ProxyTracer), and the application can inject the actual tracing implementation (e.g., SDK) later during startup [2][5].

Citations:


🌐 Web query:

site:github.com/open-telemetry/opentelemetry-python "class ProxyTracer" "_real_tracer" "get_tracer_provider"

💡 Result:

In the OpenTelemetry Python API, ProxyTracer and ProxyTracerProvider are internal components used to manage the global tracer provider lifecycle [1][2]. When you call get_tracer_provider, if no global TracerProvider has been initialized or set, the library returns a ProxyTracerProvider instead of a concrete implementation [1][2]. The ProxyTracer acts as a placeholder for a real Tracer [1][2]. It maintains an internal attribute, self._real_tracer, which is initially set to None [1][2]. When an operation is performed using the proxy, it checks the global _TRACER_PROVIDER [1][2]. If a provider has been set since the proxy was created, the proxy uses that provider to retrieve a real Tracer (the _real_tracer) and subsequently delegates calls to it [1][2]. If no global provider is available, it defaults to a NoOpTracer to ensure the application continues to function without errors [1][2]. This mechanism allows instrumentation code to acquire tracers before the global SDK configuration is fully loaded or set in the application's lifecycle [1][2].

Citations:


🏁 Script executed:

#!/bin/bash
set -euo pipefail

url='https://raw.githubusercontent.com/open-telemetry/opentelemetry-python/v1.44.0/opentelemetry-api/src/opentelemetry/trace/__init__.py'
curl -fsSL "$url" |
  rg -n -A 45 -B 8 \
    'class ProxyTracer|def set_tracer_provider|_TRACER_PROVIDER_SET_ONCE|def get_tracer_provider'

Repository: lightspeed-core/lightspeed-stack

Length of output: 8156


Restore OpenTelemetry global state in a finally block.

Save _TRACER_PROVIDER and _TRACER_PROVIDER_SET_ONCE._done before installation. Shut down only the test provider, then restore both values. ProxyTracer caches the test provider's tracer, so later tests can emit through a shut-down provider after this fixture sets the global provider to None.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/integration/test_otel_trace_propagation.py` around lines 55 - 63,
Update the fixture around the provider installation and yield to save the
original values of trace._TRACER_PROVIDER and
trace._TRACER_PROVIDER_SET_ONCE._done, then use a finally block to shut down
only the test provider and restore both saved values. Keep the test provider
setup intact while ensuring cleanup runs even when the test fails.

Comment on lines +72 to +82
def _inject_w3c_context(traceparent: str) -> object:
"""Extract a W3C traceparent header into OTel context and attach it.

Parameters:
traceparent: W3C Trace Context header value.

Returns:
Context token to pass to ``otel_context.detach``.
"""
ctx = TraceContextTextMapPropagator().extract({"traceparent": traceparent})
return otel_context.attach(ctx)

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.

🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift

Test W3C extraction through the HTTP request path.

_inject_w3c_context sets the current process context before the direct handler call. It does not send TRACEPARENT in Request headers. It bypasses FastAPI instrumentation and HTTP header extraction.

A missing or broken extractor can pass test_incoming_trace_context_is_continued and test_root_span_is_child_of_incoming_parent. Send the header through the mounted ASGI route, then assert the emitted root span context.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/integration/test_otel_trace_propagation.py` around lines 72 - 82,
Update the integration tests to propagate W3C context through the mounted
ASGI/HTTP route instead of calling _inject_w3c_context before the handler. Send
the TRACEPARENT header on the Request, remove direct context attachment from
these scenarios, and assert the emitted root span context retains the incoming
parent trace ID and span ID.

@tisnik tisnik 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.

LGTM. There are minor nits, but these can be handled later.

@asimurka asimurka 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.

LGTM

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