diff --git a/.github/workflows/bump-version.yml b/.github/workflows/bump-version.yml index 6f7ba2b..216fc1e 100644 --- a/.github/workflows/bump-version.yml +++ b/.github/workflows/bump-version.yml @@ -20,7 +20,7 @@ jobs: fetch-depth: 0 token: "${{ secrets.PERSONAL_ACCESS_TOKEN }}" - name: Create bump and changelog - uses: commitizen-tools/commitizen-action@5b0848cd060263e24602d1eba03710e056ef7711 # 0.24.0 + uses: commitizen-tools/commitizen-action@9615e7be1cf341393c52e865ebbdaa0712176d81 # 0.25.0 with: github_token: ${{ secrets.PERSONAL_ACCESS_TOKEN }} changelog_increment_filename: body.md diff --git a/.gitmodules b/.gitmodules index 0bbfcae..e70e53a 100644 --- a/.gitmodules +++ b/.gitmodules @@ -4,6 +4,3 @@ [submodule "third_party/oidc"] path = third_party/oidc url = https://github.com/cbcoutinho/oidc -[submodule "third_party/notes"] - path = third_party/notes - url = https://github.com/cbcoutinho/notes diff --git a/CHANGELOG.md b/CHANGELOG.md index 388be68..0b5d6b4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,43 @@ +## v0.39.0 (2025-11-16) + +### Feat + +- Implement BM25 hybrid search with native Qdrant RRF fusion + +### Fix + +- Handle named vectors in visualization and semantic search +- Update vizApp to use bm25_hybrid algorithm and remove deprecated weights +- Update viz routes to use BM25 hybrid search after refactor + +## v0.38.0 (2025-11-16) + +### Feat + +- add concurrent uploads and --force flag to upload command +- implement RAG evaluation framework with CLI tooling + +### Fix + +- download qrels from BEIR ZIP instead of HuggingFace + +### Refactor + +- migrate asyncio to anyio for consistent structured concurrency +- replace httpx client with NextcloudClient in upload command + +### Perf + +- Eliminate double-fetching in semantic search sampling +- fix vector viz search performance and visual encoding +- make note deletion concurrent in upload --force + +## v0.37.0 (2025-11-16) + +### Feat + +- Add OpenTelemetry tracing to @instrument_tool decorator + ## v0.36.0 (2025-11-15) ### BREAKING CHANGE diff --git a/Dockerfile b/Dockerfile index df7cca7..5fdbf58 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,4 +1,3 @@ -#FROM ghcr.io/astral-sh/uv:0.9.9-python3.11-bookworm-slim FROM python:3.12-slim-trixie COPY --from=ghcr.io/astral-sh/uv:latest /uv /uvx /bin/ diff --git a/app-hooks/post-installation/10-install-notes-app.sh b/app-hooks/post-installation/10-install-notes-app.sh index 09d99c9..8704e39 100755 --- a/app-hooks/post-installation/10-install-notes-app.sh +++ b/app-hooks/post-installation/10-install-notes-app.sh @@ -2,30 +2,4 @@ set -euox pipefail -echo "Installing and configuring notes app for testing..." - -# Check if development notes app is mounted at /opt/apps/notes -if [ -d /opt/apps/notes ]; then - echo "Development notes app found at /opt/apps/notes" - - # Remove any existing notes app in apps (from app store or old symlink) - if [ -e /var/www/html/apps/notes ]; then - echo "Removing existing notes in apps..." - rm -rf /var/www/html/apps/notes - fi - - # Create symlink from apps to the mounted development version - # Per Nextcloud docs: apps outside server root need symlinks in server root - echo "Creating symlink: apps/notes -> /opt/apps/notes" - ln -sf /opt/apps/notes /var/www/html/apps/notes - - echo "Enabling notes app from /opt/apps (development mode via symlink)" - php /var/www/html/occ app:enable notes -elif [ -d /var/www/html/apps/notes ]; then - echo "notes app directory found in apps (already installed)" - php /var/www/html/occ app:enable notes -else - echo "notes app not found, installing from app store..." - php /var/www/html/occ app:install notes - php /var/www/html/occ app:enable notes -fi +php /var/www/html/occ app:enable notes diff --git a/charts/nextcloud-mcp-server/Chart.yaml b/charts/nextcloud-mcp-server/Chart.yaml index a16ad76..ee15cfa 100644 --- a/charts/nextcloud-mcp-server/Chart.yaml +++ b/charts/nextcloud-mcp-server/Chart.yaml @@ -2,8 +2,8 @@ apiVersion: v2 name: nextcloud-mcp-server description: A Helm chart for Nextcloud MCP Server - enables AI assistants to interact with Nextcloud type: application -version: 0.36.0 -appVersion: "0.36.0" +version: 0.39.0 +appVersion: "0.39.0" keywords: - nextcloud - mcp diff --git a/docker-compose.yml b/docker-compose.yml index 33a07a0..4669033 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -242,17 +242,6 @@ services: profiles: - qdrant - open-webui: - image: ghcr.io/open-webui/open-webui:main - environment: - - OLLAMA_BASE_URL=https://ollama.internal.coutinho.io - ports: - - 127.0.0.1:3000:8080 - volumes: - - open-webui:/app/backend/data - profiles: - - open-webui - volumes: nextcloud: db: @@ -262,4 +251,3 @@ volumes: keycloak-oauth-storage: qdrant-data: mcp-data: - open-webui: diff --git a/docs/observability.md b/docs/observability.md index 17f49f5..5975781 100644 --- a/docs/observability.md +++ b/docs/observability.md @@ -243,7 +243,7 @@ If you see cardinality warnings: The observability stack integrates at multiple layers: 1. **HTTP Layer**: `ObservabilityMiddleware` tracks all HTTP requests -2. **MCP Layer**: Tools use `@trace_mcp_tool` for span creation +2. **MCP Layer**: Tools use `@instrument_tool` for automatic metrics and trace span creation 3. **Client Layer**: `BaseNextcloudClient` tracks all API calls 4. **OAuth Layer**: Token operations are traced and metered 5. **Background Tasks**: Vector sync operations emit metrics/traces diff --git a/nextcloud_mcp_server/observability/metrics.py b/nextcloud_mcp_server/observability/metrics.py index e97596f..610a22d 100644 --- a/nextcloud_mcp_server/observability/metrics.py +++ b/nextcloud_mcp_server/observability/metrics.py @@ -404,10 +404,11 @@ def update_vector_sync_queue_size(size: int) -> None: def instrument_tool(func): """ - Decorator to automatically instrument MCP tool functions with metrics. + Decorator to automatically instrument MCP tool functions with metrics and tracing. - Wraps async tool functions to record execution time and success/error status. - Compatible with @mcp.tool() and @require_scopes() decorators. + Wraps async tool functions to record execution time, success/error status, and + create OpenTelemetry trace spans. Compatible with @mcp.tool() and @require_scopes() + decorators. Usage: @mcp.tool() @@ -420,24 +421,46 @@ def instrument_tool(func): func: The async function to instrument Returns: - Wrapped function with metrics instrumentation + Wrapped function with metrics and tracing instrumentation """ import functools import time + from nextcloud_mcp_server.observability.tracing import trace_operation + @functools.wraps(func) async def wrapper(*args, **kwargs): tool_name = func.__name__ start_time = time.time() - try: - result = await func(*args, **kwargs) - duration = time.time() - start_time - record_tool_call(tool_name, duration, "success") - return result - except Exception as e: - duration = time.time() - start_time - record_tool_call(tool_name, duration, "error") - record_tool_error(tool_name, type(e).__name__) - raise + + # Extract tool arguments for tracing (sanitize sensitive fields) + # kwargs contains the actual arguments passed to the tool + tool_args = { + k: v + for k, v in kwargs.items() + if k not in ("password", "token", "secret", "api_key", "etag", "ctx") + } + + # Create trace span with metrics collection + with trace_operation( + f"mcp.tool.{tool_name}", + attributes={ + "mcp.tool.name": tool_name, + "mcp.tool.args": str(tool_args)[:500] + if tool_args + else None, # Limit to 500 chars + }, + record_exception=True, + ): + try: + result = await func(*args, **kwargs) + duration = time.time() - start_time + record_tool_call(tool_name, duration, "success") + return result + except Exception as e: + duration = time.time() - start_time + record_tool_call(tool_name, duration, "error") + record_tool_error(tool_name, type(e).__name__) + raise return wrapper diff --git a/pyproject.toml b/pyproject.toml index ca0c48e..3fe8407 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "nextcloud-mcp-server" -version = "0.36.0" +version = "0.39.0" description = "Model Context Protocol (MCP) server for Nextcloud integration - enables AI assistants to interact with Nextcloud data" authors = [ {name = "Chris Coutinho", email = "chris@coutinho.io"} diff --git a/tests/unit/test_instrument_tool.py b/tests/unit/test_instrument_tool.py new file mode 100644 index 0000000..d7abb1a --- /dev/null +++ b/tests/unit/test_instrument_tool.py @@ -0,0 +1,217 @@ +""" +Unit tests for @instrument_tool decorator. + +Tests that the decorator correctly instruments MCP tools with both +Prometheus metrics and OpenTelemetry tracing. +""" + +from unittest.mock import MagicMock, patch + +import pytest + +from nextcloud_mcp_server.observability.metrics import instrument_tool + +pytestmark = pytest.mark.unit + + +@pytest.fixture +def mock_metrics(): + """Mock Prometheus metrics.""" + with ( + patch( + "nextcloud_mcp_server.observability.metrics.record_tool_call" + ) as mock_record, + patch( + "nextcloud_mcp_server.observability.metrics.record_tool_error" + ) as mock_error, + ): + yield {"record_tool_call": mock_record, "record_tool_error": mock_error} + + +@pytest.fixture +def mock_tracer(): + """Mock OpenTelemetry tracer.""" + with patch( + "nextcloud_mcp_server.observability.tracing.trace_operation" + ) as mock_trace: + # Configure mock to act as a context manager that allows exceptions to propagate + mock_trace.return_value.__enter__ = MagicMock(return_value=None) + mock_trace.return_value.__exit__ = MagicMock( + return_value=False + ) # Return False to allow exceptions to propagate + yield mock_trace + + +class TestInstrumentToolDecorator: + """Test the @instrument_tool decorator.""" + + async def test_decorator_creates_trace_span(self, mock_tracer, mock_metrics): + """Test that decorator creates OpenTelemetry span with correct attributes.""" + + @instrument_tool + async def example_tool(query: str, limit: int = 10): + return {"results": []} + + # Call the tool + await example_tool(query="test query", limit=5) + + # Verify trace_operation was called with correct parameters + mock_tracer.assert_called_once() + call_args = mock_tracer.call_args + + # Check span name + assert call_args[0][0] == "mcp.tool.example_tool" + + # Check span attributes + attributes = call_args[1]["attributes"] + assert attributes["mcp.tool.name"] == "example_tool" + assert "query" in attributes["mcp.tool.args"] + assert "test query" in attributes["mcp.tool.args"] + assert "limit" in attributes["mcp.tool.args"] + + # Verify record_exception parameter + assert call_args[1]["record_exception"] is True + + async def test_decorator_sanitizes_sensitive_arguments( + self, mock_tracer, mock_metrics + ): + """Test that sensitive arguments are excluded from span attributes.""" + + @instrument_tool + async def example_tool( + query: str, password: str, token: str, api_key: str, ctx: object + ): + return {"success": True} + + # Call with sensitive parameters + await example_tool( + query="test", + password="secret123", + token="bearer_token", + api_key="api_key_123", + ctx=MagicMock(), + ) + + # Verify trace was created + mock_tracer.assert_called_once() + attributes = mock_tracer.call_args[1]["attributes"] + + # Check that sensitive fields are NOT in attributes + tool_args = attributes["mcp.tool.args"] + assert "password" not in tool_args + assert "secret123" not in tool_args + assert "token" not in tool_args + assert "bearer_token" not in tool_args + assert "api_key" not in tool_args + assert "api_key_123" not in tool_args + assert "ctx" not in tool_args + + # Check that non-sensitive field IS included + assert "query" in tool_args + assert "test" in tool_args + + async def test_decorator_limits_argument_string_length( + self, mock_tracer, mock_metrics + ): + """Test that tool arguments are limited to 500 characters.""" + + @instrument_tool + async def example_tool(query: str): + return {"results": []} + + # Create a very long query string (>500 chars) + long_query = "x" * 1000 + + await example_tool(query=long_query) + + # Verify arguments were truncated + mock_tracer.assert_called_once() + attributes = mock_tracer.call_args[1]["attributes"] + tool_args = attributes["mcp.tool.args"] + + assert len(tool_args) <= 500 + + async def test_decorator_records_success_metrics(self, mock_tracer, mock_metrics): + """Test that successful tool execution records metrics.""" + + @instrument_tool + async def example_tool(): + return {"success": True} + + # Call the tool + await example_tool() + + # Verify success metrics were recorded + mock_metrics["record_tool_call"].assert_called_once() + call_args = mock_metrics["record_tool_call"].call_args + assert call_args[0][0] == "example_tool" # tool_name + assert isinstance(call_args[0][1], float) # duration + assert call_args[0][2] == "success" # status + + async def test_decorator_records_error_metrics(self, mock_tracer, mock_metrics): + """Test that tool errors are recorded in metrics.""" + + @instrument_tool + async def failing_tool(): + raise ValueError("Test error") + + # Call the tool and expect exception + with pytest.raises(ValueError, match="Test error"): + await failing_tool() + + # Verify error metrics were recorded + mock_metrics["record_tool_call"].assert_called_once() + call_args = mock_metrics["record_tool_call"].call_args + assert call_args[0][0] == "failing_tool" # tool_name + assert isinstance(call_args[0][1], float) # duration + assert call_args[0][2] == "error" # status + + # Verify error type was recorded + mock_metrics["record_tool_error"].assert_called_once() + error_args = mock_metrics["record_tool_error"].call_args + assert error_args[0][0] == "failing_tool" # tool_name + assert error_args[0][1] == "ValueError" # error_type + + async def test_decorator_preserves_function_metadata( + self, mock_tracer, mock_metrics + ): + """Test that decorator preserves function name and docstring.""" + + @instrument_tool + async def example_tool(): + """This is a test tool.""" + return {"success": True} + + # Verify function metadata is preserved + assert example_tool.__name__ == "example_tool" + assert example_tool.__doc__ == "This is a test tool." + + async def test_decorator_preserves_return_value(self, mock_tracer, mock_metrics): + """Test that decorator returns the original function's return value.""" + + @instrument_tool + async def example_tool(value: int): + return {"result": value * 2} + + # Call the tool + result = await example_tool(value=5) + + # Verify return value is unchanged + assert result == {"result": 10} + + async def test_decorator_with_no_arguments(self, mock_tracer, mock_metrics): + """Test decorator with tool that takes no arguments.""" + + @instrument_tool + async def no_args_tool(): + return {"status": "ok"} + + # Call the tool + await no_args_tool() + + # Verify tracing works with no arguments + mock_tracer.assert_called_once() + attributes = mock_tracer.call_args[1]["attributes"] + + # tool_args should be None when there are no kwargs + assert attributes["mcp.tool.args"] is None diff --git a/third_party/notes b/third_party/notes deleted file mode 160000 index 735f06b..0000000 --- a/third_party/notes +++ /dev/null @@ -1 +0,0 @@ -Subproject commit 735f06b393f3f7f09eaf423a19ebcf47eb9ae5ea diff --git a/third_party/oidc b/third_party/oidc index e83dabb..9616294 160000 --- a/third_party/oidc +++ b/third_party/oidc @@ -1 +1 @@ -Subproject commit e83dabbac17142f7ece6219e5b33ca145fc99ae6 +Subproject commit 96162949117d9325e45e06acd3bbdd0fdb20450c diff --git a/uv.lock b/uv.lock index b688318..2f35b19 100644 --- a/uv.lock +++ b/uv.lock @@ -1857,7 +1857,7 @@ wheels = [ [[package]] name = "nextcloud-mcp-server" -version = "0.36.0" +version = "0.39.0" source = { editable = "." } dependencies = [ { name = "aiosqlite" },