Address feedback from Copilot

Author: pamelafox

infra/aca.bicep

@@ -59,6 +59,7 @@ module app 'core/host/container-app-upsert.bicep' = {
         name: 'AZURE_COSMOSDB_CONTAINER'
         value: cosmosDbContainer
       }
+      // We typically store sensitive values in secrets, but App Insights connection strings are not considered highly sensitive
       {
         name: 'APPLICATIONINSIGHTS_CONNECTION_STRING'
         value: applicationInsightsConnectionString

infra/main.bicep

@@ -754,4 +754,5 @@ output AZURE_COSMOSDB_ENDPOINT string = cosmosDb.outputs.endpoint
 output AZURE_COSMOSDB_DATABASE string = cosmosDbDatabaseName
 output AZURE_COSMOSDB_CONTAINER string = cosmosDbContainerName
 
+// We typically do not output sensitive values, but App Insights connection strings are not considered highly sensitive
 output APPLICATIONINSIGHTS_CONNECTION_STRING string = useMonitoring ? applicationInsights.outputs.connectionString : ''

infra/write_env.ps1

@@ -11,4 +11,5 @@ Add-Content -Path $ENV_FILE_PATH -Value "AZURE_TENANT_ID=$(azd env get-value AZU
 Add-Content -Path $ENV_FILE_PATH -Value "AZURE_COSMOSDB_ACCOUNT=$(azd env get-value AZURE_COSMOSDB_ACCOUNT)"
 Add-Content -Path $ENV_FILE_PATH -Value "AZURE_COSMOSDB_DATABASE=$(azd env get-value AZURE_COSMOSDB_DATABASE)"
 Add-Content -Path $ENV_FILE_PATH -Value "AZURE_COSMOSDB_CONTAINER=$(azd env get-value AZURE_COSMOSDB_CONTAINER)"
+Add-Content -Path $ENV_FILE_PATH -Value "APPLICATIONINSIGHTS_CONNECTION_STRING=$(azd env get-value APPLICATIONINSIGHTS_CONNECTION_STRING)"
 Add-Content -Path $ENV_FILE_PATH -Value "API_HOST=azure"

infra/write_env.sh

@@ -15,4 +15,5 @@ echo "AZURE_TENANT_ID=$(azd env get-value AZURE_TENANT_ID)" >> "$ENV_FILE_PATH"
 echo "AZURE_COSMOSDB_ACCOUNT=$(azd env get-value AZURE_COSMOSDB_ACCOUNT)" >> "$ENV_FILE_PATH"
 echo "AZURE_COSMOSDB_DATABASE=$(azd env get-value AZURE_COSMOSDB_DATABASE)" >> "$ENV_FILE_PATH"
 echo "AZURE_COSMOSDB_CONTAINER=$(azd env get-value AZURE_COSMOSDB_CONTAINER)" >> "$ENV_FILE_PATH"
-echo "API_HOST=azure" >> "$ENV_FILE_PATH"
+echo "APPLICATIONINSIGHTS_CONNECTION_STRING=$(azd env get-value APPLICATIONINSIGHTS_CONNECTION_STRING)" >> "$ENV_FILE_PATH"
+echo "API_HOST=azure" >> "$ENV_FILE_PATH"

pyproject.toml

@@ -17,7 +17,8 @@ dependencies = [
     "agent-framework>=1.0.0b251016",
     "azure-cosmos>=4.9.0",
     "azure-monitor-opentelemetry>=1.6.4",
-    "logfire>=3.11.0",
+    "opentelemetry-instrumentation-starlette>=0.49b0",
+    "logfire>=4.15.1",
     "azure-core-tracing-opentelemetry>=1.0.0b12"
 ]
 

servers/deployed_mcp.py

@@ -1,3 +1,5 @@
+"""Run with: cd servers && uvicorn deployed_mcp:app --host 0.0.0.0 --port 8000"""
+
 import logging
 import os
 import uuid
@@ -12,7 +14,12 @@
 from azure.monitor.opentelemetry import configure_azure_monitor
 from dotenv import load_dotenv
 from fastmcp import FastMCP
-from opentelemetry_middleware import OpenTelemetryMiddleware
+from opentelemetry.instrumentation.starlette import StarletteInstrumentor
+
+try:
+    from opentelemetry_middleware import OpenTelemetryMiddleware
+except ImportError:
+    from servers.opentelemetry_middleware import OpenTelemetryMiddleware
 
 RUNNING_IN_PRODUCTION = os.getenv("RUNNING_IN_PRODUCTION", "false").lower() == "true"
 
@@ -23,13 +30,14 @@
 logger = logging.getLogger("ExpensesMCP")
 logger.setLevel(logging.INFO)
 
-# Configure OpenTelemetry tracing
+# Configure OpenTelemetry tracing, either via Azure Monitor or Logfire
+# We don't support both at the same time due to potential conflicts with tracer providers
 if os.getenv("APPLICATIONINSIGHTS_CONNECTION_STRING"):
     logger.info("Setting up Azure Monitor instrumentation")
     configure_azure_monitor()
 elif os.getenv("LOGFIRE_PROJECT_NAME"):
     logger.info("Setting up Logfire instrumentation")
-    settings.tracing_implementation = "opentelemetry"  # Send Azure Monitor traces via OpenTelemetry
+    settings.tracing_implementation = "opentelemetry"  # Configure Azure SDK to use OpenTelemetry tracing
     logfire.configure(service_name="expenses-mcp", send_to_logfire=True)
 
 # Cosmos DB configuration from environment variables
@@ -169,7 +177,4 @@ def analyze_spending_prompt(
 
 # ASGI application for uvicorn
 app = mcp.http_app()
-
-if __name__ == "__main__":
-    logger.info("MCP Expenses server starting (HTTP mode on port 8000)")
-    mcp.run(transport="http", host="0.0.0.0", port=8000)
+StarletteInstrumentor.instrument_app(app)

servers/opentelemetry_middleware.py

@@ -19,6 +19,8 @@ async def on_call_tool(self, context: MiddlewareContext, call_next):
                 "mcp.method": context.method,
                 "mcp.source": context.source,
                 "mcp.tool.name": tool_name,
+                # If arguments are sensitive, consider omitting or sanitizing them
+                # If arguments are long/nested, consider adding a size or depth limit
                 "mcp.tool.arguments": str(context.message.arguments),
             },
         ) as span: