Skip to content

OLS-2025: Collect config on startup #417

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
onmete wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

OLS-2025: Collect config on startup #417

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
476e064
Collect config on startup
onmete Aug 19, 2025
c063a63
Update the data collection docs
onmete Aug 19, 2025
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
10 changes: 10 additions & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,8 @@ user_data_collection:
feedback_storage: "/tmp/data/feedback"
transcripts_enabled: true
transcripts_storage: "/tmp/data/transcripts"
config_enabled: true
config_storage: "/tmp/data/config"
```

### MCP Server and Tool Configuration
Expand Down Expand Up @@ -241,6 +243,8 @@ user_data_collection:
feedback_storage: "/tmp/data/feedback"
transcripts_enabled: true
transcripts_storage: "/tmp/data/transcripts"
config_enabled: true
config_storage: "/tmp/data/config"
```

## User data collection
Expand All @@ -255,6 +259,8 @@ user_data_collection:
feedback_storage: "/tmp/data/feedback"
transcripts_enabled: true
transcripts_storage: "/tmp/data/transcripts"
config_enabled: true
config_storage: "/tmp/data/config"
```

**Configuration options:**
Expand All @@ -263,6 +269,8 @@ user_data_collection:
- `feedback_storage`: Directory path where feedback JSON files are stored
- `transcripts_enabled`: Enable/disable collection of conversation transcripts
- `transcripts_storage`: Directory path where transcript JSON files are stored
- `config_enabled`: Enable/disable collection of service configuration at startup
- `config_storage`: Directory path where configuration JSON files are stored

> **Note**: The data collection system is designed to be extensible. Additional data types can be configured and collected as needed for your specific use case.

Expand Down Expand Up @@ -650,6 +658,8 @@ The Lightspeed Core Stack integrates with the [lightspeed-to-dataverse-exporter]
feedback_storage: "/shared/data/feedback"
transcripts_enabled: true
transcripts_storage: "/shared/data/transcripts"
config_enabled: true
config_storage: "/shared/data/config"
```

2. **Deploy the exporter service** pointing to the same data directories
Expand Down
4 changes: 4 additions & 0 deletions docs/deployment_guide.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1099,6 +1099,8 @@ user_data_collection:
feedback_storage: "/tmp/data/feedback"
transcripts_enabled: true
transcripts_storage: "/tmp/data/transcripts"
config_enabled: true
config_storage: "/tmp/data/config"

authentication:
module: "noop"
Expand Down Expand Up @@ -1254,6 +1256,8 @@ user_data_collection:
feedback_storage: "/tmp/data/feedback"
transcripts_enabled: true
transcripts_storage: "/tmp/data/transcripts"
config_enabled: true
config_storage: "/tmp/data/config"

authentication:
module: "noop"
Expand Down
2 changes: 2 additions & 0 deletions docs/getting_started.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -264,6 +264,8 @@ user_data_collection:
feedback_storage: "/tmp/data/feedback"
transcripts_enabled: true
transcripts_storage: "/tmp/data/transcripts"
config_enabled: true
config_storage: "/tmp/data/config"

authentication:
module: "noop"
Expand Down
2 changes: 2 additions & 0 deletions examples/lightspeed-stack-lls-external.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ user_data_collection:
feedback_storage: "/tmp/data/feedback"
transcripts_enabled: true
transcripts_storage: "/tmp/data/transcripts"
config_enabled: true
config_storage: "/tmp/data/config"
data_collector:
enabled: false
ingress_server_url: null
Expand Down
2 changes: 2 additions & 0 deletions examples/lightspeed-stack-lls-library.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ user_data_collection:
feedback_storage: "/tmp/data/feedback"
transcripts_enabled: true
transcripts_storage: "/tmp/data/transcripts"
config_enabled: true
config_storage: "/tmp/data/config"
data_collector:
enabled: false
ingress_server_url: null
Expand Down
2 changes: 2 additions & 0 deletions lightspeed-stack.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ user_data_collection:
feedback_storage: "/tmp/data/feedback"
transcripts_enabled: true
transcripts_storage: "/tmp/data/transcripts"
config_enabled: true
config_storage: "/tmp/data/config"
Comment on lines +23 to +24
Copy link
Contributor

@coderabbitai coderabbitai bot Aug 19, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Do not enable config collection by default in shipped config (privacy/security risk).

Having config_enabled: true in the default config may persist raw configuration on first run, potentially capturing secrets, API keys, or internal file paths without explicit user consent. Recommend defaulting to false here and letting docs/examples instruct how to enable.

Apply this diff:

-  config_enabled: true
+  config_enabled: false
   config_storage: "/tmp/data/config"
🤖 Prompt for AI Agents 
In lightspeed-stack.yaml around lines 23 to 24, the default config_enabled is
set to true; change it to false to avoid enabling config collection by default,
leaving config_storage as-is; also update any docs/examples to show how to
explicitly enable config collection (and mention user consent and any migration
steps for existing installs) so users opt in rather than having it enabled by
default.

Copy link
Contributor Author

@onmete onmete Aug 19, 2025

Choose a reason for hiding this comment

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

Hmm, this is actually a good comment. In OLS, the config doesn't contain any secrets (if it does, it is user's fault and we are fine collecting it in this case), but the lightspeed-stack.yaml contains api_key. So I'll revisit the config and do some extra redaction.
cc @tisnik

Copy link
Contributor

@coderabbitai coderabbitai bot Aug 19, 2025

Choose a reason for hiding this comment

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

Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!


authentication:
module: "noop"
2 changes: 2 additions & 0 deletions src/app/endpoints/config.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@
"feedback_storage": "/tmp/data/feedback",
"transcripts_enabled": False,
"transcripts_storage": None,
"config_enabled": True,
"config_storage": "/tmp/data/config",
},
"mcp_servers": [
{"name": "server1", "provider_id": "provider1", "url": "http://url.com:1"},
Expand Down
49 changes: 49 additions & 0 deletions src/lightspeed_stack.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,18 @@

from argparse import ArgumentParser
import asyncio
import json
import logging
from datetime import datetime, timezone
from pathlib import Path
from rich.logging import RichHandler


from runners.uvicorn import start_uvicorn
from configuration import configuration
from client import AsyncLlamaStackClientHolder
from utils import suid
import version

FORMAT = "%(message)s"
logging.basicConfig(
Expand All @@ -21,6 +27,42 @@
logger = logging.getLogger(__name__)


def store_config(cfg_file: str) -> None:
"""Store service configuration in the local filesystem.

This function stores the original configuration file content once at startup.
Since the configuration is immutable for a single service deployment,
this avoids duplicating the same config data in every transcript/feedback.

Args:
cfg_file: Path to the original configuration file.
"""
with open(cfg_file, "r", encoding="utf-8") as f:
config_content = f.read()

data_to_store = {
"metadata": {
"timestamp": datetime.now(timezone.utc).isoformat(),
"service_version": version.__version__,
"config_file_path": cfg_file,
},
"configuration": config_content,
Copy link
Contributor

@umago umago Aug 20, 2025

Choose a reason for hiding this comment

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

Another concern I have reading this, is that we do not redact anything from the configuration file. These files may include passwords [0][1], API keys [2], etc...

I'm not sure clients will want to send that anywhere. And even those storing this kind of content might have to deal with legal issues/procedures in order to do so.

[0]

lightspeed-stack/src/models/config.py

Line 19 in 45eb299

tls_key_password: Optional[FilePath] = None

[1]

lightspeed-stack/src/models/config.py

Line 64 in 45eb299

password: str

[2]

lightspeed-stack/src/models/config.py

Line 151 in 45eb299

api_key: Optional[str] = None

Copy link
Contributor Author

@onmete onmete Aug 20, 2025

Choose a reason for hiding this comment

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

Yup - see previous comment: #417 (comment)
I'll have a look at the redaction of these fields.

}

# Store the data in the local filesystem
config_storage = configuration.user_data_collection_configuration.config_storage
if config_storage is None:
raise ValueError("config_storage must be set when config collection is enabled")
storage_path = Path(config_storage)
storage_path.mkdir(parents=True, exist_ok=True)
config_file_path = storage_path / f"{suid.get_suid()}.json"

with open(config_file_path, "w", encoding="utf-8") as config_file:
json.dump(data_to_store, config_file, indent=2)

logger.info("Service configuration stored in '%s'", config_file_path)


def create_argument_parser() -> ArgumentParser:
"""Create and configure argument parser object."""
parser = ArgumentParser()
Expand Down Expand Up @@ -62,6 +104,13 @@ def main() -> None:
logger.info(
"Llama stack configuration: %s", configuration.llama_stack_configuration
)

# store service configuration if enabled
if configuration.user_data_collection_configuration.config_enabled:
store_config(args.config_file)
else:
logger.debug("Config collection is disabled in configuration")

logger.info("Creating AsyncLlamaStackClient")
asyncio.run(
AsyncLlamaStackClientHolder().load(configuration.configuration.llama_stack)
Expand Down
6 changes: 6 additions & 0 deletions src/models/config.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,8 @@ class UserDataCollection(BaseModel):
feedback_storage: Optional[str] = None
transcripts_enabled: bool = False
transcripts_storage: Optional[str] = None
config_enabled: bool = False
config_storage: Optional[str] = None

@model_validator(mode="after")
def check_storage_location_is_set_when_needed(self) -> Self:
Expand All @@ -215,6 +217,10 @@ def check_storage_location_is_set_when_needed(self) -> Self:
raise ValueError(
"transcripts_storage is required when transcripts is enabled"
)
if self.config_enabled and self.config_storage is None:
raise ValueError(
"config_storage is required when config collection is enabled"
)
return self


Expand Down
4 changes: 4 additions & 0 deletions tests/configuration/lightspeed-stack.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,10 @@ llama_stack:
user_data_collection:
feedback_enabled: true
feedback_storage: "/tmp/data/feedback"
transcripts_enabled: true
transcripts_storage: "/tmp/data/transcripts"
config_enabled: true
config_storage: "/tmp/data/config"
mcp_servers:
- name: "server1"
provider_id: "provider1"
Expand Down
2 changes: 2 additions & 0 deletions tests/unit/models/test_config.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -598,6 +598,8 @@ def test_dump_configuration(tmp_path) -> None:
"feedback_storage": None,
"transcripts_enabled": False,
"transcripts_storage": None,
"config_enabled": False,
"config_storage": None,
},
"mcp_servers": [],
"authentication": {
Expand Down
Loading