Add opentelemetry instrumentation instructions to cloud run MCP sample#13991
Add opentelemetry instrumentation instructions to cloud run MCP sample#13991iennae merged 8 commits intoGoogleCloudPlatform:mainfrom
Conversation
product-auto-label
Bot
added
samples
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request integrates OpenTelemetry into the MCP server sample to enable observability for traces, logs, and metrics. It includes the necessary dependencies in pyproject.toml and uv.lock, adds a new setup script for OpenTelemetry, and updates the server and test files to initialize it. I have added a suggestion to include a note about the experimental status of the OpenTelemetry Logging SDK in the README to better inform users regarding production readiness.
aabmass
force-pushed
the
push-mklxmwyxyzkt
branch
from
8b476c9 to
567084e
Compare
| """Sets up OpenTelemetry to send traces, logs, and metrics to GCP Observability.""" | ||
| credentials, project_id = google.auth.default() | ||
| if not project_id: | ||
| raise Exception("Could not determine Google Cloud project ID.") |
There was a problem hiding this comment.
Could we refine the error handling here? It would be great to add import google.api_core.exceptions and catch the more specific google.auth.exceptions.DefaultCredentialsError. This helps make the error handling more robust and explicit."
There was a problem hiding this comment.
I added this check because the returned project_id can be None even without an exception being raised IIUC.
I think it's OK to let DefaultCredentialsError bubble up from the sample right?
| logger = logging.getLogger(__name__) | ||
|
|
||
|
|
||
| def setup_opentelemetry(service_name: str) -> None: |
There was a problem hiding this comment.
can you add a try...except block and catch not just the credentials issue but whether it successfully was able to connect to the telemetry endpoint?
There was a problem hiding this comment.
Unfortunately there's not a great way to catch it, the connection happens lazily/asynchronously.
Would you prefer if I split this into another sample duplicating most of the code to decouple the samples?
aabmass
left a comment
aabmass
left a comment
There was a problem hiding this comment.
Thanks for the review!
| logger = logging.getLogger(__name__) | ||
|
|
||
|
|
||
| def setup_opentelemetry(service_name: str) -> None: |
There was a problem hiding this comment.
Unfortunately there's not a great way to catch it, the connection happens lazily/asynchronously.
Would you prefer if I split this into another sample duplicating most of the code to decouple the samples?
| """Sets up OpenTelemetry to send traces, logs, and metrics to GCP Observability.""" | ||
| credentials, project_id = google.auth.default() | ||
| if not project_id: | ||
| raise Exception("Could not determine Google Cloud project ID.") |
There was a problem hiding this comment.
I added this check because the returned project_id can be None even without an exception being raised IIUC.
I think it's OK to let DefaultCredentialsError bubble up from the sample right?
aabmass
force-pushed
the
push-mklxmwyxyzkt
branch
from
567084e to
7d4cb65
Compare
|
Here is the summary of changes. You are about to add 3 region tags.
This comment is generated by snippet-bot.
|
aabmass
force-pushed
the
push-mklxmwyxyzkt
branch
5 times, most recently
from
cc6e585 to
cf7a1c3
Compare
aabmass
force-pushed
the
push-mklxmwyxyzkt
branch
from
50bfc4f to
9d3e86d
Compare
Split the test_server.py into separate OTel script
aabmass
force-pushed
the
push-mklxmwyxyzkt
branch
from
9d3e86d to
cc66ecf
Compare
4 participants
Description
Adds OpenTelemetry SDK setup to the Cloud Run MCP sample. FastMCP has instrumentation built in, so this starts sending it to Cloud Trace
Checklist
nox -s py-3.9(see Test Environment Setup)nox -s lint(see Test Environment Setup)