Skip to content

Commit ec68142

Browse files
committed
feat: add OpenTelemetry tracing for tool calls
1 parent 48f2a94 commit ec68142

10 files changed

Lines changed: 683 additions & 82 deletions

File tree

README.md

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,7 @@ kubeflow-mcp serve \
121121
--instruction-tier full \ # full | compact | minimal
122122
--transport stdio \ # stdio | http | sse
123123
--auth-token SECRET \ # bearer token for HTTP auth (dev/staging)
124+
--otel-endpoint URL \ # OTLP HTTP endpoint (optional tracing)
124125
--log-level INFO \ # DEBUG | INFO | WARNING | ERROR
125126
--log-format console \ # console | json (auto-detected if omitted)
126127
--no-banner # suppress startup banner
@@ -165,6 +166,23 @@ kubeflow-mcp agent \
165166

166167
</details>
167168

169+
## Observability
170+
171+
OpenTelemetry tracing is optional and can be enabled without changing tool code.
172+
173+
- Install optional dependencies: `pip install ".[otel]"`
174+
- Enable tracing with CLI flag or env var:
175+
176+
```bash
177+
kubeflow-mcp serve --otel-endpoint http://localhost:4318/v1/traces
178+
# or
179+
export KUBEFLOW_MCP_OTEL_ENDPOINT=http://localhost:4318/v1/traces
180+
kubeflow-mcp serve
181+
```
182+
183+
Each tool invocation emits a span with attributes:
184+
`tool.name`, `tool.success`, `tool.duration_ms`, `kubeflow.persona`, and `correlation_id`.
185+
168186
## Development
169187

170188
```bash

kubeflow_mcp/cli.py

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,12 @@ def cli() -> None:
9494
"Falls back to KUBEFLOW_MCP_AUTH_TOKEN env var, config file. "
9595
"Ignored for stdio transport.",
9696
)
97+
@click.option(
98+
"--otel-endpoint",
99+
default=None,
100+
help="OpenTelemetry OTLP HTTP endpoint for tracing. "
101+
"Falls back to KUBEFLOW_MCP_OTEL_ENDPOINT env var, config file.",
102+
)
97103
def serve(
98104
clients: str | None,
99105
persona: str | None,
@@ -104,6 +110,7 @@ def serve(
104110
instruction_tier: str | None,
105111
no_banner: bool,
106112
auth_token: str | None,
113+
otel_endpoint: str | None,
107114
) -> None:
108115
"""Start the MCP server.
109116
@@ -116,6 +123,7 @@ def serve(
116123
from kubeflow_mcp.core.logging import setup_logging
117124
from kubeflow_mcp.core.resilience import configure_circuit_breaker
118125
from kubeflow_mcp.core.server import configure_resilience, create_server
126+
from kubeflow_mcp.core.telemetry import setup_tracing
119127

120128
cfg = load_config()
121129

@@ -128,8 +136,11 @@ def serve(
128136

129137
if auth_token:
130138
cfg.auth.auth_token = auth_token
139+
if otel_endpoint:
140+
cfg.observability.otel_endpoint = otel_endpoint
131141

132142
logger = setup_logging(level=log_level, format=log_format)
143+
tracing_enabled = setup_tracing(endpoint=cfg.observability.otel_endpoint)
133144
logger.info(
134145
"Starting kubeflow-mcp",
135146
extra={
@@ -138,6 +149,7 @@ def serve(
138149
"transport": transport,
139150
"mode": mode,
140151
"instruction_tier": instruction_tier,
152+
"tracing_enabled": tracing_enabled,
141153
},
142154
)
143155

kubeflow_mcp/cli_test.py

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,7 @@ def _make_serve_mocks(config=None):
9797
mock_load_config = MagicMock(return_value=config)
9898
mock_build_auth_provider = MagicMock(return_value=None)
9999
mock_configure_circuit_breaker = MagicMock()
100+
mock_setup_tracing = MagicMock(return_value=False)
100101

101102
fake_server_mod = MagicMock()
102103
fake_server_mod.create_server = mock_create_server
@@ -109,13 +110,16 @@ def _make_serve_mocks(config=None):
109110
fake_auth_mod.build_auth_provider = mock_build_auth_provider
110111
fake_resilience_mod = MagicMock()
111112
fake_resilience_mod.configure_circuit_breaker = mock_configure_circuit_breaker
113+
fake_telemetry_mod = MagicMock()
114+
fake_telemetry_mod.setup_tracing = mock_setup_tracing
112115

113116
modules_patch = {
114117
"kubeflow_mcp.core.server": fake_server_mod,
115118
"kubeflow_mcp.core.logging": fake_logging_mod,
116119
"kubeflow_mcp.core.config": fake_config_mod,
117120
"kubeflow_mcp.core.auth": fake_auth_mod,
118121
"kubeflow_mcp.core.resilience": fake_resilience_mod,
122+
"kubeflow_mcp.core.telemetry": fake_telemetry_mod,
119123
}
120124
return mock_server, mock_create_server, modules_patch
121125

@@ -401,3 +405,40 @@ def test_serve_default_shows_banner():
401405

402406
_, kwargs = mock_server.run.call_args
403407
assert kwargs.get("show_banner") is True
408+
409+
410+
def test_serve_calls_setup_tracing_with_config_endpoint():
411+
from kubeflow_mcp.core.config import ObservabilityConfig
412+
413+
config = _make_default_config()
414+
config.observability = ObservabilityConfig(otel_endpoint="http://otel-collector:4318/v1/traces")
415+
mock_server, _, modules_patch = _make_serve_mocks(config=config)
416+
fake_telemetry_mod = modules_patch["kubeflow_mcp.core.telemetry"]
417+
418+
with patch.dict(sys.modules, modules_patch):
419+
runner = CliRunner()
420+
runner.invoke(cli, ["serve"])
421+
422+
fake_telemetry_mod.setup_tracing.assert_called_once_with(
423+
endpoint="http://otel-collector:4318/v1/traces"
424+
)
425+
426+
427+
def test_serve_otel_endpoint_cli_overrides_config():
428+
from kubeflow_mcp.core.config import ObservabilityConfig
429+
430+
config = _make_default_config()
431+
config.observability = ObservabilityConfig(otel_endpoint="http://old-endpoint:4318/v1/traces")
432+
mock_server, _, modules_patch = _make_serve_mocks(config=config)
433+
fake_telemetry_mod = modules_patch["kubeflow_mcp.core.telemetry"]
434+
435+
with patch.dict(sys.modules, modules_patch):
436+
runner = CliRunner()
437+
runner.invoke(
438+
cli,
439+
["serve", "--otel-endpoint", "http://new-endpoint:4318/v1/traces"],
440+
)
441+
442+
fake_telemetry_mod.setup_tracing.assert_called_once_with(
443+
endpoint="http://new-endpoint:4318/v1/traces"
444+
)

kubeflow_mcp/core/config.py

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,9 @@
5050
level: INFO
5151
format: json
5252
53+
observability:
54+
otel_endpoint: http://localhost:4318/v1/traces
55+
5356
Namespace restrictions are enforced via ``~/.kf-mcp-policy.yaml``
5457
(``policy.namespaces``), not through server config.
5558
"""
@@ -135,6 +138,12 @@ class LoggingConfig(BaseModel):
135138
format: str | None = Field(default=None)
136139

137140

141+
class ObservabilityConfig(BaseModel):
142+
"""Observability configuration."""
143+
144+
otel_endpoint: str | None = Field(default=None)
145+
146+
138147
class Config(BaseModel):
139148
"""Root configuration."""
140149

@@ -144,6 +153,7 @@ class Config(BaseModel):
144153
trainer: TrainerConfig = Field(default_factory=TrainerConfig)
145154
optimizer: OptimizerConfig = Field(default_factory=OptimizerConfig)
146155
logging: LoggingConfig = Field(default_factory=LoggingConfig)
156+
observability: ObservabilityConfig = Field(default_factory=ObservabilityConfig)
147157

148158

149159
def _find_config_file() -> Path | None:
@@ -229,6 +239,14 @@ def load_config(config_path: Path | None = None) -> Config:
229239
format=os.getenv("LOG_FORMAT", logging_file.get("format")),
230240
)
231241

242+
observability_file = file_config.get("observability", {})
243+
observability = ObservabilityConfig(
244+
otel_endpoint=os.getenv(
245+
"KUBEFLOW_MCP_OTEL_ENDPOINT",
246+
observability_file.get("otel_endpoint"),
247+
)
248+
)
249+
232250
# Build client-specific configs
233251
trainer_file = file_config.get("trainer", {})
234252
trainer = TrainerConfig(
@@ -283,6 +301,7 @@ def load_config(config_path: Path | None = None) -> Config:
283301
trainer=trainer,
284302
optimizer=optimizer,
285303
logging=logging_config,
304+
observability=observability,
286305
)
287306

288307

kubeflow_mcp/core/logging.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ def format(self, record: logging.LogRecord) -> str:
4949
if ctx is not None:
5050
log_dict["context"] = ctx
5151

52-
extra_keys = {"audit", "tool", "parameters", "success", "duration_ms"}
52+
extra_keys = {"audit", "tool", "parameters", "success", "duration_ms", "tracing_enabled"}
5353
for key in extra_keys:
5454
if hasattr(record, key):
5555
log_dict[key] = getattr(record, key)

kubeflow_mcp/core/server.py

Lines changed: 82 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -41,10 +41,16 @@
4141
HEALTH_TOOLS,
4242
)
4343
from kubeflow_mcp.core.logging import with_correlation_id
44-
from kubeflow_mcp.core.policy import apply_policy_filters, get_allowed_tools, is_read_only
44+
from kubeflow_mcp.core.policy import (
45+
apply_policy_filters,
46+
get_allowed_tools,
47+
get_effective_persona,
48+
is_read_only,
49+
)
4550
from kubeflow_mcp.core.resilience import RateLimiter, get_breaker
4651
from kubeflow_mcp.core.resources import register_resources
4752
from kubeflow_mcp.core.security import mask_sensitive_data
53+
from kubeflow_mcp.core.telemetry import get_tracer
4854

4955
logger = logging.getLogger(__name__)
5056

@@ -88,66 +94,84 @@ def _audit_wrap(tool_func):
8894
@functools.wraps(tool_func)
8995
def wrapper(**kwargs):
9096
tool_name = tool_func.__name__
91-
92-
if _rate_limiter is not None and not _rate_limiter.acquire():
93-
logger.warning("rate_limited", extra={"tool": tool_name})
94-
return {
95-
"error": "Rate limit exceeded. Retry after a brief pause.",
96-
"error_code": ErrorCode.RATE_LIMITED,
97-
}
98-
99-
breaker = get_breaker(tool_name)
100-
if not breaker.can_execute():
101-
logger.warning("circuit_open", extra={"tool": tool_name})
102-
return {
103-
"error": f"Circuit breaker open for '{tool_name}' — K8s API may be degraded. Retries automatically after recovery timeout.",
104-
"error_code": ErrorCode.CIRCUIT_OPEN,
105-
}
106-
10797
cid = with_correlation_id()
108-
masked = mask_sensitive_data(kwargs) if kwargs else {}
98+
tracer = get_tracer("kubeflow_mcp.tools")
99+
persona = get_effective_persona()
109100
start = time.monotonic()
110-
try:
111-
result = tool_func(**kwargs)
112-
duration_ms = int((time.monotonic() - start) * 1000)
113-
is_success = (
114-
"error_code" not in result and "error" not in result
115-
if isinstance(result, dict)
116-
else True
117-
)
118-
if is_success:
119-
breaker.record_success()
120-
elif is_infrastructure_error(result):
121-
breaker.record_failure()
122101

123-
logger.info(
124-
"tool_call",
125-
extra={
126-
"audit": True,
127-
"correlation_id": cid,
128-
"tool": tool_name,
129-
"parameters": masked,
130-
"success": is_success,
131-
"duration_ms": duration_ms,
132-
},
133-
)
134-
return _inject_meta(result, tool_name)
135-
except Exception:
136-
duration_ms = int((time.monotonic() - start) * 1000)
137-
breaker.record_failure()
138-
logger.error(
139-
"tool_call_failed",
140-
extra={
141-
"audit": True,
142-
"correlation_id": cid,
143-
"tool": tool_name,
144-
"parameters": masked,
145-
"success": False,
146-
"duration_ms": duration_ms,
147-
},
148-
exc_info=True,
149-
)
150-
raise
102+
with tracer.start_as_current_span("tool_call") as span:
103+
span.set_attribute("tool.name", tool_name)
104+
span.set_attribute("kubeflow.persona", persona)
105+
span.set_attribute("correlation_id", cid)
106+
107+
if _rate_limiter is not None and not _rate_limiter.acquire():
108+
duration_ms = int((time.monotonic() - start) * 1000)
109+
span.set_attribute("tool.success", False)
110+
span.set_attribute("tool.duration_ms", duration_ms)
111+
logger.warning("rate_limited", extra={"tool": tool_name})
112+
return {
113+
"error": "Rate limit exceeded. Retry after a brief pause.",
114+
"error_code": ErrorCode.RATE_LIMITED,
115+
}
116+
117+
breaker = get_breaker(tool_name)
118+
if not breaker.can_execute():
119+
duration_ms = int((time.monotonic() - start) * 1000)
120+
span.set_attribute("tool.success", False)
121+
span.set_attribute("tool.duration_ms", duration_ms)
122+
logger.warning("circuit_open", extra={"tool": tool_name})
123+
return {
124+
"error": f"Circuit breaker open for '{tool_name}' — K8s API may be degraded. Retries automatically after recovery timeout.",
125+
"error_code": ErrorCode.CIRCUIT_OPEN,
126+
}
127+
128+
masked = mask_sensitive_data(kwargs) if kwargs else {}
129+
try:
130+
result = tool_func(**kwargs)
131+
duration_ms = int((time.monotonic() - start) * 1000)
132+
is_success = (
133+
"error_code" not in result and "error" not in result
134+
if isinstance(result, dict)
135+
else True
136+
)
137+
span.set_attribute("tool.success", is_success)
138+
span.set_attribute("tool.duration_ms", duration_ms)
139+
if is_success:
140+
breaker.record_success()
141+
elif is_infrastructure_error(result):
142+
breaker.record_failure()
143+
144+
logger.info(
145+
"tool_call",
146+
extra={
147+
"audit": True,
148+
"correlation_id": cid,
149+
"tool": tool_name,
150+
"parameters": masked,
151+
"success": is_success,
152+
"duration_ms": duration_ms,
153+
},
154+
)
155+
return _inject_meta(result, tool_name)
156+
except Exception as exc:
157+
duration_ms = int((time.monotonic() - start) * 1000)
158+
breaker.record_failure()
159+
span.set_attribute("tool.success", False)
160+
span.set_attribute("tool.duration_ms", duration_ms)
161+
span.record_exception(exc)
162+
logger.error(
163+
"tool_call_failed",
164+
extra={
165+
"audit": True,
166+
"correlation_id": cid,
167+
"tool": tool_name,
168+
"parameters": masked,
169+
"success": False,
170+
"duration_ms": duration_ms,
171+
},
172+
exc_info=True,
173+
)
174+
raise
151175

152176
return wrapper
153177

0 commit comments

Comments
 (0)