LCORE-1826: Expose OpenTelemetry configuration in v1/config endpoint - #2283
LCORE-1826: Expose OpenTelemetry configuration in v1/config endpoint#2283anik120 wants to merge 1 commit into
Conversation
WalkthroughThe configuration model derives ChangesObservability configuration
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Environment
participant ObservabilityConfiguration
participant Configuration
participant config_endpoint_handler
Environment->>ObservabilityConfiguration: Read OTEL_* variables
ObservabilityConfiguration->>ObservabilityConfiguration: Redact sensitive values
ObservabilityConfiguration->>Configuration: Populate observability
Configuration->>config_endpoint_handler: Provide configuration
config_endpoint_handler-->>Configuration: Return observability.otel
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 7✅ Passed checks (7 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 |
| for key, value in os.environ.items(): | ||
| if key.startswith("OTEL_"): | ||
| # Exclude sensitive variables that might contain tokens or keys | ||
| # Currently no secret OTEL_ vars are used, but this is future-proof |
There was a problem hiding this comment.
This is actually not true. Anyone can export optional env variables and those will be leaked now.
Please research which OTEL variables can contain sensitive values and wrap those into SecretStr for example. The list of all configurable OTEL variables is attached in design doc
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 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/devel_doc/openapi.json`:
- Around line 15133-15148: Update ObservabilityConfiguration.from_environment()
to include OTEL_EXPORTER_OTLP_TRACES_HEADERS,
OTEL_EXPORTER_OTLP_METRICS_HEADERS, and OTEL_EXPORTER_OTLP_LOGS_HEADERS in the
existing redaction set, ensuring these per-signal header values are masked in
the public /v1/config response alongside OTEL_EXPORTER_OTLP_HEADERS.
In `@src/models/config.py`:
- Around line 39-45: Replace the exact-name _SECRET_OTEL_VARS approach with a
complete Pydantic validator on ObservabilityConfiguration. Have the validator
redact generic and signal-specific OTEL headers, certificates, and client key
variants, return a new mapping without mutating caller data, and ensure both
collector output and direct ObservabilityConfiguration(otel=...) construction
are covered by the validation path.
In `@tests/integration/endpoints/test_config_integration.py`:
- Around line 124-167: The test_config_endpoint_observability_collects_otel_vars
test currently bypasses the endpoint and only validates
ObservabilityConfiguration.from_environment(). After setting the OTEL variables,
recreate or reload the root configuration, invoke config_endpoint_handler, and
assert the expected OTEL values through
response.configuration.observability.otel, ensuring endpoint model-to-response
wiring is exercised.
In `@tests/unit/models/config/test_dump_configuration.py`:
- Around line 47-49: Update the tests using _DEFAULT_OBSERVABILITY_DUMP to
isolate them from ambient OTEL_* environment variables: clear those variables in
a fixture before constructing Configuration, or derive the expected dump from
the controlled environment. Ensure expectations remain stable on instrumented CI
runners while preserving the existing default behavior.
In `@tests/unit/models/config/test_observability_configuration.py`:
- Around line 16-17: Update the affected test functions, including
test_from_environment_no_otel_vars and the additionally referenced tests, to
document every fixture and parametrized input in their docstrings. Add a
Parameters: section describing each applicable monkeypatch, otel_dict, and
expected_count parameter, while preserving the existing test descriptions and
behavior.
- Around line 170-198: Update _SECRET_OTEL_VARS and
test_from_environment_redacts_all_secret_vars so signal-specific OTLP header
variables—OTEL_EXPORTER_OTLP_TRACES_HEADERS, OTEL_EXPORTER_OTLP_METRICS_HEADERS,
and OTEL_EXPORTER_OTLP_LOGS_HEADERS—are redacted alongside
OTEL_EXPORTER_OTLP_HEADERS, while non-secret variables remain unchanged.
🪄 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: 9f50b0c6-44a2-4be5-9c0b-18bf134d51eb
📒 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. (14)
- GitHub Check: build-pr
- GitHub Check: integration_tests (3.13)
- GitHub Check: integration_tests (3.12)
- GitHub Check: Pylinter
- GitHub Check: unit_tests (3.12)
- GitHub Check: unit_tests (3.13)
- GitHub Check: E2E Tests for Lightspeed Evaluation job
- GitHub Check: E2E: library mode / ci / group 2
- GitHub Check: E2E: library mode / ci / group 1
- GitHub Check: E2E: library mode / ci / group 3
- GitHub Check: E2E: server mode / ci / group 1
- GitHub Check: E2E: server mode / ci / group 3
- GitHub Check: E2E: server mode / ci / group 2
- 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/integration/endpoints/test_config_integration.pysrc/models/config.pytests/unit/models/config/test_observability_configuration.pytests/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.pytests/integration/endpoints/test_config_integration.pysrc/models/config.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.pytests/integration/endpoints/test_config_integration.pysrc/models/config.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
🔇 Additional comments (5)
src/models/config.py (2)
2842-2850: LGTM!
3039-3044: 🎯 Functional CorrectnessVerify exact configuration-dump fixtures include the new default.
default_factory=ObservabilityConfiguration.from_environmentadds anobservabilitysection to everyConfiguration.model_dump(), even when no OTEL variables exist, and makes dumps depend on the process environment. Existing full-dictionary dump fixtures must either include this field or explicitly isolate/override the environment-derived configuration.src/models/api/responses/successful/configuration.py (1)
90-98: LGTM!docs/devel_doc/openapi.json (1)
12379-12383: LGTM!tests/integration/endpoints/test_config_integration.py (1)
94-121: LGTM!
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: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/models/config.py`:
- Around line 2850-2908: Update ObservabilityConfiguration.redact_secrets to use
a reviewed allowlist of explicitly safe OTEL variable names, redacting every
unknown key—including resource attributes and vendor extensions—by default.
Preserve only allowlisted non-sensitive values, and sanitize compound values so
embedded secrets are not returned verbatim; ensure from_environment’s complete
OTEL_* collection cannot bypass this validator.
In `@tests/unit/models/config/test_dump_configuration.py`:
- Around line 57-74: Update the docstrings for clear_otel_env_fixture and
_get_expected_observability_dump. Add a Parameters: section documenting the
monkeypatch parameter in clear_otel_env_fixture, and revise the helper docstring
to state that it returns the fixed expected observability dump with an empty
otel mapping, without implying it reads the 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: 4545a430-0abb-414f-9fde-78d031e58651
📒 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. (16)
- GitHub Check: mypy
- GitHub Check: integration_tests (3.13)
- GitHub Check: unit_tests (3.12)
- GitHub Check: integration_tests (3.12)
- GitHub Check: unit_tests (3.13)
- GitHub Check: E2E: library mode / ci / group 3
- GitHub Check: E2E: library mode / ci / group 2
- GitHub Check: E2E: server mode / ci / group 3
- GitHub Check: E2E: library mode / ci / group 1
- GitHub Check: E2E: server mode / ci / group 2
- GitHub Check: E2E: server mode / ci / group 1
- GitHub Check: build-pr
- GitHub Check: Pyright
- GitHub Check: Pylinter
- GitHub Check: E2E Tests for Lightspeed Evaluation job
- 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.jsonsrc/models/config.pytests/integration/endpoints/test_config_integration.pytests/unit/models/config/test_dump_configuration.pytests/unit/models/config/test_observability_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_dump_configuration.pytests/unit/models/config/test_observability_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_dump_configuration.pytests/unit/models/config/test_observability_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_dump_configuration.pytests/unit/models/config/test_observability_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
🪛 ast-grep (0.45.0)
tests/unit/models/config/test_observability_configuration.py
[warning] 259-259: Do not make http calls without encryption
Context: "http://collector:4317"
Note: [CWE-319] Cleartext Transmission of Sensitive Information.
(requests-http)
[warning] 270-270: Do not make http calls without encryption
Context: "http://collector:4317"
Note: [CWE-319] Cleartext Transmission of Sensitive Information.
(requests-http)
🔇 Additional comments (12)
src/models/config.py (3)
5-5: LGTM!
2834-2848: LGTM!
3072-3077: LGTM!src/models/api/responses/successful/configuration.py (1)
90-98: LGTM!docs/devel_doc/openapi.json (2)
15133-15148: Same redaction gap flagged previously.This re-introduces the
ObservabilityConfigurationschema with the same structure previously reviewed. The prior round noted that onlyOTEL_EXPORTER_OTLP_HEADERSappears redacted, while per-signal variants (OTEL_EXPORTER_OTLP_TRACES_HEADERS,_METRICS_HEADERS,_LOGS_HEADERS) are validOTEL_*values that can carry bearer/API tokens and should also be masked before exposure via/v1/config.src/models/config.py(wherefrom_environment()/redaction logic lives) isn't included in this batch, so I can't confirm whether this was addressed — please verify.
6756-6764: LGTM!Also applies to: 12379-12383, 12480-12488
tests/unit/models/config/test_observability_configuration.py (2)
1-146: LGTM!
149-313: LGTM! These redaction tests (including the new signal-specific header/cert coverage at lines 254-294) close the gap flagged in the prior review round.tests/unit/models/config/test_dump_configuration.py (2)
7-11: LGTM!
257-257: LGTM!Also applies to: 485-485, 864-864, 1127-1127, 1423-1423, 1646-1646, 2029-2029, 2258-2258, 2487-2487, 2723-2723
tests/integration/endpoints/test_config_integration.py (2)
93-121: LGTM!
152-158: 🩺 Stability & AvailabilityNo change needed.
current_configis function-scoped and the integration conftest auto-resets singleton configuration state before each test.
| @field_validator("otel", mode="before") | ||
| @classmethod | ||
| def redact_secrets(cls, value: dict[str, str]) -> dict[str, str]: | ||
| """Redact sensitive OTEL environment variables. | ||
|
|
||
| Redacts headers, certificates, and client keys for generic and signal-specific | ||
| (traces, metrics, logs) OTLP exporters. | ||
|
|
||
| Parameters: | ||
| ---------- | ||
| value: Dictionary of OTEL environment variables | ||
|
|
||
| Returns: | ||
| New dictionary with sensitive values redacted as "[REDACTED]" | ||
| """ | ||
| if not value: | ||
| return value | ||
|
|
||
| # Create a new dict to avoid mutating caller's data | ||
| redacted = {} | ||
|
|
||
| for key, val in value.items(): | ||
| # Redact generic and signal-specific OTLP headers | ||
| # Matches: OTEL_EXPORTER_OTLP_HEADERS, | ||
| # OTEL_EXPORTER_OTLP_TRACES_HEADERS, | ||
| # OTEL_EXPORTER_OTLP_METRICS_HEADERS, | ||
| # OTEL_EXPORTER_OTLP_LOGS_HEADERS | ||
| if "HEADERS" in key and "OTLP" in key: | ||
| redacted[key] = "[REDACTED]" | ||
| # Redact generic and signal-specific certificates | ||
| # Matches: OTEL_EXPORTER_OTLP_CERTIFICATE, | ||
| # OTEL_EXPORTER_OTLP_TRACES_CERTIFICATE, etc. | ||
| elif "CERTIFICATE" in key and "OTLP" in key: | ||
| redacted[key] = "[REDACTED]" | ||
| # Redact generic and signal-specific client keys | ||
| # Matches: OTEL_EXPORTER_OTLP_CLIENT_KEY, | ||
| # OTEL_EXPORTER_OTLP_TRACES_CLIENT_KEY, etc. | ||
| elif "CLIENT_KEY" in key and "OTLP" in key: | ||
| redacted[key] = "[REDACTED]" | ||
| else: | ||
| redacted[key] = val | ||
|
|
||
| return redacted | ||
|
|
||
| @classmethod | ||
| def from_environment(cls) -> "ObservabilityConfiguration": | ||
| """Collect all OTEL_* environment variables from the environment. | ||
|
|
||
| Sensitive variables (headers, certificates, keys) are automatically redacted | ||
| by the field validator. | ||
|
|
||
| Returns: | ||
| ObservabilityConfiguration with otel dict populated from environment. | ||
| """ | ||
| otel_vars = {} | ||
| for key, value in os.environ.items(): | ||
| if key.startswith("OTEL_"): | ||
| otel_vars[key] = value | ||
| return cls(otel=otel_vars) |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
Redact unknown OTEL_* values by default.
The collector exports every OTEL_* variable, but the validator masks only keys containing HEADERS, CERTIFICATE, or CLIENT_KEY. Values such as OTEL_RESOURCE_ATTRIBUTES or vendor extension variables (for example, OTEL_VENDOR_API_TOKEN) are returned verbatim through the configuration response. Use a reviewed safe allowlist, redact unknown values, and sanitize compound values.
Based on learnings, verify the current fix fully resolves the earlier broad environment-variable disclosure concern.
🤖 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 2850 - 2908, Update
ObservabilityConfiguration.redact_secrets to use a reviewed allowlist of
explicitly safe OTEL variable names, redacting every unknown key—including
resource attributes and vendor extensions—by default. Preserve only allowlisted
non-sensitive values, and sanitize compound values so embedded secrets are not
returned verbatim; ensure from_environment’s complete OTEL_* collection cannot
bypass this validator.
Source: Learnings
| @pytest.fixture(name="clear_otel_env", autouse=True) | ||
| def clear_otel_env_fixture(monkeypatch: pytest.MonkeyPatch) -> None: | ||
| """Clear OTEL_* environment variables to prevent ambient leakage in tests. | ||
|
|
||
| This fixture ensures dump configuration tests have stable expectations | ||
| regardless of whether they run on instrumented CI runners. | ||
| """ | ||
| for key in list(os.environ.keys()): | ||
| if key.startswith("OTEL_"): | ||
| monkeypatch.delenv(key, raising=False) | ||
|
|
||
|
|
||
| def _get_expected_observability_dump() -> dict[str, dict[str, str]]: | ||
| """Get expected observability dump based on current environment. | ||
|
|
||
| Returns empty otel dict when OTEL_* vars are cleared by fixture. | ||
| """ | ||
| return {"otel": {}} |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Document the fixture's monkeypatch parameter; tighten the helper's docstring.
clear_otel_env_fixture takes monkeypatch but its docstring has no Parameters: section. As per coding guidelines, "document all modules, classes, and functions, including Parameters, Returns, Raises, and Attributes sections as applicable." Based on learnings, use the Parameters: header (not Args:).
Separately, _get_expected_observability_dump's docstring says the result is computed "based on current environment," but the function just returns a hardcoded {"otel": {}} — it never reads os.environ. It's correct today because the autouse fixture always clears OTEL_* first, but the wording invites someone to reuse this helper without that guarantee.
📝 Proposed docstring fixes
`@pytest.fixture`(name="clear_otel_env", autouse=True)
def clear_otel_env_fixture(monkeypatch: pytest.MonkeyPatch) -> None:
"""Clear OTEL_* environment variables to prevent ambient leakage in tests.
This fixture ensures dump configuration tests have stable expectations
regardless of whether they run on instrumented CI runners.
+
+ Parameters:
+ ----------
+ monkeypatch (pytest.MonkeyPatch): Pytest fixture for environment manipulation.
"""
for key in list(os.environ.keys()):
if key.startswith("OTEL_"):
monkeypatch.delenv(key, raising=False)
def _get_expected_observability_dump() -> dict[str, dict[str, str]]:
- """Get expected observability dump based on current environment.
-
- Returns empty otel dict when OTEL_* vars are cleared by fixture.
- """
+ """Get the expected observability dump for tests in this module.
+
+ Returns a static empty otel dict, relying on the autouse
+ `clear_otel_env` fixture to guarantee no OTEL_* vars are set.
+ """
return {"otel": {}}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @pytest.fixture(name="clear_otel_env", autouse=True) | |
| def clear_otel_env_fixture(monkeypatch: pytest.MonkeyPatch) -> None: | |
| """Clear OTEL_* environment variables to prevent ambient leakage in tests. | |
| This fixture ensures dump configuration tests have stable expectations | |
| regardless of whether they run on instrumented CI runners. | |
| """ | |
| for key in list(os.environ.keys()): | |
| if key.startswith("OTEL_"): | |
| monkeypatch.delenv(key, raising=False) | |
| def _get_expected_observability_dump() -> dict[str, dict[str, str]]: | |
| """Get expected observability dump based on current environment. | |
| Returns empty otel dict when OTEL_* vars are cleared by fixture. | |
| """ | |
| return {"otel": {}} | |
| `@pytest.fixture`(name="clear_otel_env", autouse=True) | |
| def clear_otel_env_fixture(monkeypatch: pytest.MonkeyPatch) -> None: | |
| """Clear OTEL_* environment variables to prevent ambient leakage in tests. | |
| This fixture ensures dump configuration tests have stable expectations | |
| regardless of whether they run on instrumented CI runners. | |
| Parameters: | |
| ---------- | |
| monkeypatch (pytest.MonkeyPatch): Pytest fixture for environment manipulation. | |
| """ | |
| for key in list(os.environ.keys()): | |
| if key.startswith("OTEL_"): | |
| monkeypatch.delenv(key, raising=False) | |
| def _get_expected_observability_dump() -> dict[str, dict[str, str]]: | |
| """Get the expected observability dump for tests in this module. | |
| Returns a static empty otel dict, relying on the autouse | |
| `clear_otel_env` fixture to guarantee no OTEL_* vars are set. | |
| """ | |
| return {"otel": {}} |
🤖 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 57 - 74,
Update the docstrings for clear_otel_env_fixture and
_get_expected_observability_dump. Add a Parameters: section documenting the
monkeypatch parameter in clear_otel_env_fixture, and revise the helper docstring
to state that it returns the fixed expected observability dump with an empty
otel mapping, without implying it reads the environment.
Sources: Coding guidelines, Learnings
2 participants
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
observabilitysection to the configuration response.OTEL_*environment variables.Tests
/configresponse structure andOTEL_*collection.