fix(python): support public tracer providers - #859
Conversation
WalkthroughTracing setup now accepts any provider exposing ChangesTracing provider compatibility
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
302c4c3 to
5ca69c7
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
python/tests/test_tracing_setup.py (1)
144-157: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert processor attachment in this regression test.
The test passes only
instrumentors, so it does not verify the PR’s collector/exporter attachment behavior. Pass a spy processor/exporter and assertprovider.add_span_processor(...)is called, while also confirming the global provider is not replaced.Based on the PR objective, this path must attach collectors/exporters to the existing provider.
🤖 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 `@python/tests/test_tracing_setup.py` around lines 144 - 157, Update test_instruments_an_existing_public_provider to provide a spy processor/exporter through the _do_setup configuration, then assert the existing provider’s add_span_processor method receives it and trace.set_tracer_provider is not called. Preserve the existing assertion that the instrumentor receives the original provider.
🤖 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 `@python/scenario/_tracing/setup.py`:
- Line 95: Complete the explicit Python annotations: annotate the provider
parameter in _get_concrete_provider, parameterize the Sequence return or
parameter annotation with the instrumentor type in setup.py, and annotate both
span_processor parameters in the affected test functions. Apply the changes at
python/scenario/_tracing/setup.py lines 95 and 140-145, and
python/tests/test_tracing_setup.py lines 98-99 and 146-147.
- Around line 104-109: Require providers to expose both get_tracer and
add_span_processor before treating them as _SpanProcessorProvider instances:
update _SpanProcessorProvider, _get_concrete_provider, and all provider test
doubles in python/scenario/_tracing/setup.py:28-31,
python/scenario/_tracing/setup.py:104-109,
python/tests/test_tracing_setup.py:96-106, and
python/tests/test_tracing_setup.py:144-157. Ensure only providers satisfying the
full contract are passed to instrumentation.
---
Nitpick comments:
In `@python/tests/test_tracing_setup.py`:
- Around line 144-157: Update test_instruments_an_existing_public_provider to
provide a spy processor/exporter through the _do_setup configuration, then
assert the existing provider’s add_span_processor method receives it and
trace.set_tracer_provider is not called. Preserve the existing assertion that
the instrumentor receives the original provider.
🪄 Autofix (Beta)
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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: fb3eee0e-16dd-4d31-9275-aa9c610b4b7b
📒 Files selected for processing (2)
python/scenario/_tracing/setup.pypython/tests/test_tracing_setup.py
|
|
||
|
|
||
| def _get_concrete_provider(provider) -> Optional[TracerProvider]: | ||
| def _get_concrete_provider(provider) -> Optional[_SpanProcessorProvider]: |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Complete the new Python annotations.
python/scenario/_tracing/setup.py#L95-L95: annotateprovider.python/scenario/_tracing/setup.py#L140-L145: parameterizeSequencewith the instrumentor type.python/tests/test_tracing_setup.py#L98-L99: annotate the firstspan_processorparameter.python/tests/test_tracing_setup.py#L146-L147: annotate the secondspan_processorparameter.
As per coding guidelines, Python functions under python/**/*.py must use explicit, specific type annotations.
📍 Affects 2 files
python/scenario/_tracing/setup.py#L95-L95(this comment)python/scenario/_tracing/setup.py#L140-L145python/tests/test_tracing_setup.py#L98-L99python/tests/test_tracing_setup.py#L146-L147
🤖 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 `@python/scenario/_tracing/setup.py` at line 95, Complete the explicit Python
annotations: annotate the provider parameter in _get_concrete_provider,
parameterize the Sequence return or parameter annotation with the instrumentor
type in setup.py, and annotate both span_processor parameters in the affected
test functions. Apply the changes at python/scenario/_tracing/setup.py lines 95
and 140-145, and python/tests/test_tracing_setup.py lines 98-99 and 146-147.
Source: Coding guidelines
| # OpenTelemetry's public provider interface does not require an SDK | ||
| # TracerProvider subclass. Providers such as Temporal's replay-safe wrapper | ||
| # expose the processor hook directly and are safe to configure in place. | ||
| if callable(getattr(provider, "add_span_processor", None)): | ||
| return cast(_SpanProcessorProvider, provider) | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate files =="
git ls-files | rg 'python/scenario/_tracing/setup\.py|python/tests/test_tracing_setup\.py|pyproject\.toml|requirements|poetry.lock|uv.lock' || true
echo "== setup relevant lines =="
wc -l python/scenario/_tracing/setup.py python/tests/test_tracing_setup.py
sed -n '1,170p' python/scenario/_tracing/setup.py
sed -n '80,170p' python/tests/test_tracing_setup.py
echo "== package opentelemetry deps =="
rg -n "opentelemetry|otel" python pyproject.toml Pipfile requirements.txt uv.lock poetry.lock 2>/dev/null || true
echo "== source references for get_tracers/add_span_processor =="
rg -n "get_tracer|_get_concrete_provider|add_span_processor|_do_setup|tracer_provider" python/scenario/_tracing/setup.py python/tests/test_tracing_setup.pyRepository: langwatch/scenario
Length of output: 39749
🌐 Web query:
OpenTelemetry Python TracerProvider defines get_tracer tracer_provider argument instrumentation source
💡 Result:
In the OpenTelemetry Python SDK, the TracerProvider.get_tracer method is used to acquire a Tracer instance [1][2]. The argument often referred to as the "instrumentation source" is the instrumenting_module_name [1][3][2]. The get_tracer method signature is as follows [1][4]: get_tracer(instrumenting_module_name, instrumenting_library_version=None, schema_url=None, attributes=None) Key details regarding this argument include: 1. Identification: The instrumenting_module_name acts as a unique identifier for the instrumentation scope [1][2]. It should represent the library, package, module, or class that is performing the instrumentation [1][3]. 2. Naming Convention: It is explicitly recommended to use a fixed string—such as the full name of the instrumentation package—rather than using the dynamic name variable, as name can vary depending on which file the tracer is initialized in [1][4][2]. 3. Instrumentation vs. Instrumented: A critical distinction is that this name should identify the code doing the instrumentation, not the library being instrumented [1][2][5]. For example, when instrumenting the "requests" library, one should use "opentelemetry.instrumentation.requests" rather than just "requests" [1][3][2]. 4. Convenience Wrapper: The opentelemetry.trace.get_tracer function serves as a convenience wrapper for the TracerProvider.get_tracer method [1][2]. If a TracerProvider is not explicitly provided, it defaults to the currently configured global TracerProvider [1][4][2]. These identifiers (name and version) are crucial for telemetry processing, allowing users to understand the source of the data, filter signals, and debug instrumentation issues [6][5][7].
Citations:
- 1: https://opentelemetry-python.readthedocs.io/en/stable/api/trace.html
- 2: https://github.com/open-telemetry/opentelemetry-python/blob/main/opentelemetry-api/src/opentelemetry/trace/__init__.py
- 3: https://opentelemetry-python.readthedocs.io/en/latest/_modules/opentelemetry/trace.html
- 4: https://opentelemetry-python.readthedocs.io/en/latest/api/trace.html
- 5: https://github.com/open-telemetry/oteps/blob/main/text/0016-named-tracers.md
- 6: https://opentelemetry.io/docs/concepts/instrumentation/libraries/
- 7: https://github.com/open-telemetry/opentelemetry-specification/blob/8bbac83c/oteps/0016-named-tracers.md
🌐 Web query:
opentelemetry-python instrument.py inject tracer_provider get_tracer provider.tracer_provider
💡 Result:
In OpenTelemetry Python, the tracer_provider and the get_tracer function are core components used to manage and create tracers for instrumentation [1][2][3]. TracerProvider The TracerProvider acts as the factory for tracers [2][4]. You typically initialize a TracerProvider, configure it with span processors and exporters, and then set it as the global default provider [1][3]. Global Configuration To set the global TracerProvider, you use: from opentelemetry import trace from opentelemetry.sdk.trace import TracerProvider provider = TracerProvider trace.set_tracer_provider(provider) This allows other parts of your application to access the same provider via trace.get_tracer_provider [5][2]. get_tracer The get_tracer function is a convenience wrapper provided by the opentelemetry.trace API to create a tracer [5][2]. Its signature is: get_tracer(instrumenting_module_name, instrumenting_library_version=None, tracer_provider=None, schema_url=None, attributes=None) Usage Patterns: 1. Using the Global Provider (Default): If you omit the tracer_provider argument, the function automatically uses the currently configured global tracer provider [5][2]. tracer = trace.get_tracer("my.instrumentation.name") 2. Providing a Specific Provider: If you need to use a specific, non-global provider (e.g., in advanced scenarios requiring different resource configurations), you can pass it explicitly [6]: tracer = trace.get_tracer("my.instrumentation.name", tracer_provider=my_custom_provider) Regarding your query about instrument.py, note that automatic instrumentation tools like the opentelemetry-instrument CLI automatically handle the configuration of the global tracer provider based on environment variables [7]. Manual instrumentation requires explicitly initializing these components in your code as shown above [8][3].
Citations:
- 1: https://opentelemetry.io/docs/languages/python/instrumentation
- 2: https://opentelemetry-python.readthedocs.io/en/latest/api/trace.html
- 3: https://opentelemetry.io/docs/languages/python/instrumentation/
- 4: https://opentelemetry-python.readthedocs.io/en/stable/api/trace.html?highlight=TracerProvider
- 5: https://github.com/open-telemetry/opentelemetry-python/blob/main/opentelemetry-api/src/opentelemetry/trace/__init__.py
- 6: https://opentelemetry.io/docs/languages/python/cookbook/
- 7: https://github.com/open-telemetry/opentelemetry-python-contrib/blob/a4285851/opentelemetry-instrumentation/README.rst
- 8: https://opentelemetry.io/docs/zero-code/python/example/
🌐 Web query:
site:github.com/open-telemetry/opentelemetry-python-contrib tracer_provider.tracer_provider get_tracer tracer_provider
💡 Result:
In OpenTelemetry Python, the expression tracer_provider.get_tracer refers to the method used to retrieve a tracer instance from a specific TracerProvider object [1]. While users often access the global tracer provider via trace.get_tracer_provider [2][3], libraries and instrumentations frequently accept an optional tracer_provider argument [4][5]. When this argument is provided, the instrumentation uses that specific TracerProvider instance to create its tracer via the get_tracer method, falling back to the global provider only if one is not explicitly provided [4][5][6]. The standard signature for this method is typically: tracer = tracer_provider.get_tracer( instrumenting_module_name, instrumenting_library_version=None, schema_url=None) This mechanism ensures that instrumented code can be configured with a specific provider (which may have unique resource attributes or span processors) rather than relying solely on the global state [4][5].
Citations:
- 1: https://github.com/open-telemetry/opentelemetry-python-contrib/blob/a4285851/instrumentation/opentelemetry-instrumentation-logging/tests/test_logging.py
- 2: Requests instrumentation http semantic convention migration open-telemetry/opentelemetry-python-contrib#2002
- 3: [redis] Unable to initiate tracer open-telemetry/opentelemetry-python-contrib#405
- 4: Update instrumentations to use tracer_provider for creating tracer if given, otherwise use global tracer provider open-telemetry/opentelemetry-python-contrib#402
- 5: Instrumentations do not use
tracer_providerargument to create a tracer instances open-telemetry/opentelemetry-python-contrib#401 - 6: https://github.com/open-telemetry/opentelemetry-python-contrib/blob/main/instrumentation/opentelemetry-instrumentation-httpx/src/opentelemetry/instrumentation/httpx/__init__.py
🌐 Web query:
opentelemetry-instrumentation-requests get_tracer tracer_provider Instrumentor._instrument tracer_provider
💡 Result:
In the OpenTelemetry Python instrumentation for the requests library, the components you mentioned—get_tracer, tracer_provider, and Instrumentor._instrument—are part of the internal implementation used to set up and manage telemetry collection [1][2]. 1. tracer_provider: This is an optional argument passed to the instrumentation methods [3][4]. It allows you to specify a custom TracerProvider if you do not wish to use the global default provider [3][4]. 2. get_tracer: Inside the instrumentation package, this function is called to retrieve a tracer instance [1][2]. It typically uses the provided tracer_provider (or the global default) along with library-specific information (like name and version) to initialize a tracer that records spans for outgoing HTTP requests [1][2]. 3. Instrumentor._instrument: The RequestsInstrumentor class inherits from BaseInstrumentor [3][4]. The _instrument method is an internal implementation method that you generally do not call directly [5]. When you call.instrument on the RequestsInstrumentor instance, it internally calls this _instrument method, passing along configuration options like the tracer_provider [4][5]. This method then proceeds to perform the actual monkey-patching of the requests library to enable tracing [4][6]. In typical usage, you do not need to interact with these internal methods. Instead, you use the public API: from opentelemetry.instrumentation.requests import RequestsInstrumentor # Automatically instruments 'requests' using global providers RequestsInstrumentor.instrument # Or, optionally pass a custom TracerProvider RequestsInstrumentor.instrument(tracer_provider=my_custom_tracer_provider)
Citations:
- 1: https://github.com/open-telemetry/opentelemetry-python-contrib/blob/main/instrumentation/opentelemetry-instrumentation-requests/src/opentelemetry/instrumentation/requests/__init__.py
- 2: https://opentelemetry-python-contrib.readthedocs.io/en/latest/%5Fmodules/opentelemetry/instrumentation/requests.html
- 3: https://opentelemetry-python-contrib.readthedocs.io/en/latest/instrumentation/requests/requests.html
- 4: https://opentelemetry-python-kinvolk.readthedocs.io/en/latest/_modules/opentelemetry/instrumentation/requests.html
- 5: https://github.com/open-telemetry/opentelemetry-python-contrib/blob/main/opentelemetry-instrumentation/src/opentelemetry/instrumentation/instrumentor.py
- 6: https://github.com/open-telemetry/opentelemetry-python-contrib/blob/main/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py
🌐 Web query:
github.com/open-telemetry/opentelemetry-instrumentation BaseInstrumentor tracer_provider get_tracer tracer_provider
💡 Result:
In OpenTelemetry Python instrumentation, the BaseInstrumentor class serves as an abstract base class for creating instrumentation libraries [1][2]. While BaseInstrumentor itself provides methods for instrumenting and uninstrumenting libraries [2], the acquisition of a tracer is handled by the OpenTelemetry API's tracer provider mechanisms [3][4]. To acquire a tracer within an instrumentation, you use the TracerProvider's get_tracer method [3][4]. The recommended approach involves the following steps: 1. Tracer Acquisition: Use opentelemetry.trace.get_tracer. This is a convenience wrapper that delegates to the configured TracerProvider [3][4]. 2. Tracer Provider Handling: While you can call trace.get_tracer directly (which defaults to the global tracer provider), many custom instrumentors are designed to accept an optional tracer_provider as a keyword argument (e.g., in their init or instrument method) [5][6]. If a tracer_provider is provided, it should be used to create the tracer; otherwise, the global tracer provider is used [3][7][4]. 3. Scope Identification: When calling get_tracer, you must provide a unique identifier for the instrumentation scope (instrumenting_module_name), typically the name of the package or module performing the instrumentation [3][4]. It is best practice to use a fixed string rather than name, as the latter can result in inconsistent tracer names if the instrumentation spans multiple files [4][8]. Example usage pattern: from opentelemetry import trace # Inside your instrumentor tracer = trace.get_tracer( instrumenting_module_name="my.instrumentation.package", instrumenting_library_version="1.0.0", tracer_provider=self.tracer_provider # If optional provider was passed to the instrumentor) This pattern ensures that your instrumentation can be easily integrated into different environments, including those that require custom tracer providers rather than the global default [7][5][6].
Citations:
- 1: https://opentelemetry-python-contrib.readthedocs.io/en/latest/instrumentation/base/instrumentor.html
- 2: https://github.com/open-telemetry/opentelemetry-python-contrib/blob/main/opentelemetry-instrumentation/src/opentelemetry/instrumentation/instrumentor.py
- 3: https://github.com/open-telemetry/opentelemetry-python/blob/main/opentelemetry-api/src/opentelemetry/trace/__init__.py
- 4: https://opentelemetry-python.readthedocs.io/en/stable/api/trace.html?highlight=TracerProvider
- 5: https://opentelemetry-python-contrib.readthedocs.io/en/latest/instrumentation/botocore/botocore.html
- 6: https://opentelemetry-python-contrib.readthedocs.io/en/latest/instrumentation/logging/logging.html
- 7: Update instrumentations to use tracer_provider for creating tracer if given, otherwise use global tracer provider open-telemetry/opentelemetry-python-contrib#402
- 8: Python docs incorrectly use
__name__forinstrumenting_module_nameopen-telemetry/opentelemetry.io#2517
🌐 Web query:
site:github.com/open-telemetry/opentelemetry-python-contrib "tracer_provider.tracer_provider" "tracer_provider.get_tracer" "instrumentor.py"
💡 Result:
In the context of the OpenTelemetry Python instrumentation libraries, tracer_provider and get_tracer are used to manage how spans are created and reported [1][2]. The standard pattern for OpenTelemetry instrumentations involves using a tracer provider to create tracer instances, which are then used to instrument library code [1][2][3]. Key aspects of this pattern include: 1. Tracer Provider Usage: Instrumentations are designed to accept an optional tracer_provider argument [1][2][3]. If a tracer provider is provided, the instrumentation uses it to create its tracer; if it is not provided, the instrumentation falls back to the global tracer provider [1][2][3]. This is important because different tracer providers can have different configurations, such as distinct Resource attributes [1][2]. 2. Implementation in instrumentor.py: The instrumentor.py file, located in the opentelemetry-instrumentation package, defines the BaseInstrumentor class [4]. While the base class provides the structure for instrumentation lifecycle management (such as instrument and uninstrument methods) [5][4], the actual tracer initialization typically occurs within the specific instrumentation packages (e.g., opentelemetry-instrumentation-fastapi or opentelemetry-instrumentation-aws-lambda) [6][3]. 3. Creating the Tracer: The get_tracer function from the opentelemetry.trace API is used to instantiate the tracer [3]. It is commonly called as follows: tracer = get_tracer(name, version, tracer_provider=tracer_provider, schema_url=...) [3] This ensures that the instrumentation is properly associated with the correct tracer provider and semantic convention schema [3].
Citations:
- 1: Update instrumentations to use tracer_provider for creating tracer if given, otherwise use global tracer provider open-telemetry/opentelemetry-python-contrib#402
- 2: Instrumentations do not use
tracer_providerargument to create a tracer instances open-telemetry/opentelemetry-python-contrib#401 - 3: https://github.com/open-telemetry/opentelemetry-python-contrib/blob/a4285851/instrumentation/opentelemetry-instrumentation-aws-lambda/src/opentelemetry/instrumentation/aws_lambda/__init__.py
- 4: https://github.com/open-telemetry/opentelemetry-python-contrib/blob/a4285851/opentelemetry-instrumentation/src/opentelemetry/instrumentation/instrumentor.py
- 5: https://github.com/open-telemetry/opentelemetry-python-contrib/blob/main/opentelemetry-instrumentation/src/opentelemetry/instrumentation/instrumentor.py
- 6: https://github.com/open-telemetry/opentelemetry-python-contrib/blob/main/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py
🌐 Web query:
github.com/open-telemetry/opentelemetry-python-contrib/instrumentation/opentelemetry-instrumentation-requests/src/opentelemetry/instrumentation/requests/__init__.py tracer_provider
💡 Result:
In the OpenTelemetry Python Requests instrumentation library, the tracer_provider is an optional keyword argument that can be passed to the instrument method of the RequestsInstrumentor class [1][2]. When invoking RequestsInstrumentor.instrument, you can provide a custom TracerProvider instance to control where spans are sent [1][3]. If no tracer_provider is specified, the instrumentation defaults to using the global TracerProvider (configured via trace.set_tracer_provider) [4][2]. Example usage: from opentelemetry.instrumentation.requests import RequestsInstrumentor from opentelemetry.sdk.trace import TracerProvider # Create a custom tracer provider provider = TracerProvider # Pass the provider to the instrumentor RequestsInstrumentor.instrument(tracer_provider=provider) The tracer_provider argument allows developers to use specific, isolated tracing configurations for their requests, rather than relying on the globally registered provider [4][2]. Internal implementations of the instrumentation use this provider when initializing the tracer via get_tracer [5][2].
Citations:
- 1: https://opentelemetry-python-contrib.readthedocs.io/en/latest/instrumentation/requests/requests.html
- 2: https://github.com/open-telemetry/opentelemetry-python-contrib/blob/v0.52b1/instrumentation/opentelemetry-instrumentation-requests/src/opentelemetry/instrumentation/requests/__init__.py
- 3: https://opentelemetry-python-kinvolk.readthedocs.io/en/latest/instrumentation/requests/requests.html
- 4: https://oneuptime.com/blog/post/2026-02-06-trace-python-requests-library-opentelemetry/view
- 5: https://github.com/open-telemetry/opentelemetry-python-contrib/blob/main/instrumentation/opentelemetry-instrumentation-requests/src/opentelemetry/instrumentation/requests/__init__.py
Require the full tracer-provider contract before instrumenting.
instrumentor.instrument(tracer_provider=provider) may pass this object into OpenTelemetry instrumentation code, where it is expected to support get_tracer(...); only add_span_processor leaves the contract incomplete. Require both methods in _SpanProcessorProvider, _get_concrete_provider, and the provider test doubles.
📍 Affects 2 files
python/scenario/_tracing/setup.py#L104-L109(this comment)python/scenario/_tracing/setup.py#L28-L31python/tests/test_tracing_setup.py#L96-L106python/tests/test_tracing_setup.py#L144-L157
🤖 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 `@python/scenario/_tracing/setup.py` around lines 104 - 109, Require providers
to expose both get_tracer and add_span_processor before treating them as
_SpanProcessorProvider instances: update _SpanProcessorProvider,
_get_concrete_provider, and all provider test doubles in
python/scenario/_tracing/setup.py:28-31,
python/scenario/_tracing/setup.py:104-109,
python/tests/test_tracing_setup.py:96-106, and
python/tests/test_tracing_setup.py:144-157. Ensure only providers satisfying the
full contract are passed to instrumentation.
langwatch-agent
left a comment
langwatch-agent
left a comment
There was a problem hiding this comment.
Static review found no actionable correctness, security, or tracing-ownership issue in the current fork change. The public-provider capability check remains restricted to attaching a span processor and instrumentors receive the selected provider. I did not execute the untrusted head or its scripts.
langwatch-agent
left a comment
langwatch-agent
left a comment
There was a problem hiding this comment.
Static review of external contribution at 5ca69c7dd9c445e87b6d4ab299320f11c7aad3bc (no PR-branch code or dependencies executed). No merge-blocking issue found. The structural provider check accepts public tracer providers only when they expose the needed processor-registration method, preserves the no-provider path, and passes the selected provider through to available instrumentors. CI evidence was considered but not rerun under the external-contribution trust boundary.
langwatch-agent
added
hound-checked
langwatch-agent
left a comment
langwatch-agent
left a comment
There was a problem hiding this comment.
Clean static review. The duck-typed provider boundary is constrained to the processor operation used here, and the added instrumentation path is covered. Residual risk: the fork is stale and has no current full CI result.\n\nLangWatch-Review: verdict=clean sha=5ca69c7dd9c445e87b6d4ab299320f11c7aad3bc p0=0 p1=0 p2=0 p3=0
langwatch-agent
left a comment
langwatch-agent
left a comment
There was a problem hiding this comment.
External static review: no blocking concern found in the current diff. I did not execute branch code, install dependencies, or run contributor-provided scripts. Residual risk: runtime behavior remains covered by the repository CI.
LangWatch-Review: verdict=clean sha=5ca69c7dd9c445e87b6d4ab299320f11c7aad3bc p0=0 p1=0 p2=0 p3=0
Human Review BriefCaution This attaches Scenario's exporter to a provider Scenario does not own and cannot flush. The new The motivating case makes this concrete rather than theoretical. Temporal's replay-safe provider is replay-safe precisely because it suppresses span emission during replay. Attaching to it is correct, and it means a scenario run inside a replay produces no spans, silently and by design. Someone will hit that and read it as Scenario being broken. The other thing to check is ordering. The duck-type check runs before proxy unwrapping, and an SDK
What is being fixedScenario needs somewhere to send its spans. Previously it recognised only the OpenTelemetry SDK But OpenTelemetry's public provider API does not require an SDK subclass. Other implementations expose That is the right instinct: attaching to what is there beats replacing what someone else configured. Worth asking the authorWho flushes at the end of a run? Scenario's report is only useful if the spans arrive. With an SDK provider there is a known shutdown path. With a foreign provider there is not, and a run that ends by exiting the process may lose everything buffered. If Scenario keeps a handle on its own Does an instrumentor accept a non-SDK provider? Should the SDK check come first? Putting the Is the Temporal replay case documented anywhere? No spans during replay is correct behaviour that will read as a bug. One line in the docs saves someone a day. Note Attaching to an existing provider is the right move; owning nothing about its lifecycle is the cost. The review question is what guarantees the spans get out. |
3 participants
Scenario currently recognizes only the SDK
TracerProvider(or one delegated SDK provider). OpenTelemetry’s public provider API allows other implementations to exposeadd_span_processor; Temporal 1.30 uses that shape for its replay-safe provider. Detect the public processor hook before proxy unwrapping so Scenario attaches its collector/exporters to the existing provider instead of attempting to replace the global provider. Includes a focused regression test.