LCORE-1826: Expose OpenTelemetry configuration in v1/config endpoint - #2273
LCORE-1826: Expose OpenTelemetry configuration in v1/config endpoint#2273anik120 wants to merge 1 commit into
Conversation
WalkthroughThe configuration model now collects ChangesObservability configuration
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant Environment
participant ObservabilityConfiguration
participant Configuration
participant ConfigEndpoint
Environment->>ObservabilityConfiguration: provide OTEL_* variables
ObservabilityConfiguration->>Configuration: create observability.otel
Configuration->>ConfigEndpoint: expose configuration
ConfigEndpoint-->>Configuration: return observability.otel
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error)
✅ Passed checks (6 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@src/models/config.py`:
- Around line 2840-2863: Update ObservabilityConfiguration.from_environment to
prevent OTEL header variables from being exposed through
ConfigurationResponse.configuration: collect only a reviewed non-sensitive
OTEL_* allowlist or explicitly redact OTEL_EXPORTER_OTLP_HEADERS,
OTEL_EXPORTER_OTLP_TRACES_HEADERS, OTEL_EXPORTER_OTLP_METRICS_HEADERS, and
OTEL_EXPORTER_OTLP_LOGS_HEADERS before constructing the model, while preserving
safe OpenTelemetry configuration values.
In `@tests/integration/endpoints/test_config_integration.py`:
- Around line 125-167: Update
test_config_endpoint_observability_collects_otel_vars to set the OTEL
environment variables before rebuilding the app/config fixture, then invoke
config_endpoint_handler and assert the values through
response.configuration.observability.otel. Remove the direct
ObservabilityConfiguration.from_environment() assertions so the test validates
endpoint behavior and catches configuration default-factory regressions.
In `@tests/unit/models/config/test_observability_configuration.py`:
- Around line 16-17: Update the affected test docstrings in the observability
configuration tests to include a Parameters: section documenting each function
argument: monkeypatch, otel_dict, and expected_count. Apply this consistently to
all referenced tests, using the parameter names and their purposes without
changing test behavior.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: b75e3160-73fd-45a1-a556-25bfeae797cf
📒 Files selected for processing (4)
src/models/api/responses/successful/configuration.pysrc/models/config.pytests/integration/endpoints/test_config_integration.pytests/unit/models/config/test_observability_configuration.py
📜 Review details
⏰ Context from checks skipped due to timeout. (13)
- GitHub Check: bandit
- GitHub Check: Pylinter
- GitHub Check: integration_tests (3.12)
- GitHub Check: integration_tests (3.13)
- GitHub Check: build-pr
- GitHub Check: E2E: library mode / ci / group 1
- GitHub Check: E2E: server mode / ci / group 1
- GitHub Check: E2E: server mode / ci / group 2
- GitHub Check: E2E: library mode / ci / group 2
- GitHub Check: E2E: library mode / ci / group 3
- GitHub Check: E2E: server mode / ci / group 3
- GitHub Check: E2E Tests for Lightspeed Evaluation job
- GitHub Check: Konflux kflux-prd-rh02 / lightspeed-stack-0-7-on-pull-request
⚠️ CI failures not shown inline (2)
GitHub Actions: OpenAPI (Spectral) / spectral: LCORE-1826: Expose OpenTelemetry configuration in v1/config endpoint
Conclusion: failure
##[group]Run set -euo pipefail
�[36;1mset -euo pipefail�[0m
�[36;1muv run python scripts/generate_openapi_schema.py /tmp/openapi-generated.json�[0m
�[36;1mif ! diff -u docs/devel_doc/openapi.json /tmp/openapi-generated.json; then�[0m
�[36;1m echo "::error::docs/devel_doc/openapi.json is out of date. Regenerate with: uv run scripts/generate_openapi_schema.py docs/devel_doc/openapi.json"�[0m
GitHub Actions: OpenAPI (Spectral) / 0_spectral.txt: LCORE-1826: Expose OpenTelemetry configuration in v1/config endpoint
Conclusion: failure
##[group]Run set -euo pipefail
�[36;1mset -euo pipefail�[0m
�[36;1muv run python scripts/generate_openapi_schema.py /tmp/openapi-generated.json�[0m
�[36;1mif ! diff -u docs/devel_doc/openapi.json /tmp/openapi-generated.json; then�[0m
�[36;1m echo "::error::docs/devel_doc/openapi.json is out of date. Regenerate with: uv run scripts/generate_openapi_schema.py docs/devel_doc/openapi.json"�[0m
🧰 Additional context used
📓 Path-based instructions (5)
**/*
📄 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:
src/models/api/responses/successful/configuration.pytests/unit/models/config/test_observability_configuration.pysrc/models/config.pytests/integration/endpoints/test_config_integration.py
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Use absolute imports for internal Python modules.
All modules must begin with descriptive docstrings explaining their purpose.
Uselogger = get_logger(__name__)fromlog.pyfor module logging.
Use complete type annotations for function parameters and return types.
Use modern union syntax such asstr | int; useOptional[Type]for optional values.
Usetyping_extensions.Selffor model validators.
Functions must use descriptive, action-oriented snake_case names such asget_,validate_, andcheck_.
Avoid modifying mutable parameters in place; return a new data structure instead.
Useasync deffor I/O operations and external API calls.
HandleAPIConnectionErrorfrom Llama Stack.
All classes must have descriptive docstrings and complete type annotations for class attributes; use specific types instead ofAny.
Use PascalCase for classes and descriptive standard suffixes such asConfiguration,Error/Exception,Resolver, andInterface.
Use ABC and@abstractmethodfor abstract interfaces.
Follow Google Python docstring conventions; document all modules, classes, and functions, includingParameters,Returns,Raises, andAttributessections as applicable.
Never commit secrets or keys; use environment variables for sensitive data.
Files:
src/models/api/responses/successful/configuration.pytests/unit/models/config/test_observability_configuration.pysrc/models/config.pytests/integration/endpoints/test_config_integration.py
src/models/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
src/models/**/*.py: ExtendConfigurationBasefor configuration models andBaseModelfor data models.
Use@model_validatorand@field_validatorfor Pydantic model validation.
Files:
src/models/api/responses/successful/configuration.pysrc/models/config.py
tests/unit/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
tests/unit/**/*.py: Use pytest for all unit tests; do not use unittest.
Useconftest.pyfor shared fixtures,pytest-mockforAsyncMockobjects, andpytest.mark.asynciofor asynchronous tests.
Files:
tests/unit/models/config/test_observability_configuration.py
tests/integration/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Use pytest for all integration tests; do not use unittest.
Files:
tests/integration/endpoints/test_config_integration.py
🧠 Learnings (5)
📚 Learning: 2026-01-12T10:58:40.230Z
Learnt from: blublinsky
Repo: lightspeed-core/lightspeed-stack PR: 972
File: src/models/config.py:459-513
Timestamp: 2026-01-12T10:58:40.230Z
Learning: In lightspeed-core/lightspeed-stack, for Python files under src/models, when a user claims a fix is done but the issue persists, verify the current code state before accepting the fix. Steps: review the diff, fetch the latest changes, run relevant tests, reproduce the issue, search the codebase for lingering references to the original problem, confirm the fix is applied and not undone by subsequent commits, and validate with local checks to ensure the issue is resolved.
Applied to files:
src/models/api/responses/successful/configuration.pysrc/models/config.py
📚 Learning: 2026-02-25T07:46:33.545Z
Learnt from: asimurka
Repo: lightspeed-core/lightspeed-stack PR: 1211
File: src/models/responses.py:8-16
Timestamp: 2026-02-25T07:46:33.545Z
Learning: In the Python codebase, requests.py should use OpenAIResponseInputTool as Tool while responses.py uses OpenAIResponseTool as Tool. This difference is intentional due to differing schemas for input vs output tools in llama-stack-api. Apply this distinction consistently to other models under src/models (e.g., ensure request-related tools use the InputTool variant and response-related tools use the ResponseTool variant). If adding new tools, choose the corresponding InputTool or Tool class based on whether the tool represents input or output, and document the rationale in code comments.
Applied to files:
src/models/api/responses/successful/configuration.pysrc/models/config.py
📚 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:
src/models/api/responses/successful/configuration.pytests/unit/models/config/test_observability_configuration.pysrc/models/config.pytests/integration/endpoints/test_config_integration.py
📚 Learning: 2026-07-06T15:26:18.398Z
Learnt from: Jdubrick
Repo: lightspeed-core/lightspeed-stack PR: 2071
File: src/models/config.py:2416-2422
Timestamp: 2026-07-06T15:26:18.398Z
Learning: In this repo’s Python code under src/**, don’t treat differences in string concatenation style as a style inconsistency when Black has effectively forced (or made clearer) use of explicit `+` string concatenation in multi-line logger/string expressions. If adjacent-literal implicit concatenation is avoided/changed specifically to accommodate Black’s formatting in these call sites, accept the `+` usage and don’t recommend converting it solely for consistency with nearby blocks that use implicit concatenation.
Applied to files:
src/models/api/responses/successful/configuration.pysrc/models/config.py
📚 Learning: 2026-07-17T19:25:05.325Z
Learnt from: Jdubrick
Repo: lightspeed-core/lightspeed-stack PR: 2166
File: src/utils/saved_prompts.py:129-157
Timestamp: 2026-07-17T19:25:05.325Z
Learning: For any endpoint that handles saved prompts and calls `src/utils/saved_prompts.py::create_saved_prompt`, treat the endpoint as the validation boundary. Before calling `create_saved_prompt`, validate the incoming saved-prompt name and content, specifically using `validate_saved_prompt_name` and then persist (store) the normalized value it returns. Do not call `create_saved_prompt` with unvalidated/raw name/content.
Applied to files:
src/models/api/responses/successful/configuration.pysrc/models/config.py
🔇 Additional comments (1)
src/models/api/responses/successful/configuration.py (1)
90-97: LGTM!
| async def test_config_endpoint_observability_collects_otel_vars( | ||
| current_config: AppConfig, # pylint: disable=unused-argument | ||
| test_request: Request, # pylint: disable=unused-argument | ||
| test_auth: AuthTuple, # pylint: disable=unused-argument | ||
| monkeypatch: pytest.MonkeyPatch, | ||
| ) -> None: | ||
| """Test that observability config collects OTEL_* environment variables. | ||
|
|
||
| This integration test verifies: | ||
| - OTEL_* environment variables are collected into observability.otel | ||
| - Non-OTEL variables are not included | ||
| - Environment variables set at runtime are reflected in config | ||
|
|
||
| Parameters: | ||
| ---------- | ||
| current_config (AppConfig): Loads root configuration | ||
| test_request (Request): FastAPI request | ||
| test_auth (AuthTuple): noop authentication tuple | ||
| monkeypatch (pytest.MonkeyPatch): Fixture to modify environment variables | ||
| """ | ||
| # pylint: disable=import-outside-toplevel | ||
| from models.config import ObservabilityConfiguration | ||
|
|
||
| # Set OTEL environment variables | ||
| monkeypatch.setenv("OTEL_SDK_DISABLED", "true") | ||
| monkeypatch.setenv("OTEL_SERVICE_NAME", "test-service") | ||
| monkeypatch.setenv("OTEL_EXPORTER_OTLP_ENDPOINT", "http://localhost:4317") | ||
|
|
||
| # Reload configuration to pick up new env vars | ||
| updated_observability = ObservabilityConfiguration.from_environment() | ||
|
|
||
| # Verify OTEL vars are present in the environment | ||
| # (Note: The config is loaded at startup, so these may not be in the response | ||
| # unless we reload the entire config. This test verifies the mechanism works.) | ||
| assert "OTEL_SDK_DISABLED" in os.environ | ||
| assert "OTEL_SERVICE_NAME" in os.environ | ||
| assert "OTEL_EXPORTER_OTLP_ENDPOINT" in os.environ | ||
|
|
||
| # Verify that when we call from_environment(), it picks up the vars | ||
| assert "OTEL_SDK_DISABLED" in updated_observability.otel | ||
| assert updated_observability.otel["OTEL_SDK_DISABLED"] == "true" | ||
| assert "OTEL_SERVICE_NAME" in updated_observability.otel | ||
| assert updated_observability.otel["OTEL_SERVICE_NAME"] == "test-service" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Assert the mutated values through the endpoint.
This test never invokes config_endpoint_handler after setting the variables; it only re-tests ObservabilityConfiguration.from_environment(). Set the environment before rebuilding the app/config fixture, call the endpoint, and assert the OTEL values in response.configuration.observability.otel. Otherwise removal of the Configuration default factory can still pass this integration test.
🤖 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/endpoints/test_config_integration.py` around lines 125 -
167, Update test_config_endpoint_observability_collects_otel_vars to set the
OTEL environment variables before rebuilding the app/config fixture, then invoke
config_endpoint_handler and assert the values through
response.configuration.observability.otel. Remove the direct
ObservabilityConfiguration.from_environment() assertions so the test validates
endpoint behavior and catches configuration default-factory regressions.
| def test_from_environment_no_otel_vars(monkeypatch: pytest.MonkeyPatch) -> None: | ||
| """Test from_environment with no OTEL_* environment variables.""" |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Document test parameters.
Add Parameters: sections for monkeypatch, otel_dict, and expected_count in these test docstrings.
Based on learnings, this repository uses the Parameters: header for function arguments.
Also applies to: 27-28, 48-51, 100-101
🤖 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/unit/models/config/test_observability_configuration.py` around lines 16
- 17, Update the affected test docstrings in the observability configuration
tests to include a Parameters: section documenting each function argument:
monkeypatch, otel_dict, and expected_count. Apply this consistently to all
referenced tests, using the parameter names and their purposes without changing
test behavior.
Sources: Coding guidelines, Learnings
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 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 `@docs/LCORE-Strategic-Path-Forward.md`:
- Around line 42-59: Update the fenced code blocks in
docs/LCORE-Strategic-Path-Forward.md, including the diagrams around the
“Lightspeed Core Stack 2.0” section and the other referenced blocks, by adding
an appropriate language identifier such as text and blank lines before and after
each fence. Preserve all diagram content and formatting.
- Around line 96-107: Update the Table of Contents links for “The Problem” and
“The Opportunity” to use the current heading anchors, replacing the stale
problem slug with `#the-problem-lcores-failed-abstraction` and matching the
opportunity link to the slug generated by the document’s current opportunity
heading. Leave the remaining entries unchanged.
- Around line 61-69: Update the timeline and all referenced date sections to use
internally consistent calendar dates: correct the weekday labels for July 22,
July 26, and August 5, replace any relative “today” wording with an absolute
date, and align the 10-week plan starting August 5, 2026 with its approximately
October 14 completion rather than mid-September. Ensure the Gemini deadline
statement and the sections covering the additional referenced ranges reflect the
same schedule.
- Around line 201-216: Clarify the extraction gate’s coupling metric near the
“Rediscovering the Original Insight” analysis and its related checkpoint
sections. Define the reproducible counting method, categorize which dependencies
count as OpenShift coupling, state the denominator used for the percentage, and
ensure the “94 references,” “95% product-agnostic,” and “>20% coupling”
threshold use the same metric.
- Around line 35-37: Reconcile the build-from-scratch estimate throughout the
document, including the headline, comparison table, and 120/150-week scenarios,
by selecting one consistent baseline. Recalculate the time-advantage ratio and
all “10x faster” or related comparison claims from that baseline, and update the
decision analysis accordingly.
- Around line 174-184: Revise the “The Forcing Function: OGX Decommissioning”
section so it distinguishes confirmed milestones from the unconfirmed
decommissioning date. In the “If we stay on OGX” and “LCORE’s foundation”
statements, cite only the confirmed July announcement and explicitly frame the
strategic decision as contingent on the decommissioning timeline remaining
unconfirmed.
- Around line 373-417: The minimax analysis must use consistent probability and
cost units. Update the “Interpretation” section to retain the documented 10%
Build worst-case probability, or explicitly label 30% as a separate assumption
supported elsewhere; also either state the loaded dollar cost per engineering
week before using $16K/$600K or replace those dollar amounts with the
established 4 and 150+ eng-week values. Keep the scenario matrix, minimax
calculation, and conclusion aligned.
In `@tests/integration/endpoints/test_config_integration.py`:
- Around line 133-167: Extend the test around
ObservabilityConfiguration.from_environment() to assert
OTEL_EXPORTER_OTLP_ENDPOINT is present in updated_observability.otel with its
configured value, then set a non-OTEL environment variable and assert it is
excluded from updated_observability.otel.
In `@tests/unit/models/config/test_dump_configuration.py`:
- Around line 47-50: Isolate OTEL environment state in the dump tests by adding
a fixture that clears all OTEL_* variables, or by supplying an explicit empty
ObservabilityConfiguration when constructing Configuration. Ensure assertions
against _DEFAULT_OBSERVABILITY_DUMP remain deterministic regardless of the test
runner environment.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: f547b912-3718-4665-8e8d-c91579aa2798
📒 Files selected for processing (7)
docs/LCORE-Strategic-Path-Forward.mddocs/devel_doc/openapi.jsonsrc/models/api/responses/successful/configuration.pysrc/models/config.pytests/integration/endpoints/test_config_integration.pytests/unit/models/config/test_dump_configuration.pytests/unit/models/config/test_observability_configuration.py
📜 Review details
⏰ Context from checks skipped due to timeout. (13)
- GitHub Check: Pylinter
- GitHub Check: unit_tests (3.12)
- GitHub Check: integration_tests (3.12)
- GitHub Check: integration_tests (3.13)
- GitHub Check: build-pr
- GitHub Check: Konflux kflux-prd-rh02 / lightspeed-stack-0-7-on-pull-request
- GitHub Check: E2E Tests for Lightspeed Evaluation job
- GitHub Check: E2E: server mode / ci / group 1
- GitHub Check: E2E: library mode / ci / group 2
- GitHub Check: E2E: server mode / ci / group 2
- GitHub Check: E2E: library mode / ci / group 1
- GitHub Check: E2E: server mode / ci / group 3
- GitHub Check: E2E: library mode / ci / group 3
🧰 Additional context used
📓 Path-based instructions (5)
**/*
📄 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:
src/models/api/responses/successful/configuration.pysrc/models/config.pydocs/devel_doc/openapi.jsontests/integration/endpoints/test_config_integration.pytests/unit/models/config/test_observability_configuration.pydocs/LCORE-Strategic-Path-Forward.mdtests/unit/models/config/test_dump_configuration.py
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Use absolute imports for internal Python modules.
All modules must begin with descriptive docstrings explaining their purpose.
Uselogger = get_logger(__name__)fromlog.pyfor module logging.
Use complete type annotations for function parameters and return types.
Use modern union syntax such asstr | int; useOptional[Type]for optional values.
Usetyping_extensions.Selffor model validators.
Functions must use descriptive, action-oriented snake_case names such asget_,validate_, andcheck_.
Avoid modifying mutable parameters in place; return a new data structure instead.
Useasync deffor I/O operations and external API calls.
HandleAPIConnectionErrorfrom Llama Stack.
All classes must have descriptive docstrings and complete type annotations for class attributes; use specific types instead ofAny.
Use PascalCase for classes and descriptive standard suffixes such asConfiguration,Error/Exception,Resolver, andInterface.
Use ABC and@abstractmethodfor abstract interfaces.
Follow Google Python docstring conventions; document all modules, classes, and functions, includingParameters,Returns,Raises, andAttributessections as applicable.
Never commit secrets or keys; use environment variables for sensitive data.
Files:
src/models/api/responses/successful/configuration.pysrc/models/config.pytests/integration/endpoints/test_config_integration.pytests/unit/models/config/test_observability_configuration.pytests/unit/models/config/test_dump_configuration.py
src/models/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
src/models/**/*.py: ExtendConfigurationBasefor configuration models andBaseModelfor data models.
Use@model_validatorand@field_validatorfor Pydantic model validation.
Files:
src/models/api/responses/successful/configuration.pysrc/models/config.py
tests/integration/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Use pytest for all integration tests; do not use unittest.
Files:
tests/integration/endpoints/test_config_integration.py
tests/unit/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
tests/unit/**/*.py: Use pytest for all unit tests; do not use unittest.
Useconftest.pyfor shared fixtures,pytest-mockforAsyncMockobjects, andpytest.mark.asynciofor asynchronous tests.
Files:
tests/unit/models/config/test_observability_configuration.pytests/unit/models/config/test_dump_configuration.py
🧠 Learnings (5)
📚 Learning: 2026-01-12T10:58:40.230Z
Learnt from: blublinsky
Repo: lightspeed-core/lightspeed-stack PR: 972
File: src/models/config.py:459-513
Timestamp: 2026-01-12T10:58:40.230Z
Learning: In lightspeed-core/lightspeed-stack, for Python files under src/models, when a user claims a fix is done but the issue persists, verify the current code state before accepting the fix. Steps: review the diff, fetch the latest changes, run relevant tests, reproduce the issue, search the codebase for lingering references to the original problem, confirm the fix is applied and not undone by subsequent commits, and validate with local checks to ensure the issue is resolved.
Applied to files:
src/models/api/responses/successful/configuration.pysrc/models/config.py
📚 Learning: 2026-02-25T07:46:33.545Z
Learnt from: asimurka
Repo: lightspeed-core/lightspeed-stack PR: 1211
File: src/models/responses.py:8-16
Timestamp: 2026-02-25T07:46:33.545Z
Learning: In the Python codebase, requests.py should use OpenAIResponseInputTool as Tool while responses.py uses OpenAIResponseTool as Tool. This difference is intentional due to differing schemas for input vs output tools in llama-stack-api. Apply this distinction consistently to other models under src/models (e.g., ensure request-related tools use the InputTool variant and response-related tools use the ResponseTool variant). If adding new tools, choose the corresponding InputTool or Tool class based on whether the tool represents input or output, and document the rationale in code comments.
Applied to files:
src/models/api/responses/successful/configuration.pysrc/models/config.py
📚 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:
src/models/api/responses/successful/configuration.pysrc/models/config.pytests/integration/endpoints/test_config_integration.pytests/unit/models/config/test_observability_configuration.pytests/unit/models/config/test_dump_configuration.py
📚 Learning: 2026-07-06T15:26:18.398Z
Learnt from: Jdubrick
Repo: lightspeed-core/lightspeed-stack PR: 2071
File: src/models/config.py:2416-2422
Timestamp: 2026-07-06T15:26:18.398Z
Learning: In this repo’s Python code under src/**, don’t treat differences in string concatenation style as a style inconsistency when Black has effectively forced (or made clearer) use of explicit `+` string concatenation in multi-line logger/string expressions. If adjacent-literal implicit concatenation is avoided/changed specifically to accommodate Black’s formatting in these call sites, accept the `+` usage and don’t recommend converting it solely for consistency with nearby blocks that use implicit concatenation.
Applied to files:
src/models/api/responses/successful/configuration.pysrc/models/config.py
📚 Learning: 2026-07-17T19:25:05.325Z
Learnt from: Jdubrick
Repo: lightspeed-core/lightspeed-stack PR: 2166
File: src/utils/saved_prompts.py:129-157
Timestamp: 2026-07-17T19:25:05.325Z
Learning: For any endpoint that handles saved prompts and calls `src/utils/saved_prompts.py::create_saved_prompt`, treat the endpoint as the validation boundary. Before calling `create_saved_prompt`, validate the incoming saved-prompt name and content, specifically using `validate_saved_prompt_name` and then persist (store) the normalized value it returns. Do not call `create_saved_prompt` with unvalidated/raw name/content.
Applied to files:
src/models/api/responses/successful/configuration.pysrc/models/config.py
🪛 LanguageTool
docs/LCORE-Strategic-Path-Forward.md
[style] ~4-~4: Some style guides suggest that commas should set off the year in a month-day-year date.
Context: ...Anik Bhattacharjee Date: July 22, 2026 Status: Strategic Proposal - Seek...
(MISSING_COMMA_AFTER_YEAR)
[style] ~166-~166: To elevate your writing, try using more formal phrasing here.
Context: ...e - llama-stack gaps) - "Migration cost keeps growing as we add features" (true - 18 months d...
(CONTINUE_TO_VB)
[style] ~335-~335: Consider using an extreme adjective for ‘small’.
Context: ...irt Sizing) Size Legend: - XS (Extra Small) = Trivial/Exists (0-1 weeks) - S (...
(EXTREME_ADJECTIVES)
[inconsistency] ~550-~550: Did you mean to refer to the current year? July 22, 2026 is not a Tuesday, but a Wednesday.
Context: ...imeline) ### This Week (July 22-26) Tuesday, July 22 (today): - Share document with teams ...
(EN_DATE_WEEKDAY_CURRENTYEAR)
[inconsistency] ~552-~552: Did you mean to refer to the current year? July 26, 2026 is not a Friday, but a Sunday.
Context: ... with teams - Schedule strategy review (Friday, July 26) Friday, July 26: - **Strategy rev...
(EN_DATE_WEEKDAY_CURRENTYEAR)
[inconsistency] ~554-~554: Did you mean to refer to the current year? July 26, 2026 is not a Friday, but a Sunday.
Context: ...le strategy review (Friday, July 26) Friday, July 26: - Strategy review meeting (2 hou...
(EN_DATE_WEEKDAY_CURRENTYEAR)
[inconsistency] ~560-~560: Did you mean to refer to the current year? August 5, 2026 is not a Monday, but a Wednesday.
Context: ...k of August 5: Implementation Starts Monday, August 5: - Fork OLS → lightspeed-core-stack...
(EN_DATE_WEEKDAY_CURRENTYEAR)
[style] ~651-~651: Some style guides suggest that commas should set off the year in a month-day-year date.
Context: ...uperseded Revision Date: July 22, 2026 Next Review: July 26, 2026 (Strat...
(MISSING_COMMA_AFTER_YEAR)
🪛 markdownlint-cli2 (0.23.1)
docs/LCORE-Strategic-Path-Forward.md
[warning] 42-42: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 42-42: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 51-51: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 51-51: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 98-98: Link fragments should be valid
(MD051, link-fragments)
[warning] 99-99: Link fragments should be valid
(MD051, link-fragments)
[warning] 138-138: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 138-138: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 283-283: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 314-314: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 314-314: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 425-425: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 431-431: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 435-435: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 440-440: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 473-473: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (6)
src/models/config.py (1)
2857-2863: Redact sensitive OTEL header values before serialization.This still copies
OTEL_EXPORTER_OTLP_HEADERSand signal-specific header variables verbatim, despite the comment claiming secrets are excluded. SinceConfigurationis returned by the config response, exporter credentials can be disclosed. This repeats the existing unresolved finding.Source: Coding guidelines
tests/unit/models/config/test_observability_configuration.py (1)
16-17: Document test parameters.These docstrings still omit
Parameters:sections for fixture and parametrized inputs. As per coding guidelines, document function parameters using the repository’sParameters:format.Also applies to: 27-28, 48-51, 100-101
Sources: Coding guidelines, Learnings
docs/devel_doc/openapi.json (1)
6756-6763: LGTM!Also applies to: 12378-12382, 12479-12486, 15131-15146
tests/integration/endpoints/test_config_integration.py (2)
124-167: Exercise the/configendpoint in this test.The test still validates only
ObservabilityConfiguration.from_environment(). Rebuild the configuration after setting the variables, invokeconfig_endpoint_handler, and assert the values through the response so stale endpoint state cannot pass unnoticed. This is the same unresolved issue from the prior review.
3-3: LGTM!Also applies to: 94-123
tests/unit/models/config/test_dump_configuration.py (1)
239-239: LGTM!Also applies to: 467-467, 846-846, 1109-1109, 1405-1405, 1628-1628, 2011-2011, 2240-2240, 2469-2469, 2705-2705
| This integration test verifies: | ||
| - OTEL_* environment variables are collected into observability.otel | ||
| - Non-OTEL variables are not included | ||
| - Environment variables set at runtime are reflected in config | ||
|
|
||
| Parameters: | ||
| ---------- | ||
| current_config (AppConfig): Loads root configuration | ||
| test_request (Request): FastAPI request | ||
| test_auth (AuthTuple): noop authentication tuple | ||
| monkeypatch (pytest.MonkeyPatch): Fixture to modify environment variables | ||
| """ | ||
| # pylint: disable=import-outside-toplevel | ||
| from models.config import ObservabilityConfiguration | ||
|
|
||
| # Set OTEL environment variables | ||
| monkeypatch.setenv("OTEL_SDK_DISABLED", "true") | ||
| monkeypatch.setenv("OTEL_SERVICE_NAME", "test-service") | ||
| monkeypatch.setenv("OTEL_EXPORTER_OTLP_ENDPOINT", "http://localhost:4317") | ||
|
|
||
| # Reload configuration to pick up new env vars | ||
| updated_observability = ObservabilityConfiguration.from_environment() | ||
|
|
||
| # Verify OTEL vars are present in the environment | ||
| # (Note: The config is loaded at startup, so these may not be in the response | ||
| # unless we reload the entire config. This test verifies the mechanism works.) | ||
| assert "OTEL_SDK_DISABLED" in os.environ | ||
| assert "OTEL_SERVICE_NAME" in os.environ | ||
| assert "OTEL_EXPORTER_OTLP_ENDPOINT" in os.environ | ||
|
|
||
| # Verify that when we call from_environment(), it picks up the vars | ||
| assert "OTEL_SDK_DISABLED" in updated_observability.otel | ||
| assert updated_observability.otel["OTEL_SDK_DISABLED"] == "true" | ||
| assert "OTEL_SERVICE_NAME" in updated_observability.otel | ||
| assert updated_observability.otel["OTEL_SERVICE_NAME"] == "test-service" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Assert the complete environment contract.
Add an assertion that OTEL_EXPORTER_OTLP_ENDPOINT appears in updated_observability.otel, and set/assert a non-OTEL variable is excluded. The current test verifies only setup state for those cases.
🤖 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/endpoints/test_config_integration.py` around lines 133 -
167, Extend the test around ObservabilityConfiguration.from_environment() to
assert OTEL_EXPORTER_OTLP_ENDPOINT is present in updated_observability.otel with
its configured value, then set a non-OTEL environment variable and assert it is
excluded from updated_observability.otel.
| _DEFAULT_OBSERVABILITY_DUMP: dict[str, dict[str, str]] = { | ||
| "otel": {}, | ||
| } | ||
|
|
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Isolate OTEL environment state in dump tests.
Because Configuration derives observability from OTEL_*, these tests can fail whenever the runner already has an OTEL variable. Clear OTEL_* via a fixture or pass an explicit empty ObservabilityConfiguration before asserting the fixed dump.
🤖 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/unit/models/config/test_dump_configuration.py` around lines 47 - 50,
Isolate OTEL environment state in the dump tests by adding a fixture that clears
all OTEL_* variables, or by supplying an explicit empty
ObservabilityConfiguration when constructing Configuration. Ensure assertions
against _DEFAULT_OBSERVABILITY_DUMP remain deterministic regardless of the test
runner environment.
Add observability configuration to the v1/config endpoint, exposing all OTEL_* environment variables to provide visibility into the active OpenTelemetry tracing setup. Signed-off-by: Anik Bhattacharjee <anbhatta@redhat.com>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
tests/unit/models/config/test_observability_configuration.py (1)
16-17: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winDocument all test parameters.
These test docstrings omit
Parameters:sections formonkeypatch,otel_dict, andexpected_count. Add them consistently.As per coding guidelines and repository learnings, function parameters must use the repository’s
Parameters:documentation header.Also applies to: 27-28, 48-51, 100-101
🤖 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/unit/models/config/test_observability_configuration.py` around lines 16 - 17, Update the affected test docstrings, including test_from_environment_no_otel_vars and the tests at the referenced locations, to add a Parameters: section documenting every function argument: monkeypatch, otel_dict, and expected_count where present. Apply the repository’s established parameter-documentation format consistently without changing test behavior.Sources: Coding guidelines, Learnings
src/models/config.py (1)
2840-2841: 🔒 Security & Privacy | 🔴 Critical | ⚡ Quick winRedact OTEL credentials before serializing configuration.
from_environment()copies everyOTEL_*value intoConfiguration.observability, which is serialized by the config response anddump(). This includesOTEL_EXPORTER_OTLP_HEADERSand signal-specific header variables that may contain bearer tokens or API keys; the “excluding secrets” documentation is false. Use a reviewed allowlist or redact sensitive OTEL keys, and add a regression test.As per coding guidelines, sensitive data must not be leaked in API responses.
Also applies to: 2857-2863, 3027-3032
🤖 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 `@src/models/config.py` around lines 2840 - 2841, Update Configuration.from_environment() so OTEL credentials and header-bearing variables, including OTEL_EXPORTER_OTLP_HEADERS and signal-specific headers, are excluded or redacted before populating observability; preserve only a reviewed non-secret allowlist. Ensure serialized config responses and dump() cannot expose these values, and add a regression test covering the sensitive keys.Source: Coding guidelines
🤖 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/endpoints/test_config_integration.py`:
- Around line 148-167: Update ObservabilityConfiguration.from_environment and
its OTEL response model to exclude credential-bearing keys such as
OTEL_EXPORTER_OTLP_HEADERS when collecting OTEL_* environment variables. Extend
this integration test to set that variable and assert it is absent from
updated_observability.otel, while preserving exposure of non-sensitive OTEL
settings.
---
Duplicate comments:
In `@src/models/config.py`:
- Around line 2840-2841: Update Configuration.from_environment() so OTEL
credentials and header-bearing variables, including OTEL_EXPORTER_OTLP_HEADERS
and signal-specific headers, are excluded or redacted before populating
observability; preserve only a reviewed non-secret allowlist. Ensure serialized
config responses and dump() cannot expose these values, and add a regression
test covering the sensitive keys.
In `@tests/unit/models/config/test_observability_configuration.py`:
- Around line 16-17: Update the affected test docstrings, including
test_from_environment_no_otel_vars and the tests at the referenced locations, to
add a Parameters: section documenting every function argument: monkeypatch,
otel_dict, and expected_count where present. Apply the repository’s established
parameter-documentation format consistently without changing test behavior.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 1c1e708b-fc8b-4c20-a353-f550a835f421
📒 Files selected for processing (6)
docs/devel_doc/openapi.jsonsrc/models/api/responses/successful/configuration.pysrc/models/config.pytests/integration/endpoints/test_config_integration.pytests/unit/models/config/test_dump_configuration.pytests/unit/models/config/test_observability_configuration.py
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: Konflux kflux-prd-rh02 / lightspeed-stack-0-7-on-pull-request
🧰 Additional context used
📓 Path-based instructions (5)
**/*
📄 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:
src/models/api/responses/successful/configuration.pydocs/devel_doc/openapi.jsontests/unit/models/config/test_observability_configuration.pytests/integration/endpoints/test_config_integration.pytests/unit/models/config/test_dump_configuration.pysrc/models/config.py
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Use absolute imports for internal Python modules.
All modules must begin with descriptive docstrings explaining their purpose.
Uselogger = get_logger(__name__)fromlog.pyfor module logging.
Use complete type annotations for function parameters and return types.
Use modern union syntax such asstr | int; useOptional[Type]for optional values.
Usetyping_extensions.Selffor model validators.
Functions must use descriptive, action-oriented snake_case names such asget_,validate_, andcheck_.
Avoid modifying mutable parameters in place; return a new data structure instead.
Useasync deffor I/O operations and external API calls.
HandleAPIConnectionErrorfrom Llama Stack.
All classes must have descriptive docstrings and complete type annotations for class attributes; use specific types instead ofAny.
Use PascalCase for classes and descriptive standard suffixes such asConfiguration,Error/Exception,Resolver, andInterface.
Use ABC and@abstractmethodfor abstract interfaces.
Follow Google Python docstring conventions; document all modules, classes, and functions, includingParameters,Returns,Raises, andAttributessections as applicable.
Never commit secrets or keys; use environment variables for sensitive data.
Files:
src/models/api/responses/successful/configuration.pytests/unit/models/config/test_observability_configuration.pytests/integration/endpoints/test_config_integration.pytests/unit/models/config/test_dump_configuration.pysrc/models/config.py
src/models/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
src/models/**/*.py: ExtendConfigurationBasefor configuration models andBaseModelfor data models.
Use@model_validatorand@field_validatorfor Pydantic model validation.
Files:
src/models/api/responses/successful/configuration.pysrc/models/config.py
tests/unit/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
tests/unit/**/*.py: Use pytest for all unit tests; do not use unittest.
Useconftest.pyfor shared fixtures,pytest-mockforAsyncMockobjects, andpytest.mark.asynciofor asynchronous tests.
Files:
tests/unit/models/config/test_observability_configuration.pytests/unit/models/config/test_dump_configuration.py
tests/integration/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Use pytest for all integration tests; do not use unittest.
Files:
tests/integration/endpoints/test_config_integration.py
🧠 Learnings (5)
📚 Learning: 2026-01-12T10:58:40.230Z
Learnt from: blublinsky
Repo: lightspeed-core/lightspeed-stack PR: 972
File: src/models/config.py:459-513
Timestamp: 2026-01-12T10:58:40.230Z
Learning: In lightspeed-core/lightspeed-stack, for Python files under src/models, when a user claims a fix is done but the issue persists, verify the current code state before accepting the fix. Steps: review the diff, fetch the latest changes, run relevant tests, reproduce the issue, search the codebase for lingering references to the original problem, confirm the fix is applied and not undone by subsequent commits, and validate with local checks to ensure the issue is resolved.
Applied to files:
src/models/api/responses/successful/configuration.pysrc/models/config.py
📚 Learning: 2026-02-25T07:46:33.545Z
Learnt from: asimurka
Repo: lightspeed-core/lightspeed-stack PR: 1211
File: src/models/responses.py:8-16
Timestamp: 2026-02-25T07:46:33.545Z
Learning: In the Python codebase, requests.py should use OpenAIResponseInputTool as Tool while responses.py uses OpenAIResponseTool as Tool. This difference is intentional due to differing schemas for input vs output tools in llama-stack-api. Apply this distinction consistently to other models under src/models (e.g., ensure request-related tools use the InputTool variant and response-related tools use the ResponseTool variant). If adding new tools, choose the corresponding InputTool or Tool class based on whether the tool represents input or output, and document the rationale in code comments.
Applied to files:
src/models/api/responses/successful/configuration.pysrc/models/config.py
📚 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:
src/models/api/responses/successful/configuration.pytests/unit/models/config/test_observability_configuration.pytests/integration/endpoints/test_config_integration.pytests/unit/models/config/test_dump_configuration.pysrc/models/config.py
📚 Learning: 2026-07-06T15:26:18.398Z
Learnt from: Jdubrick
Repo: lightspeed-core/lightspeed-stack PR: 2071
File: src/models/config.py:2416-2422
Timestamp: 2026-07-06T15:26:18.398Z
Learning: In this repo’s Python code under src/**, don’t treat differences in string concatenation style as a style inconsistency when Black has effectively forced (or made clearer) use of explicit `+` string concatenation in multi-line logger/string expressions. If adjacent-literal implicit concatenation is avoided/changed specifically to accommodate Black’s formatting in these call sites, accept the `+` usage and don’t recommend converting it solely for consistency with nearby blocks that use implicit concatenation.
Applied to files:
src/models/api/responses/successful/configuration.pysrc/models/config.py
📚 Learning: 2026-07-17T19:25:05.325Z
Learnt from: Jdubrick
Repo: lightspeed-core/lightspeed-stack PR: 2166
File: src/utils/saved_prompts.py:129-157
Timestamp: 2026-07-17T19:25:05.325Z
Learning: For any endpoint that handles saved prompts and calls `src/utils/saved_prompts.py::create_saved_prompt`, treat the endpoint as the validation boundary. Before calling `create_saved_prompt`, validate the incoming saved-prompt name and content, specifically using `validate_saved_prompt_name` and then persist (store) the normalized value it returns. Do not call `create_saved_prompt` with unvalidated/raw name/content.
Applied to files:
src/models/api/responses/successful/configuration.pysrc/models/config.py
🔇 Additional comments (6)
src/models/config.py (1)
5-5: LGTM!tests/unit/models/config/test_observability_configuration.py (1)
1-13: LGTM!Also applies to: 66-77, 106-117, 119-125
src/models/api/responses/successful/configuration.py (1)
90-97: LGTM!docs/devel_doc/openapi.json (1)
6756-6763: LGTM!Also applies to: 12378-12382, 12479-12486, 15131-15146
tests/integration/endpoints/test_config_integration.py (1)
125-167: The endpoint coverage gap remains.This still calls
ObservabilityConfiguration.from_environment()directly rather than rebuilding configuration and asserting the handler response, so it cannot catch a brokenConfigurationdefault factory.tests/unit/models/config/test_dump_configuration.py (1)
47-50: The environment-dependent dump expectation remains.The fixed empty mapping still fails when the test runner has any
OTEL_*variable, becauseConfigurationderives observability at construction time.
| # Set OTEL environment variables | ||
| monkeypatch.setenv("OTEL_SDK_DISABLED", "true") | ||
| monkeypatch.setenv("OTEL_SERVICE_NAME", "test-service") | ||
| monkeypatch.setenv("OTEL_EXPORTER_OTLP_ENDPOINT", "http://localhost:4317") | ||
|
|
||
| # Reload configuration to pick up new env vars | ||
| updated_observability = ObservabilityConfiguration.from_environment() | ||
|
|
||
| # Verify OTEL vars are present in the environment | ||
| # (Note: The config is loaded at startup, so these may not be in the response | ||
| # unless we reload the entire config. This test verifies the mechanism works.) | ||
| assert "OTEL_SDK_DISABLED" in os.environ | ||
| assert "OTEL_SERVICE_NAME" in os.environ | ||
| assert "OTEL_EXPORTER_OTLP_ENDPOINT" in os.environ | ||
|
|
||
| # Verify that when we call from_environment(), it picks up the vars | ||
| assert "OTEL_SDK_DISABLED" in updated_observability.otel | ||
| assert updated_observability.otel["OTEL_SDK_DISABLED"] == "true" | ||
| assert "OTEL_SERVICE_NAME" in updated_observability.otel | ||
| assert updated_observability.otel["OTEL_SERVICE_NAME"] == "test-service" |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Exclude credential-bearing OTEL variables from the response contract.
OTEL_EXPORTER_OTLP_HEADERS commonly carries authorization tokens. The supplied collector copies every OTEL_* value, while this test validates that broad contract. Filter sensitive keys in the model and assert this variable is absent from the endpoint response.
As per coding guidelines, flag sensitive data leaked in API responses.
🤖 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/endpoints/test_config_integration.py` around lines 148 -
167, Update ObservabilityConfiguration.from_environment and its OTEL response
model to exclude credential-bearing keys such as OTEL_EXPORTER_OTLP_HEADERS when
collecting OTEL_* environment variables. Extend this integration test to set
that variable and assert it is absent from updated_observability.otel, while
preserving exposure of non-sensitive OTEL settings.
Source: Coding guidelines
1 participant
Description
Add observability configuration to the v1/config endpoint, exposing all OTEL_* environment variables to provide visibility into the active OpenTelemetry tracing setup.
Type of change
Tools used to create PR
Identify any AI code assistants used in this PR (for transparency and review context)
Related Tickets & Documents
Checklist before requesting a review
Testing
Summary by CodeRabbit
New Features
OTEL_.Documentation
Bug Fixes