feat(otel): Phase 4 - Reactive execution instrumentation #2149
+916
−146
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Instrument individual reactive computations (calcs, effects, outputs)
with OpenTelemetry spans at REACTIVITY collection level.
**New Features:**
- Label generation for reactive spans with function names
- Source code attribute extraction (file, line number, function name)
- Calc execution spans with descriptive labels
- Effect execution spans with descriptive labels
- Output rendering spans with output names
- Source attributes included in all reactive spans
**Implementation:**
1. **Label Generation** (`shiny/otel/_labels.py`):
- `generate_reactive_label()` creates descriptive span names
- Handles function names, lambdas, namespaces, and modifiers
- Examples: "reactive my_calc", "observe my_effect", "output greeting"
2. **Source Reference Extraction** (`shiny/otel/_attributes.py`):
- `extract_source_ref()` extracts code location attributes
- Returns `code.filepath`, `code.lineno`, `code.function`
- Gracefully handles built-ins and dynamically generated functions
3. **Calc Instrumentation** (`shiny/reactive/_reactives.py`):
- Extract OTel attributes at Calc initialization
- Wrap `Calc_.update_value()` with OTel span when REACTIVITY level enabled
- Span includes generated label and source attributes
4. **Effect Instrumentation** (`shiny/reactive/_reactives.py`):
- Extract OTel attributes at Effect initialization
- Wrap `Effect_._run()` with OTel span when REACTIVITY level enabled
- Span includes generated label and source attributes
5. **Output Instrumentation** (`shiny/session/_session.py`):
- Extract source attributes from renderer function at render time
- Wrap `renderer.render()` call with OTel span when REACTIVITY level enabled
- Span name includes output ID (e.g., "output greeting")
**Tests** (`tests/pytest/test_otel_reactive_execution.py`):
- Label generation tests for all reactive types
- Source reference extraction tests
- Calc span creation and collection level tests
- Effect span creation and collection level tests
- Output span placeholder test
- Span hierarchy tests (reactive.update → calc/effect spans)
**Span Hierarchy:**
```
session.start
└── reactive.update
├── reactive my_calc
├── observe my_effect
└── output greeting
```
**Acceptance Criteria:**
- ✅ Reactive computations create spans when REACTIVITY level enabled
- ✅ Spans have descriptive labels (function names, not generic)
- ✅ Source location attributes included when available
- ✅ Async context propagation works (child spans nest properly)
- ✅ Collection level controls span creation
- ✅ All 71 OTel tests pass
🤖 Generated with [Claude Code](https://claude.com/claude-code)
fix(otel): Fix flake8 and type annotation issues in Phase 4
- Remove unused imports (Mock, ReactiveEnvironment)
- Add proper type annotations to Callable parameters
- Use Callable[..., Any] for generic function types
- Update return type hints to dict[str, Any]
Co-Authored-By: Claude <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR implements Phase 4 of the OpenTelemetry integration, adding instrumentation for individual reactive computations (calcs, effects, and outputs) with spans at the REACTIVITY collection level.
Changes:
- Adds label generation utility for creating descriptive span names from function names, handling lambdas, namespaces, and modifiers
- Implements source code reference extraction using Python's inspect module for code location attributes
- Instruments Calc_, Effect_, and Output rendering with OTel spans when REACTIVITY level is enabled
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
shiny/otel/_labels.py |
New module for generating descriptive reactive span labels with support for namespaces and modifiers |
shiny/otel/_attributes.py |
Adds extract_source_ref function to extract code location attributes from functions |
shiny/reactive/_reactives.py |
Instruments Calc_ and Effect_ classes with OTel spans, extracting attributes at init time |
shiny/session/_session.py |
Instruments output rendering with OTel spans when REACTIVITY level is enabled |
tests/pytest/test_otel_reactive_execution.py |
Comprehensive test suite with 16 tests covering label generation, source extraction, and span creation |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Improves error handling by using separate try/catch blocks for each source attribute extraction operation. This allows maximum attribute collection even if some operations fail (e.g., if getsourcefile fails, we can still get line numbers and function name).
…_update - Extract calc_parent to separate variable for type narrowing - Add type: ignore comments for OpenTelemetry SDK context attributes - Pyright doesn't understand that ReadableSpan.context is always present
- Add callable support for name parameter in with_otel_span_async() - Simplify Calc, Effect, and Output instrumentation using lazy name generation - Move OTel imports to top level in _reactives.py - Fix namespace documentation in generate_reactive_label() - Use pytest.skip for placeholder output test - Update tests to patch at correct import location
The test was failing in CI due to TracerProvider conflicts when tests run in parallel. The OpenTelemetry SDK does not allow overriding an already-set TracerProvider. Solution: Use try/finally block with trace._set_tracer_provider() internal API to properly set and restore the TracerProvider for test isolation. Fixes test_otel_reactive_flush.py::test_span_parent_child_relationship failure in CI.
Refactor the TracerProvider test setup in the span hierarchy test to use the `otel_tracer_provider_context()` context manager. This provides a reusable pattern for test isolation when setting up tracing infrastructure. Benefits: - DRY principle: Eliminates duplicate setup/teardown code - Better encapsulation: All TracerProvider logic in one place - Easier to maintain: Changes only need to be made in one location - Consistent pattern: Can be reused in other tests that need tracer setup The context manager: - Saves current TracerProvider - Sets up new provider with InMemorySpanExporter - Uses internal API to force override for testing - Restores original provider in finally block 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Refactored all OTel test files to follow DRY principle by moving repeatedly imported modules from individual test methods to the top-level imports. This improves code readability and reduces duplication. Changes: - test_otel_foundation.py: Added shiny.otel._core, with_otel_span, with_otel_span_async to top-level; removed 7 inline imports - test_otel_reactive_execution.py: Added extract_source_ref, generate_reactive_label, with_otel_span_async, Calc_, Effect_ to top-level; removed 15+ inline imports - test_otel_reactive_flush.py: Added should_otel_collect, with_otel_span_async, ReactiveEnvironment to top-level; removed 9 inline imports - test_otel_session.py: Added should_otel_collect, extract_http_attributes to top-level; removed 9 inline imports Benefits: - DRY: Eliminated ~40 lines of repeated import statements - Cleaner: Tests focus on logic, not setup - Standard: Follows common Python testing patterns - Maintainable: Change imports once, not in multiple places - Faster: Imports happen once at module load All 70 tests pass (69 passed, 1 skipped). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Added a public helper function `reset_tracing_state()` to provide a clean API for resetting the cached tracing state in tests, replacing direct manipulation of the internal `_tracing_enabled` variable. Changes: - shiny/otel/_core.py: Added `reset_tracing_state(tracing_enabled=None)` function with comprehensive documentation - shiny/otel/__init__.py: Exported `reset_tracing_state` as public API - tests/pytest/test_otel_foundation.py: Updated tests to use the new helper function instead of direct variable assignment Benefits: - Cleaner API: Provides a documented, official way to reset state - Encapsulation: Hides internal implementation details - Flexibility: Supports both reset-to-force-reevaluation (None) and explicit value setting (True/False) - Maintainability: Changes to internal state management only need to update one function - Test isolation: Explicit function makes test intent clearer The function accepts an optional parameter: - None (default): Reset state to force re-evaluation on next check - True/False: Explicitly set the tracing enabled state for testing All 70 tests pass (69 passed, 1 skipped). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
The reset_tracing_state() helper is intended for test isolation only and should not be part of the public API. Tests can still import it directly from shiny.otel._core when needed. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Since reset_tracing_state() is now internal-only (not in public API), tests need to import it directly from shiny.otel._core instead of shiny.otel. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
…king
Created a context manager helper patch_tracing_state() to replace
direct patching of internal _tracing_enabled variable. This provides:
- Shorter, more readable test code
- Better encapsulation of internal implementation
- Automatic state cleanup via context manager
- Consistent pattern across all test files
Updated all 22 occurrences across test files:
- test_otel_foundation.py (2 occurrences)
- test_otel_session.py (5 occurrences)
- test_otel_reactive_flush.py (8 occurrences)
- test_otel_reactive_execution.py (7 occurrences)
Before: with patch("shiny.otel._core._tracing_enabled", True):
After: with patch_tracing_state(True):
Note: Like reset_tracing_state(), this helper is internal-only
(not exported in public API) and only for test isolation.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
Changed reset_tracing_state() and patch_tracing_state() to use keyword-only arguments (added * before parameters). This prevents accidental positional argument usage and makes the API more explicit. Before: - reset_tracing_state(False) - patch_tracing_state(True) After: - reset_tracing_state(tracing_enabled=False) - patch_tracing_state(tracing_enabled=True) Updated all 22 test call sites across all OTel test files. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
The test_span_parent_child_relationship test was failing on Linux/Windows because it sets up a new TracerProvider but the cached _tracing_enabled state wasn't being reset, causing the test to think tracing was disabled. Added reset_tracing_state() call after setting up the TracerProvider to force re-evaluation of the tracing state with the new provider. This is the same pattern used in test_otel_reactive_execution.py for the calc span hierarchy test. Fixes CI failures on Ubuntu and Windows platforms. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+67
to
+118
| class TestLabelGeneration: | ||
| """Label generation tests""" | ||
|
|
||
| def test_generate_reactive_label_for_calc(self): | ||
| """Test generating label for a calc with function name""" | ||
|
|
||
| def my_calc(): | ||
| return 42 | ||
|
|
||
| label = generate_reactive_label(my_calc, "reactive") | ||
| assert label == "reactive my_calc" | ||
|
|
||
| def test_generate_reactive_label_for_lambda(self): | ||
| """Test generating label for anonymous/lambda function""" | ||
| label = generate_reactive_label(lambda: 42, "reactive") | ||
| assert label == "reactive <anonymous>" | ||
|
|
||
| def test_generate_reactive_label_with_namespace(self): | ||
| """Test generating label with namespace prefix""" | ||
|
|
||
| def my_calc(): | ||
| return 42 | ||
|
|
||
| label = generate_reactive_label(my_calc, "reactive", namespace="mod") | ||
| assert label == "reactive mod:my_calc" | ||
|
|
||
| def test_generate_reactive_label_with_modifier(self): | ||
| """Test generating label with modifier (e.g., cache)""" | ||
|
|
||
| def my_calc(): | ||
| return 42 | ||
|
|
||
| label = generate_reactive_label(my_calc, "reactive", modifier="cache") | ||
| assert label == "reactive cache my_calc" | ||
|
|
||
| def test_generate_observe_label(self): | ||
| """Test generating label for effect (observe)""" | ||
|
|
||
| def my_effect(): | ||
| pass | ||
|
|
||
| label = generate_reactive_label(my_effect, "observe") | ||
| assert label == "observe my_effect" | ||
|
|
||
| def test_generate_output_label(self): | ||
| """Test generating label for output rendering""" | ||
|
|
||
| def my_output(): | ||
| return "text" | ||
|
|
||
| label = generate_reactive_label(my_output, "output") | ||
| assert label == "output my_output" |
There was a problem hiding this comment.
The label generation tests cover namespace and modifier separately but do not test the combination of both parameters together. The docstring example shows "reactive cache mod:my_calc" as a valid output, but there's no test verifying this behavior works correctly.
Add a test case that verifies the combined namespace and modifier functionality to ensure the label parts are assembled in the correct order.
Comment on lines
248
to
306
| class TestEffectSpans: | ||
| """Effect execution span tests""" | ||
|
|
||
| @pytest.mark.asyncio | ||
| async def test_effect_creates_span_when_enabled(self): | ||
| """Test that Effect execution creates a span when collection level is REACTIVITY""" | ||
| with patch_tracing_state(tracing_enabled=True): | ||
| with patch.dict(os.environ, {"SHINY_OTEL_COLLECT": "reactivity"}): | ||
|
|
||
| # Create an effect | ||
| def my_effect(): | ||
| pass | ||
|
|
||
| effect = Effect_(my_effect, session=None) | ||
|
|
||
| # Mock span wrapper to verify it's called | ||
| # Must patch at the import location in _reactives module | ||
| with patch( | ||
| "shiny.reactive._reactives.with_otel_span_async" | ||
| ) as mock_span: | ||
| # Configure mock to act as async context manager | ||
| mock_span.return_value.__aenter__ = AsyncMock(return_value=None) | ||
| mock_span.return_value.__aexit__ = AsyncMock(return_value=None) | ||
|
|
||
| # Execute the effect | ||
| await effect._run() | ||
|
|
||
| # Verify span was created | ||
| mock_span.assert_called_once() | ||
| call_args = mock_span.call_args | ||
| # Verify the name callable was passed | ||
| name_callable = call_args[0][0] | ||
| assert callable(name_callable) | ||
| assert name_callable() == "observe my_effect" | ||
| assert call_args[1]["level"] == OtelCollectLevel.REACTIVITY | ||
|
|
||
| @pytest.mark.asyncio | ||
| async def test_effect_no_span_when_disabled(self): | ||
| """Test that Effect execution doesn't create span when collection level is too low""" | ||
| with patch_tracing_state(tracing_enabled=True): | ||
| with patch.dict(os.environ, {"SHINY_OTEL_COLLECT": "session"}): | ||
|
|
||
| # Create an effect | ||
| def my_effect(): | ||
| pass | ||
|
|
||
| effect = Effect_(my_effect, session=None) | ||
|
|
||
| # Mock span wrapper to verify it's not called | ||
| # Must patch at the import location in _reactives module | ||
| with patch( | ||
| "shiny.reactive._reactives.with_otel_span_async" | ||
| ) as mock_span: | ||
| # Execute the effect | ||
| await effect._run() | ||
|
|
||
| # Verify span was not created (no-op context manager) | ||
| # The span wrapper is still called but returns a no-op | ||
| mock_span.assert_called_once() |
There was a problem hiding this comment.
The Effect span tests lack coverage for source code attributes, unlike the Calc tests which include test_calc_span_includes_source_attrs. Since both Calc_ and Effect_ extract OpenTelemetry attributes at initialization using the same _extract_otel_attrs method, Effect should have equivalent test coverage.
Add a test similar to test_calc_span_includes_source_attrs that verifies Effect spans include source code attributes (code.function, code.filepath, code.lineno) when available.
The test_span_parent_child_relationship test was failing on Linux/Windows because it sets up a new TracerProvider but the cached _tracing_enabled state wasn't being reset, causing the test to think tracing was disabled. Added reset_tracing_state() call after setting up the TracerProvider to force re-evaluation of the tracing state with the new provider. This is the same pattern used in test_otel_reactive_execution.py for the calc span hierarchy test. Fixes CI failures on Ubuntu and Windows platforms. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Moved the inline import of extract_source_ref from two _extract_otel_attrs methods to the top-level imports, following the pattern established for other OTel imports (generate_reactive_label, with_otel_span_async). This eliminates repeated import overhead and improves code organization. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
The reset_tracing_state() helper now accepts optional tracer and logger parameters and resets all cached OTel objects (_tracer, _logger, and _tracing_enabled). This is critical for test isolation when setting up a new TracerProvider - without resetting the cached tracer, the old tracer from a previous provider continues to be used. This fixes CI test failures on Ubuntu/Windows where tests that set up their own TracerProvider were getting 0 spans because the cached tracer was still pointing to the default no-op provider. Also moved OpenTelemetry imports to top-level in test_otel_reactive_execution.py for consistency with other test files.
- Renamed reset_tracing_state() → reset_otel_tracing_state() - Renamed patch_tracing_state() → patch_otel_tracing_state() - Removed parameters from reset_otel_tracing_state() for clearer API - Now always resets all cached state to None - If you need to set specific values, use patch_otel_tracing_state() - Updated all test files and documentation - Clear naming convention: 'otel' prefix for OTel-specific functions
The test_span_parent_child_relationship test was failing in CI (Ubuntu, Windows) because it used trace.set_tracer_provider(), which doesn't allow overriding when pytest-xdist has already set a global provider for parallel test execution. Root cause: OpenTelemetry's public set_tracer_provider() API logs a warning and refuses to override an existing provider, causing 0 spans to be exported in CI. Solution: Use the internal trace._set_tracer_provider(provider, log=False) API (same pattern as test_otel_reactive_execution.py's helper function), which allows overriding and doesn't log warnings. Also save/restore the old provider in a try/finally block to ensure proper cleanup. This matches the pattern in otel_tracer_provider_context() helper that was already working correctly in test_otel_reactive_execution.py. Fixes CI failures on all platforms (Ubuntu, Windows, macOS with pytest-xdist parallel execution).
…ve_flush Moved inline OpenTelemetry imports from test_span_parent_child_relationship to module top-level for consistency with other OTel test files.
Created test_otel_helpers.py module with otel_tracer_provider_context() to eliminate code duplication between test_otel_reactive_execution.py and test_otel_reactive_flush.py. Benefits: - Single source of truth for TracerProvider setup/teardown in tests - Consistent handling of pytest-xdist compatibility - Reduced code duplication (~60 lines removed across 2 files) - Easier to maintain and update test isolation logic The helper properly handles: - Saving and restoring the original TracerProvider - Using internal _set_tracer_provider API for CI compatibility - Resetting OTel tracing state before and after each test
Relocated reset_otel_tracing_state() and patch_otel_tracing_state() from shiny/otel/_core.py to tests/pytest/test_otel_helpers.py to make it clear these are test-only utilities, not part of the production API. Benefits: - Clear separation between production code and test utilities - Reduced public API surface in _core module - All test helpers now in one location - Easier to maintain and discover test utilities Updated all test files to import from test_otel_helpers instead of _core.
The file contains test utilities, not tests themselves, so it should not have the test_ prefix which pytest uses for test discovery. Updated all imports in test files to reference the new module name. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
The span hierarchy tests were occasionally failing in parallel execution (pytest-xdist) with 0 spans being exported. This was a race condition where the test would check for spans before the TracerProvider had finished exporting them to the InMemorySpanExporter. Solution: Created `get_exported_spans()` helper in otel_helpers.py that calls provider.force_flush() before retrieving spans. This ensures all spans are fully processed before assertions run. Tests affected: - test_otel_reactive_flush.py::test_span_parent_child_relationship - test_otel_reactive_execution.py::test_calc_span_nested_under_reactive_update Both tests now pass reliably in parallel execution (verified with 50 consecutive runs showing 100/100 passes). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Corrects attribute access from 'self._fn' to 'self.fn' in _renderer.py and updates '_orig_fn' lookup to use 'renderer.fn' in _session.py. Ensures proper function reference and metadata extraction.
When _orig_fn doesn't exist on renderer.fn, fall back to the renderer itself instead of renderer.fn (AsyncValueFn). This ensures extract_source_ref() receives a callable with source info rather than the wrapper object. Fixes CI failures on Ubuntu/Windows.
Collaborator
Author
|
@copilot The CI |
Contributor
|
@schloerke I've opened a new pull request, #2150, to work on those changes. Once the pull request is ready, I'll request review from you. |
6 tasks
…bility (#2150) Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: schloerke <93231+schloerke@users.noreply.github.com> Co-authored-by: Barret Schloerke <barret@posit.co>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Implements Phase 4 of the OpenTelemetry integration: Reactive Execution Instrumentation.
Instruments individual reactive computations (calcs, effects, outputs) with OpenTelemetry spans at the REACTIVITY collection level.
Closes #2135
Implementation
1. Label Generation (
shiny/otel/_labels.py)generate_reactive_label()function<anonymous>), namespaces, and modifiers"reactive my_calc","observe my_effect","output greeting"2. Source Reference Extraction (
shiny/otel/_attributes.py)extract_source_ref()code.filepath,code.lineno,code.functioninspectmodule for source introspection3. Span Wrapper Enhancement (
shiny/otel/_span_wrappers.py)nameparameternamecan now bestr | Callable[[], str]4. Calc Instrumentation (
shiny/reactive/_reactives.py)Calc_.__init__Calc_.update_value()execution with span when REACTIVITY level enabledlambda: generate_reactive_label(self._fn, "reactive")5. Effect Instrumentation (
shiny/reactive/_reactives.py)Effect_.__init__Effect_._run()execution with span when REACTIVITY level enabledlambda: generate_reactive_label(self._fn, "observe")6. Output Instrumentation (
shiny/session/_session.py)renderer.render()call with span when REACTIVITY level enabled7. Import Optimization
_reactives.py8. Test Helpers (
shiny/otel/_core.py)reset_otel_tracing_state()- Reset all cached OTel state_tracing_enabled,_tracer, and_loggerto Nonereset_otel_tracing_state()patch_otel_tracing_state(*, tracing_enabled)- Context manager for temporary patchingunittest.mock.patchon internal variableswith patch_otel_tracing_state(tracing_enabled=True):Tests (
tests/pytest/test_otel_*.py)test_otel_reactive_execution.py - 16 tests:
All test files updated:
_tracing_enabledmanipulation withreset_otel_tracing_state()patch()calls withpatch_otel_tracing_state()context managerSpan Hierarchy
Phase 4 completes the reactive execution span hierarchy:
Code Quality
Type Safety
# type: ignoresuppressions (except for known OTel SDK limitation)Code Reduction
Test Coverage