Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
bfec4f3
Add data collector configuration and integrate service startup/shutdown
jrobertboos Jul 3, 2025
c717127
Add data collector service and update Makefile
jrobertboos Jul 7, 2025
dba0c08
Enhance data collector configuration and update related services
jrobertboos Jul 7, 2025
da036c0
Add types-requests to development dependencies
jrobertboos Jul 7, 2025
95ab581
Enhance unit tests for DataCollectorService
jrobertboos Jul 7, 2025
acdaa3b
Refactor DataCollectorConfiguration and enhance error handling
jrobertboos Jul 9, 2025
751d103
Add data collector constants and refactor configuration model
jrobertboos Jul 10, 2025
98fed29
Add ingress content service name to data collector configuration
jrobertboos Jul 14, 2025
4887e2a
Add data collector configuration and integrate service startup/shutdown
jrobertboos Jul 3, 2025
0c7c878
Add data collector service and update Makefile
jrobertboos Jul 7, 2025
cdf7907
Enhance data collector configuration and update related services
jrobertboos Jul 7, 2025
773374c
Add types-requests to development dependencies
jrobertboos Jul 7, 2025
e33a51b
Enhance unit tests for DataCollectorService
jrobertboos Jul 7, 2025
51ef7f4
Refactor DataCollectorConfiguration and enhance error handling
jrobertboos Jul 9, 2025
b866386
Add data collector constants and refactor configuration model
jrobertboos Jul 10, 2025
2e8d59a
Add ingress content service name to data collector configuration
jrobertboos Jul 14, 2025
636b6c9
Merge branch 'lcore-302' of github.com:jrobertboos/lightspeed-stack i…
jrobertboos Jul 14, 2025
f8becbf
Update package versions in lock file and clean up config imports
jrobertboos Jul 14, 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
3 changes: 3 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@ PYTHON_REGISTRY = pypi
run: ## Run the service locally
uv run src/lightspeed_stack.py

run-data-collector: ## Run the data collector service locally
uv run src/lightspeed_stack.py --data-collector

test-unit: ## Run the unit tests
@echo "Running unit tests..."
@echo "Reports will be written to ${ARTIFACT_DIR}"
Expand Down
44 changes: 44 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,7 @@ Usage: make <OPTIONS> ... <TARGETS>
Available targets are:

run Run the service locally
run-data-collector Run the data collector service
test-unit Run the unit tests
test-integration Run integration tests tests
test-e2e Run BDD tests for the service
Expand Down Expand Up @@ -308,3 +309,46 @@ This script re-generated OpenAPI schema for the Lightspeed Service REST API.
make schema
```

## Data Collector Service

The data collector service is a standalone service that runs separately from the main web service. It is responsible for collecting and sending user data including feedback and transcripts to an ingress server for analysis and archival.

### Features

- **Periodic Collection**: Runs at configurable intervals
- **Data Packaging**: Packages feedback and transcript files into compressed tar.gz archives
- **Secure Transmission**: Sends data to a configured ingress server with optional authentication
- **File Cleanup**: Optionally removes local files after successful transmission
- **Error Handling**: Includes retry logic and comprehensive error handling

### Configuration

The data collector service is configured through the `user_data_collection.data_collector` section in your configuration file:

```yaml
user_data_collection:
feedback_disabled: false
feedback_storage: "/tmp/data/feedback"
transcripts_disabled: false
transcripts_storage: "/tmp/data/transcripts"
data_collector:
enabled: true
ingress_server_url: "https://your-ingress-server.com"
ingress_server_auth_token: "your-auth-token"
ingress_content_service_name: "lightspeed-team"
collection_interval: 7200 # 2 hours in seconds
cleanup_after_send: true
connection_timeout: 30
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't it be a separate root-level configuration? Like

user_data_collection:
  feedback_disabled: false
  feedback_storage: "/tmp/data/feedback"
  transcripts_disabled: false
  transcripts_storage: "/tmp/data/transcripts"
user_data_export:
  enabled: true
  ingress_server_url: "https://your-ingress-server.com"
  ingress_server_auth_token: "your-auth-token"
  ingress_content_service_name: "lightspeed-team"
  collection_interval: 7200  # 2 hours in seconds
  cleanup_after_send: true
  connection_timeout: 30

As those are two independent pieces.

```

### Running the Service

To run the data collector service:

```bash
# Using Python directly
uv run src/lightspeed_stack.py --data-collector

# Using Make target
make run-data-collector
```
8 changes: 8 additions & 0 deletions lightspeed-stack.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -20,5 +20,13 @@ user_data_collection:
feedback_storage: "/tmp/data/feedback"
transcripts_disabled: false
transcripts_storage: "/tmp/data/transcripts"
data_collector:
enabled: false
ingress_server_url: null
ingress_server_auth_token: null
ingress_content_service_name: null
collection_interval: 7200 # 2 hours in seconds
cleanup_after_send: true
connection_timeout_seconds: 30
Comment on lines +23 to +30
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

Configuration section is well-structured with appropriate defaults.

The new data_collector section includes all necessary parameters with sensible default values. The service is disabled by default, which is appropriate for a new feature.

However, there's a potential field name inconsistency that should be verified.

Verify the field name consistency between the YAML configuration and the model definition:

The YAML uses connection_timeout_seconds while the model likely uses connection_timeout. This inconsistency could cause configuration parsing issues.


🏁 Script executed:

#!/bin/bash
# Check if the field name in YAML matches the model definition
echo "Checking field name in YAML configuration:"
grep -n "connection_timeout" lightspeed-stack.yaml

echo -e "\nChecking field name in model definition:"
ast-grep --pattern 'connection_timeout$_: $_' src/models/config.py

Length of output: 349


🏁 Script executed:

#!/bin/bash
# Search for 'connection_timeout_seconds' in model definitions
echo "Searching for 'connection_timeout_seconds' in model definitions:"
grep -R "connection_timeout_seconds" -n src/models

echo -e "\nSearching for DataCollector class definition and its fields:"
grep -R "class DataCollector" -n src/models

Length of output: 447


#!/bin/bash
# Locate the DataCollectorConfiguration class and list its fields
grep -n "class DataCollectorConfiguration" src/models/config.py
grep -A20 "class DataCollectorConfiguration" src/models/config.py

# Search specifically for timeout-related fields
grep -n "timeout" src/models/config.py

🏁 Script executed:

#!/bin/bash
# Locate the DataCollectorConfiguration class and list its fields
grep -n "class DataCollectorConfiguration" src/models/config.py
grep -A20 "class DataCollectorConfiguration" src/models/config.py

# Search specifically for timeout-related fields within the class
sed -n '88,120p' src/models/config.py | grep -n "timeout"

Length of output: 1387


Rename connection_timeout_seconds to connection_timeout in YAML

The Pydantic model DataCollectorConfiguration defines its timeout field as connection_timeout, so the YAML key connection_timeout_seconds will not be picked up (and may raise an “extra field” error or be ignored).

Please update lightspeed-stack.yaml at line 30:

   cleanup_after_send: true
-  connection_timeout_seconds: 30
+  connection_timeout: 30

This ensures the configuration maps correctly to DataCollectorConfiguration.connection_timeout.

🤖 Prompt for AI Agents
In lightspeed-stack.yaml at line 30, rename the key `connection_timeout_seconds`
to `connection_timeout` to match the Pydantic model field name
`connection_timeout`. This change ensures the YAML configuration correctly maps
to the DataCollectorConfiguration model without causing errors or being ignored.

authentication:
module: "noop"
1 change: 1 addition & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ dev = [
"pydocstyle>=6.3.0",
"mypy>=1.16.0",
"types-PyYAML>=6.0.2",
"types-requests>=2.28.0",
"ruff>=0.11.13",
"aiosqlite",
"behave>=1.2.6",
Expand Down
5 changes: 5 additions & 0 deletions src/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,3 +42,8 @@
}
)
DEFAULT_AUTHENTICATION_MODULE = AUTH_MOD_NOOP

# Data collector constants
DATA_COLLECTOR_COLLECTION_INTERVAL = 7200 # 2 hours in seconds
DATA_COLLECTOR_CONNECTION_TIMEOUT = 30
DATA_COLLECTOR_RETRY_INTERVAL = 300 # 5 minutes in seconds
12 changes: 12 additions & 0 deletions src/lightspeed_stack.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
from rich.logging import RichHandler

from runners.uvicorn import start_uvicorn
from runners.data_collector import start_data_collector
from configuration import configuration
from client import LlamaStackClientHolder, AsyncLlamaStackClientHolder

Expand Down Expand Up @@ -47,6 +48,13 @@ def create_argument_parser() -> ArgumentParser:
help="path to configuration file (default: lightspeed-stack.yaml)",
default="lightspeed-stack.yaml",
)
parser.add_argument(
"--data-collector",
dest="start_data_collector",
help="start data collector service instead of web service",
action="store_true",
default=False,
)
return parser


Expand All @@ -70,6 +78,10 @@ def main() -> None:

if args.dump_configuration:
configuration.configuration.dump()
elif args.start_data_collector:
start_data_collector(
configuration.user_data_collection_configuration.data_collector
)
else:
start_uvicorn(configuration.service_configuration)
logger.info("Lightspeed stack finished")
Expand Down
28 changes: 27 additions & 1 deletion src/models/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

from typing import Optional

from pydantic import BaseModel, model_validator, FilePath, AnyHttpUrl
from pydantic import BaseModel, model_validator, FilePath, AnyHttpUrl, PositiveInt
from typing_extensions import Self

import constants
Expand Down Expand Up @@ -85,13 +85,39 @@ def check_llama_stack_model(self) -> Self:
return self


class DataCollectorConfiguration(BaseModel):
"""Data collector configuration for sending data to ingress server."""

enabled: bool = False
ingress_server_url: Optional[str] = None
ingress_server_auth_token: Optional[str] = None
ingress_content_service_name: Optional[str] = None
collection_interval: PositiveInt = constants.DATA_COLLECTOR_COLLECTION_INTERVAL
cleanup_after_send: bool = True # Remove local files after successful send
connection_timeout: PositiveInt = constants.DATA_COLLECTOR_CONNECTION_TIMEOUT

@model_validator(mode="after")
def check_data_collector_configuration(self) -> Self:
"""Check data collector configuration."""
if self.enabled and self.ingress_server_url is None:
raise ValueError(
"ingress_server_url is required when data collector is enabled"
)
if self.enabled and self.ingress_content_service_name is None:
raise ValueError(
"ingress_content_service_name is required when data collector is enabled"
)
return self


class UserDataCollection(BaseModel):
"""User data collection configuration."""

feedback_disabled: bool = True
feedback_storage: Optional[str] = None
transcripts_disabled: bool = True
transcripts_storage: Optional[str] = None
data_collector: DataCollectorConfiguration = DataCollectorConfiguration()

@model_validator(mode="after")
def check_storage_location_is_set_when_needed(self) -> Self:
Expand Down
26 changes: 26 additions & 0 deletions src/runners/data_collector.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
"""Data collector runner."""

import logging

from models.config import DataCollectorConfiguration
from services.data_collector import DataCollectorService

logger: logging.Logger = logging.getLogger(__name__)


def start_data_collector(configuration: DataCollectorConfiguration) -> None:
"""Start the data collector service as a standalone process."""
logger.info("Starting data collector runner")

if not configuration.enabled:
logger.info("Data collection is disabled")
return

try:
service = DataCollectorService()
service.run()
except Exception as e:
logger.error(
"Data collector service encountered an exception: %s", e, exc_info=True
)
raise
Comment on lines +19 to +26
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

Verify configuration consistency between runner and service

The runner receives a DataCollectorConfiguration parameter but from the relevant code snippets, the DataCollectorService appears to access configuration from a global configuration object (configuration.user_data_collection_configuration.data_collector). This could lead to inconsistency where the runner thinks data collection is enabled but the service has different configuration.

Consider either:

  1. Passing the configuration to the service constructor
  2. Using the same global configuration source in both places
  3. Adding validation to ensure consistency

🏁 Script executed:

#!/bin/bash
# Check how DataCollectorService accesses configuration
ast-grep --pattern 'class DataCollectorService {
  $$$
}'

# Check for configuration access patterns in the service
rg -A 10 "configuration\." src/services/data_collector.py

Length of output: 3498


I’d like to confirm how the runner receives and passes the configuration. Let’s pull the full runner code:


🏁 Script executed:

#!/bin/bash
sed -n '1,200p' src/runners/data_collector.py

Length of output: 783


Continuing verification of how configuration is passed into the service:


🏁 Script executed:

#!/bin/bash
# Show imports and top of DataCollectorService
sed -n '1,50p' src/services/data_collector.py

# Locate DataCollectorConfiguration definition
rg -n "class DataCollectorConfiguration" -A5 src/models/config.py

Length of output: 2088


Synchronize runner and service configuration sources

The start_data_collector runner takes a DataCollectorConfiguration parameter but never passes it into DataCollectorService, which always reads from the global configuration.user_data_collection_configuration.data_collector. This disconnect can cause the service to run with different settings than the runner expects.

Please update the implementation so both use the same config:

  • In src/runners/data_collector.py, pass the runner’s configuration into the service.
  • In src/services/data_collector.py, add a constructor (or method parameter) to accept and store that config instead of pulling from the global.
  • Replace all occurrences of configuration.user_data_collection_configuration.data_collector in the service with the injected config object.

Example diff:

# src/runners/data_collector.py
- service = DataCollectorService()
+ service = DataCollectorService(configuration)

# src/services/data_collector.py
-class DataCollectorService:  # pylint: disable=too-few-public-methods
+class DataCollectorService:  # pylint: disable=too-few-public-methods
+    def __init__(self, config: DataCollectorConfiguration):
+        self._collector_config = config

     def run(self) -> None:
-        collector_config = configuration.user_data_collection_configuration.data_collector
+        collector_config = self._collector_config
         logger.info("Starting data collection service")
         …
📝 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
try:
service = DataCollectorService()
service.run()
except Exception as e:
logger.error(
"Data collector service encountered an exception: %s", e, exc_info=True
)
raise
--- src/runners/data_collector.py
@@ lines 19-26
try:
- service = DataCollectorService()
+ service = DataCollectorService(configuration)
service.run()
except Exception as e:
logger.error(
"Data collector service encountered an exception: %s", e, exc_info=True
)
raise
Suggested change
try:
service = DataCollectorService()
service.run()
except Exception as e:
logger.error(
"Data collector service encountered an exception: %s", e, exc_info=True
)
raise
--- src/services/data_collector.py
@@
class DataCollectorService: # pylint: disable=too-few-public-methods
+ def __init__(self, config: DataCollectorConfiguration):
+ self._collector_config = config
def run(self) -> None:
- collector_config = configuration.user_data_collection_configuration.data_collector
+ collector_config = self._collector_config
logger.info("Starting data collection service")
🤖 Prompt for AI Agents
In src/runners/data_collector.py around lines 19 to 26, the DataCollectorService
is instantiated without passing the runner's DataCollectorConfiguration, causing
a mismatch in configuration sources. Modify the runner to pass its configuration
object to the DataCollectorService constructor. Then, in
src/services/data_collector.py, add a constructor parameter to accept this
configuration and store it as an instance variable. Replace all references to
the global configuration.user_data_collection_configuration.data_collector in
the service with this injected configuration to ensure both runner and service
use the same settings.

1 change: 1 addition & 0 deletions src/services/__init__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
"""Services package."""
Loading