chore: Remove /health and /metrics endpoints from logging
This commit is contained in:
@@ -391,3 +391,7 @@ docker compose exec app php occ user_oidc:provider keycloak
|
|||||||
- `docs/configuration.md` - Configuration options
|
- `docs/configuration.md` - Configuration options
|
||||||
- `docs/authentication.md` - Authentication modes
|
- `docs/authentication.md` - Authentication modes
|
||||||
- `docs/running.md` - Running the server
|
- `docs/running.md` - Running the server
|
||||||
|
|
||||||
|
**For additional information regarding MCP during development, see**:
|
||||||
|
- `../../Software/modelcontextprotocol/` - MCP spec
|
||||||
|
- `../../Software/python-sdk/` - Python MCP SDK
|
||||||
|
|||||||
+4
-3
@@ -107,9 +107,10 @@ services:
|
|||||||
- QDRANT_COLLECTION=nextcloud_content
|
- QDRANT_COLLECTION=nextcloud_content
|
||||||
|
|
||||||
# Ollama configuration (optional - uses SimpleEmbeddingProvider if not set)
|
# Ollama configuration (optional - uses SimpleEmbeddingProvider if not set)
|
||||||
- OLLAMA_BASE_URL=https://ollama.internal.coutinho.io:443
|
#- OLLAMA_BASE_URL=https://ollama.internal.coutinho.io:443
|
||||||
- OLLAMA_EMBEDDING_MODEL=nomic-embed-text # Changing this creates new collection
|
#- OLLAMA_EMBEDDING_MODEL=nomic-embed-text # Changing this creates new collection
|
||||||
# - OLLAMA_VERIFY_SSL=false
|
#- OLLAMA_EMBEDDING_MODEL=embeddinggemma:300m
|
||||||
|
#- OLLAMA_VERIFY_SSL=false
|
||||||
|
|
||||||
mcp-oauth:
|
mcp-oauth:
|
||||||
build: .
|
build: .
|
||||||
|
|||||||
@@ -1379,7 +1379,7 @@ def get_app(transport: str = "sse", enabled_apps: list[str] | None = None):
|
|||||||
"Routes: /user/* with SessionAuth, /mcp with FastMCP OAuth Bearer tokens"
|
"Routes: /user/* with SessionAuth, /mcp with FastMCP OAuth Bearer tokens"
|
||||||
)
|
)
|
||||||
|
|
||||||
# Add debugging middleware to log Authorization headers
|
# Add debugging middleware to log Authorization headers and client capabilities
|
||||||
@app.middleware("http")
|
@app.middleware("http")
|
||||||
async def log_auth_headers(request, call_next):
|
async def log_auth_headers(request, call_next):
|
||||||
auth_header = request.headers.get("authorization")
|
auth_header = request.headers.get("authorization")
|
||||||
@@ -1394,6 +1394,52 @@ def get_app(transport: str = "sse", enabled_apps: list[str] | None = None):
|
|||||||
logger.warning(
|
logger.warning(
|
||||||
f"⚠️ /mcp request WITHOUT Authorization header from {request.client}"
|
f"⚠️ /mcp request WITHOUT Authorization header from {request.client}"
|
||||||
)
|
)
|
||||||
|
|
||||||
|
# Log client capabilities on initialize request
|
||||||
|
if request.method == "POST":
|
||||||
|
# Read body to check for initialize request
|
||||||
|
# Starlette caches the body internally, so it's safe to read here
|
||||||
|
body = await request.body()
|
||||||
|
try:
|
||||||
|
import json
|
||||||
|
|
||||||
|
data = json.loads(body)
|
||||||
|
# Check if this is an initialize request
|
||||||
|
if data.get("method") == "initialize":
|
||||||
|
params = data.get("params", {})
|
||||||
|
capabilities = params.get("capabilities", {})
|
||||||
|
client_info = params.get("clientInfo", {})
|
||||||
|
|
||||||
|
logger.info(
|
||||||
|
f"🔌 MCP client connected: {client_info.get('name', 'unknown')} "
|
||||||
|
f"v{client_info.get('version', 'unknown')}"
|
||||||
|
)
|
||||||
|
|
||||||
|
# Log capabilities in a structured way
|
||||||
|
cap_summary = []
|
||||||
|
# Check for presence using 'in' not truthiness (empty dict {} counts as having capability)
|
||||||
|
if "roots" in capabilities:
|
||||||
|
cap_summary.append("roots")
|
||||||
|
if "sampling" in capabilities:
|
||||||
|
cap_summary.append("sampling")
|
||||||
|
if "experimental" in capabilities:
|
||||||
|
cap_summary.append(
|
||||||
|
f"experimental({len(capabilities['experimental'])} features)"
|
||||||
|
)
|
||||||
|
|
||||||
|
logger.info(
|
||||||
|
f"📋 Client capabilities: {', '.join(cap_summary) if cap_summary else 'none'}"
|
||||||
|
)
|
||||||
|
# Log full capabilities at INFO level to diagnose capability issues
|
||||||
|
logger.info(
|
||||||
|
f"Full capabilities JSON: {json.dumps(capabilities)}"
|
||||||
|
)
|
||||||
|
except Exception as e:
|
||||||
|
# Don't fail the request if logging fails
|
||||||
|
logger.debug(
|
||||||
|
f"Failed to parse MCP request for capability logging: {e}"
|
||||||
|
)
|
||||||
|
|
||||||
response = await call_next(request)
|
response = await call_next(request)
|
||||||
return response
|
return response
|
||||||
|
|
||||||
|
|||||||
@@ -17,6 +17,32 @@ from pythonjsonlogger import jsonlogger
|
|||||||
from nextcloud_mcp_server.observability.tracing import get_trace_context
|
from nextcloud_mcp_server.observability.tracing import get_trace_context
|
||||||
|
|
||||||
|
|
||||||
|
class HealthCheckFilter(logging.Filter):
|
||||||
|
"""
|
||||||
|
Logging filter that excludes health check endpoint requests.
|
||||||
|
|
||||||
|
This prevents health check polls from cluttering logs while keeping
|
||||||
|
access logs for all other endpoints.
|
||||||
|
"""
|
||||||
|
|
||||||
|
def filter(self, record: logging.LogRecord) -> bool:
|
||||||
|
"""
|
||||||
|
Filter out health check requests from uvicorn access logs.
|
||||||
|
|
||||||
|
Args:
|
||||||
|
record: LogRecord instance
|
||||||
|
|
||||||
|
Returns:
|
||||||
|
False if this is a health check request, True otherwise
|
||||||
|
"""
|
||||||
|
# Check if the log message contains health check endpoints
|
||||||
|
message = record.getMessage()
|
||||||
|
return not any(
|
||||||
|
endpoint in message
|
||||||
|
for endpoint in ["/health/live", "/health/ready", "/metrics"]
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
class TraceContextFormatter(jsonlogger.JsonFormatter):
|
class TraceContextFormatter(jsonlogger.JsonFormatter):
|
||||||
"""
|
"""
|
||||||
JSON formatter that injects OpenTelemetry trace context into log records.
|
JSON formatter that injects OpenTelemetry trace context into log records.
|
||||||
@@ -244,12 +270,23 @@ def get_uvicorn_logging_config(
|
|||||||
"datefmt": "%Y-%m-%d %H:%M:%S",
|
"datefmt": "%Y-%m-%d %H:%M:%S",
|
||||||
},
|
},
|
||||||
},
|
},
|
||||||
|
"filters": {
|
||||||
|
"health_check_filter": {
|
||||||
|
"()": "nextcloud_mcp_server.observability.logging_config.HealthCheckFilter",
|
||||||
|
},
|
||||||
|
},
|
||||||
"handlers": {
|
"handlers": {
|
||||||
"default": {
|
"default": {
|
||||||
"formatter": "default",
|
"formatter": "default",
|
||||||
"class": "logging.StreamHandler",
|
"class": "logging.StreamHandler",
|
||||||
"stream": "ext://sys.stdout",
|
"stream": "ext://sys.stdout",
|
||||||
},
|
},
|
||||||
|
"access": {
|
||||||
|
"formatter": "default",
|
||||||
|
"class": "logging.StreamHandler",
|
||||||
|
"stream": "ext://sys.stdout",
|
||||||
|
"filters": ["health_check_filter"],
|
||||||
|
},
|
||||||
},
|
},
|
||||||
"loggers": {
|
"loggers": {
|
||||||
"": {
|
"": {
|
||||||
@@ -262,7 +299,7 @@ def get_uvicorn_logging_config(
|
|||||||
"propagate": False,
|
"propagate": False,
|
||||||
},
|
},
|
||||||
"uvicorn.access": {
|
"uvicorn.access": {
|
||||||
"handlers": ["default"],
|
"handlers": ["access"],
|
||||||
"level": "INFO",
|
"level": "INFO",
|
||||||
"propagate": False,
|
"propagate": False,
|
||||||
},
|
},
|
||||||
|
|||||||
@@ -0,0 +1,88 @@
|
|||||||
|
"""Unit tests for logging filters."""
|
||||||
|
|
||||||
|
import logging
|
||||||
|
|
||||||
|
import pytest
|
||||||
|
|
||||||
|
from nextcloud_mcp_server.observability.logging_config import HealthCheckFilter
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.unit
|
||||||
|
class TestHealthCheckFilter:
|
||||||
|
"""Tests for the HealthCheckFilter."""
|
||||||
|
|
||||||
|
def test_filters_health_live_requests(self):
|
||||||
|
"""Test that /health/live requests are filtered out."""
|
||||||
|
# Create a log record that looks like a uvicorn access log for /health/live
|
||||||
|
record = logging.LogRecord(
|
||||||
|
name="uvicorn.access",
|
||||||
|
level=logging.INFO,
|
||||||
|
pathname="",
|
||||||
|
lineno=0,
|
||||||
|
msg='127.0.0.1:12345 - "GET /health/live HTTP/1.1" 200',
|
||||||
|
args=(),
|
||||||
|
exc_info=None,
|
||||||
|
)
|
||||||
|
|
||||||
|
filter_instance = HealthCheckFilter()
|
||||||
|
assert filter_instance.filter(record) is False
|
||||||
|
|
||||||
|
def test_filters_health_ready_requests(self):
|
||||||
|
"""Test that /health/ready requests are filtered out."""
|
||||||
|
record = logging.LogRecord(
|
||||||
|
name="uvicorn.access",
|
||||||
|
level=logging.INFO,
|
||||||
|
pathname="",
|
||||||
|
lineno=0,
|
||||||
|
msg='127.0.0.1:12345 - "GET /health/ready HTTP/1.1" 200',
|
||||||
|
args=(),
|
||||||
|
exc_info=None,
|
||||||
|
)
|
||||||
|
|
||||||
|
filter_instance = HealthCheckFilter()
|
||||||
|
assert filter_instance.filter(record) is False
|
||||||
|
|
||||||
|
def test_filters_metrics_requests(self):
|
||||||
|
"""Test that /metrics requests are filtered out."""
|
||||||
|
record = logging.LogRecord(
|
||||||
|
name="uvicorn.access",
|
||||||
|
level=logging.INFO,
|
||||||
|
pathname="",
|
||||||
|
lineno=0,
|
||||||
|
msg='127.0.0.1:12345 - "GET /metrics HTTP/1.1" 200',
|
||||||
|
args=(),
|
||||||
|
exc_info=None,
|
||||||
|
)
|
||||||
|
|
||||||
|
filter_instance = HealthCheckFilter()
|
||||||
|
assert filter_instance.filter(record) is False
|
||||||
|
|
||||||
|
def test_allows_other_requests(self):
|
||||||
|
"""Test that non-health-check requests are not filtered."""
|
||||||
|
record = logging.LogRecord(
|
||||||
|
name="uvicorn.access",
|
||||||
|
level=logging.INFO,
|
||||||
|
pathname="",
|
||||||
|
lineno=0,
|
||||||
|
msg='127.0.0.1:12345 - "GET /mcp/messages HTTP/1.1" 200',
|
||||||
|
args=(),
|
||||||
|
exc_info=None,
|
||||||
|
)
|
||||||
|
|
||||||
|
filter_instance = HealthCheckFilter()
|
||||||
|
assert filter_instance.filter(record) is True
|
||||||
|
|
||||||
|
def test_allows_api_requests(self):
|
||||||
|
"""Test that API requests are not filtered."""
|
||||||
|
record = logging.LogRecord(
|
||||||
|
name="uvicorn.access",
|
||||||
|
level=logging.INFO,
|
||||||
|
pathname="",
|
||||||
|
lineno=0,
|
||||||
|
msg='127.0.0.1:12345 - "POST /oauth/login HTTP/1.1" 302',
|
||||||
|
args=(),
|
||||||
|
exc_info=None,
|
||||||
|
)
|
||||||
|
|
||||||
|
filter_instance = HealthCheckFilter()
|
||||||
|
assert filter_instance.filter(record) is True
|
||||||
Reference in New Issue
Block a user