-
Notifications
You must be signed in to change notification settings - Fork 36
Add aspire standalone #17
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.
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
base: main
Are you sure you want to change the base?
Conversation
document how to run the Aspire dashboard and view traces add OpenTelemetry middleware helper with OTLP exporters wire middleware into basic_mcp_http.py and auto-configure when OTEL endpoint set
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds OpenTelemetry instrumentation support for the MCP HTTP server to enable observability through the .NET Aspire Dashboard. It introduces a reusable middleware pattern for capturing traces, metrics, and logs from MCP operations, with automatic configuration when the OTLP endpoint environment variable is set.
- Implements
OpenTelemetryMiddlewareto create spans for MCP tool calls, resource reads, and prompt retrievals - Adds
configure_aspire_dashboardhelper function to wire up OTLP exporters for traces, metrics, and logs - Integrates the middleware into
basic_mcp_http.pywith conditional auto-configuration based on environment variable
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| servers/opentelemetry_middleware.py | Adds OpenTelemetry middleware class and Aspire Dashboard configuration function with OTLP exporters for traces, metrics, and logs |
| servers/basic_mcp_http.py | Integrates OpenTelemetry middleware and enables conditional auto-configuration when OTEL_EXPORTER_OTLP_ENDPOINT is set |
| README.md | Documents how to run and access the Aspire Dashboard for viewing OpenTelemetry data from the MCP server |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
servers/opentelemetry_middleware.py
Outdated
| # Configure Tracing | ||
| tracer_provider = TracerProvider(resource=resource) | ||
| tracer_provider.add_span_processor( | ||
| SimpleSpanProcessor(OTLPSpanExporter(endpoint=otlp_endpoint, insecure=use_insecure)) |
Copilot
AI
Dec 9, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The endpoint parameter passed to OTLP exporters should typically not include the URL scheme (e.g., http:// or https://). The gRPC exporters expect just host:port format. The insecure parameter controls whether to use TLS. Consider extracting just netloc from the parsed URL: endpoint = parsed_endpoint.netloc or otlp_endpoint to handle both formats gracefully.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 5 out of 6 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| from opentelemetry._logs import set_logger_provider | ||
| from opentelemetry.exporter.otlp.proto.grpc._log_exporter import OTLPLogExporter | ||
| from opentelemetry.exporter.otlp.proto.grpc.metric_exporter import OTLPMetricExporter | ||
| from opentelemetry.exporter.otlp.proto.grpc.trace_exporter import OTLPSpanExporter | ||
| from opentelemetry.sdk._logs import LoggerProvider, LoggingHandler | ||
| from opentelemetry.sdk._logs.export import BatchLogRecordProcessor |
Copilot
AI
Dec 9, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using private/internal OpenTelemetry APIs: The imports from opentelemetry._logs (line 6), opentelemetry.exporter.otlp.proto.grpc._log_exporter (line 7), and opentelemetry.sdk._logs (lines 10-11) are using private modules (indicated by the underscore prefix). This is not recommended as these APIs may change without notice in future versions.
The logging signal is still experimental in OpenTelemetry Python, so using underscored modules may be currently unavoidable. However, be aware this could break in future versions. Consider:
- Adding a comment explaining why private APIs are used
- Pinning the OpenTelemetry version more strictly to avoid unexpected breakage
- Monitoring for when the logging API becomes stable
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are they available in the public exported API? I agree that we shouldn't ideally be using underscored vars from an SDK
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The official Aspire documentation for Python apps uses the exact same underscored imports. The underscores indicate "experimental" status in OTel Python, not "private/internal". It's unfortunate naming, but there's no alternative public API at the moment.
|
|
||
| def configure_aspire_dashboard(service_name: str = "expenses-mcp"): | ||
| """Configure OpenTelemetry to send telemetry to the Aspire standalone dashboard.""" | ||
| otlp_endpoint = os.getenv("OTEL_EXPORTER_OTLP_ENDPOINT", "http://localhost:4317") |
Copilot
AI
Dec 9, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] Insecure default configuration: The function uses http://localhost:4317 as a default endpoint when OTEL_EXPORTER_OTLP_ENDPOINT is not set. However, if this function is called when the environment variable is not set, telemetry will silently fail to export (since no OTLP receiver is likely running at that default endpoint).
Consider either:
- Not setting a default and requiring the environment variable to be set
- Logging a warning when using the default endpoint
- Making the behavior more explicit by requiring the caller to explicitly opt-in to telemetry
Currently, basic_mcp_http.py checks for the env var before calling this function, which is good practice.
| otlp_endpoint = os.getenv("OTEL_EXPORTER_OTLP_ENDPOINT", "http://localhost:4317") | |
| otlp_endpoint = os.getenv("OTEL_EXPORTER_OTLP_ENDPOINT", "http://localhost:4317") | |
| if "OTEL_EXPORTER_OTLP_ENDPOINT" not in os.environ: | |
| logging.warning( | |
| "OTEL_EXPORTER_OTLP_ENDPOINT is not set; falling back to default endpoint '%s'. " | |
| "Telemetry may not be exported unless an OTLP receiver is running at this endpoint.", | |
| otlp_endpoint, | |
| ) |
|
|
||
|
|
||
| otel_endpoint = os.getenv("OTEL_EXPORTER_OTLP_ENDPOINT") | ||
| middleware: list = [] |
Copilot
AI
Dec 9, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] Redundant type annotation: The type annotation list is less precise than it could be. Consider using list[OpenTelemetryMiddleware] or list[Middleware] to provide better type hints for IDE support and type checkers.
middleware: list[OpenTelemetryMiddleware] = []| middleware: list = [] | |
| middleware: list[OpenTelemetryMiddleware] = [] |
|
|
||
| from dotenv import load_dotenv | ||
| from fastmcp import FastMCP | ||
| from opentelemetry_middleware import OpenTelemetryMiddleware, configure_aspire_dashboard |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To fix the isort sorting so that this is in its own section, add this line at the bottom of pyproject.toml:
lint.isort.known-first-party = ["opentelemetry_middleware"]
| logger = logging.getLogger("ExpensesMCP") | ||
|
|
||
|
|
||
| otel_endpoint = os.getenv("OTEL_EXPORTER_OTLP_ENDPOINT") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suggest just inlining this os.getenv, since we dont actually use otel_endpoint elsewhere.
So:
middleware:list = []
if os.getenv("OTEL_EXPORTER_OTLP_ENPOINT"):
...
Co-authored-by: Copilot <[email protected]>
OpenTelemetryMiddlewareplusconfigure_aspire_dashboardhelper that wires spans/metrics/logs to OTLP.basic_mcp_http.pyto enable the middleware and auto-configure Aspire whenOTEL_EXPORTER_OTLP_ENDPOINTis present.