Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 39 additions & 0 deletions docs/devel_doc/openapi.json
Original file line number Diff line number Diff line change
Expand Up @@ -6753,6 +6753,15 @@
}
],
"name": "lightspeed-stack",
"observability": {
"otel": {
"OTEL_EXPORTER_OTLP_ENDPOINT": "",
"OTEL_EXPORTER_OTLP_HEADERS": "[REDACTED]",
"OTEL_EXPORTER_OTLP_PROTOCOL": "",
"OTEL_SDK_DISABLED": "true",
"OTEL_SERVICE_NAME": ""
}
},
"quota_handlers": {
"enable_token_history": false,
"limiters": [],
Expand Down Expand Up @@ -12367,6 +12376,11 @@
"title": "Splunk configuration",
"description": "Splunk HEC configuration for sending telemetry events."
},
"observability": {
"$ref": "#/components/schemas/ObservabilityConfiguration",
"title": "Observability configuration",
"description": "OpenTelemetry and observability configuration collected from OTEL_* environment variables."
},
"deployment_environment": {
"type": "string",
"title": "Deployment environment",
Expand Down Expand Up @@ -12463,6 +12477,15 @@
}
],
"name": "lightspeed-stack",
"observability": {
"otel": {
"OTEL_EXPORTER_OTLP_ENDPOINT": "",
"OTEL_EXPORTER_OTLP_HEADERS": "[REDACTED]",
"OTEL_EXPORTER_OTLP_PROTOCOL": "",
"OTEL_SDK_DISABLED": "true",
"OTEL_SERVICE_NAME": ""
}
},
"quota_handlers": {
"enable_token_history": false,
"limiters": [],
Expand Down Expand Up @@ -15107,6 +15130,22 @@
"title": "OAuthFlows",
"description": "Defines the configuration for the supported OAuth 2.0 flows."
},
"ObservabilityConfiguration": {
"properties": {
"otel": {
"additionalProperties": {
"type": "string"
},
"type": "object",
"title": "OpenTelemetry configuration",
"description": "Active OpenTelemetry configuration from OTEL_* environment variables"
}
},
"additionalProperties": false,
"type": "object",
"title": "ObservabilityConfiguration",
"description": "OpenTelemetry observability configuration.\n\nThis configuration is automatically populated from OTEL_* environment variables\nto provide visibility into the active tracing setup.\n\nAttributes:\n otel: Dictionary of OTEL_* environment variables with secrets redacted."
},
Comment thread
coderabbitai[bot] marked this conversation as resolved.
"OkpConfiguration": {
"properties": {
"rhokp_url": {
Expand Down
9 changes: 9 additions & 0 deletions src/models/api/responses/successful/configuration.py
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,15 @@ class ConfigurationResponse(AbstractSuccessfulResponse):
"scheduler": {"period": 1},
"enable_token_history": False,
},
"observability": {
"otel": {
"OTEL_SDK_DISABLED": "true",
"OTEL_EXPORTER_OTLP_ENDPOINT": "",
"OTEL_EXPORTER_OTLP_PROTOCOL": "",
"OTEL_SERVICE_NAME": "",
"OTEL_EXPORTER_OTLP_HEADERS": "[REDACTED]",
}
},
}
}
]
Expand Down
85 changes: 85 additions & 0 deletions src/models/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

# pylint: disable=too-many-lines

import os
import re
from enum import Enum
from functools import cached_property
Expand Down Expand Up @@ -2830,6 +2831,83 @@ def compiled_patterns(self) -> CompiledPatterns:
return list(self._compiled_patterns)


class ObservabilityConfiguration(ConfigurationBase):
"""OpenTelemetry observability configuration.

This configuration is automatically populated from OTEL_* environment variables
to provide visibility into the active tracing setup.

Attributes:
otel: Dictionary of OTEL_* environment variables with secrets redacted.
"""

otel: dict[str, str] = Field(
default_factory=dict,
title="OpenTelemetry configuration",
description="Active OpenTelemetry configuration from OTEL_* environment variables",
)

@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)
Comment on lines +2850 to +2908

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 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



class Configuration(ConfigurationBase):
"""Global service configuration."""

Expand Down Expand Up @@ -2991,6 +3069,13 @@ class Configuration(ConfigurationBase):
description="Splunk HEC configuration for sending telemetry events.",
)

observability: ObservabilityConfiguration = Field(
default_factory=ObservabilityConfiguration.from_environment,
title="Observability configuration",
description="OpenTelemetry and observability configuration collected "
"from OTEL_* environment variables.",
)

deployment_environment: str = Field(
"development",
title="Deployment environment",
Expand Down
82 changes: 82 additions & 0 deletions tests/integration/endpoints/test_config_integration.py
Original file line number Diff line number Diff line change
Expand Up @@ -88,3 +88,85 @@ async def test_config_endpoint_fails_without_configuration(
assert "response" in exc_info.value.detail
detail = cast(dict[str, str], exc_info.value.detail)
assert "configuration is not loaded" in detail["response"].lower()


@pytest.mark.asyncio
async def test_config_endpoint_includes_observability(
current_config: AppConfig, # pylint: disable=unused-argument
test_request: Request,
test_auth: AuthTuple,
) -> None:
"""Test that config endpoint includes observability configuration.

This integration test verifies:
- Observability field is present in the configuration response
- OTEL environment variables are correctly collected
- Response structure includes observability.otel block

Parameters:
----------
current_config (AppConfig): Loads root configuration
test_request (Request): FastAPI request
test_auth (AuthTuple): noop authentication tuple
"""
response = await config_endpoint_handler(auth=test_auth, request=test_request)

# Verify observability field exists
assert hasattr(response.configuration, "observability")
assert response.configuration.observability is not None

# Verify otel field exists
assert hasattr(response.configuration.observability, "otel")
assert isinstance(response.configuration.observability.otel, dict)


@pytest.mark.asyncio
async def test_config_endpoint_observability_collects_otel_vars(
current_config: AppConfig,
test_request: Request,
test_auth: AuthTuple,
monkeypatch: pytest.MonkeyPatch,
) -> None:
"""Test that observability config collects OTEL_* environment variables.

This integration test verifies the full endpoint integration:
- OTEL_* environment variables are collected into observability.otel
- Configuration reload picks up new environment variables
- Endpoint response includes the updated observability configuration

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 pathlib import Path

# 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
# The observability field is populated via from_environment() during config load
config_path = Path(__file__).parent.parent.parent.parent / "lightspeed-stack.yaml"
current_config.load_configuration(str(config_path))

# Call the actual endpoint handler to test full integration
response = await config_endpoint_handler(auth=test_auth, request=test_request)

# Verify that the endpoint response includes observability configuration
assert hasattr(response.configuration, "observability")
assert response.configuration.observability is not None
assert hasattr(response.configuration.observability, "otel")

# Verify OTEL vars are present in the endpoint response
otel_config = response.configuration.observability.otel
assert "OTEL_SDK_DISABLED" in otel_config
assert otel_config["OTEL_SDK_DISABLED"] == "true"
assert "OTEL_SERVICE_NAME" in otel_config
assert otel_config["OTEL_SERVICE_NAME"] == "test-service"
assert "OTEL_EXPORTER_OTLP_ENDPOINT" in otel_config
assert otel_config["OTEL_EXPORTER_OTLP_ENDPOINT"] == "http://localhost:4317"
32 changes: 32 additions & 0 deletions tests/unit/models/config/test_dump_configuration.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,11 @@
# pyright: reportCallIssue=false

import json
import os
from pathlib import Path
from typing import Any

import pytest
from pydantic import SecretStr

import constants
Expand Down Expand Up @@ -52,6 +54,26 @@
}


@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": {}}
Comment on lines +57 to +74

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 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.

Suggested change
@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



def test_dump_configuration_minimal_cfg(tmp_path: Path) -> None:
"""
Test that the Configuration object can be serialized to a JSON file and
Expand Down Expand Up @@ -232,6 +254,7 @@ def test_dump_configuration_minimal_cfg(tmp_path: Path) -> None:
"quota_subject": None,
},
"splunk": None,
"observability": _get_expected_observability_dump(),
"deployment_environment": "development",
"reranker": {
"enabled": False,
Expand Down Expand Up @@ -459,6 +482,7 @@ def test_dump_configuration_valid_values(tmp_path: Path) -> None:
"quota_subject": None,
},
"splunk": None,
"observability": _get_expected_observability_dump(),
"deployment_environment": "development",
"reranker": {
"enabled": False,
Expand Down Expand Up @@ -837,6 +861,7 @@ def test_dump_configuration_with_quota_limiters(tmp_path: Path) -> None:
"quota_subject": None,
},
"splunk": None,
"observability": _get_expected_observability_dump(),
"deployment_environment": "development",
"reranker": {
"enabled": False,
Expand Down Expand Up @@ -1099,6 +1124,7 @@ def test_dump_configuration_with_quota_limiters_different_values(
"quota_subject": None,
},
"splunk": None,
"observability": _get_expected_observability_dump(),
"deployment_environment": "development",
"reranker": {
"enabled": False,
Expand Down Expand Up @@ -1394,6 +1420,7 @@ def test_dump_configuration_byok(tmp_path: Path) -> None:
"quota_subject": None,
},
"splunk": None,
"observability": _get_expected_observability_dump(),
"deployment_environment": "development",
"reranker": {
"enabled": False,
Expand Down Expand Up @@ -1616,6 +1643,7 @@ def test_dump_configuration_pg_namespace(tmp_path: Path) -> None:
"quota_subject": None,
},
"splunk": None,
"observability": _get_expected_observability_dump(),
"deployment_environment": "development",
"reranker": {
"enabled": False,
Expand Down Expand Up @@ -1998,6 +2026,7 @@ def test_dump_configuration_allow_degraded_mode(tmp_path: Path) -> None:
"quota_subject": None,
},
"splunk": None,
"observability": _get_expected_observability_dump(),
"deployment_environment": "development",
"reranker": {
"enabled": False,
Expand Down Expand Up @@ -2226,6 +2255,7 @@ def test_dump_configuration_max_retries_settings(tmp_path: Path) -> None:
"quota_subject": None,
},
"splunk": None,
"observability": _get_expected_observability_dump(),
"deployment_environment": "development",
"reranker": {
"enabled": False,
Expand Down Expand Up @@ -2454,6 +2484,7 @@ def test_dump_configuration_retry_count_settings(tmp_path: Path) -> None:
"quota_subject": None,
},
"splunk": None,
"observability": _get_expected_observability_dump(),
"deployment_environment": "development",
"reranker": {
"enabled": False,
Expand Down Expand Up @@ -2689,6 +2720,7 @@ def test_dump_configuration_specific_compaction_values(tmp_path: Path) -> None:
"quota_subject": None,
},
"splunk": None,
"observability": _get_expected_observability_dump(),
"deployment_environment": "development",
"reranker": {
"enabled": False,
Expand Down
Loading
Loading