fix(py/samples): guard sentry import, fix dual-trace export, improve dev UX#4503
fix(py/samples): guard sentry import, fix dual-trace export, improve dev UX#4503
Conversation
Summary of ChangesHello @yesudeep, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses a Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request correctly addresses the ModuleNotFoundError by moving the type-only import behind a TYPE_CHECKING guard. This is the standard approach for handling optional dependencies in Python for type hinting. My review includes one suggestion to add a regression test to ensure the module remains importable even if sentry-sdk is not installed, which will help prevent similar issues in the future.
4d5edd5 to
713dfce
Compare
85305c4 to
0b7d106
Compare
0b7d106 to
853a5d3
Compare
564119d to
27a3938
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
This is a great pull request that brings several valuable improvements to the Python samples. The fixes for dual-trace export and the Sentry import guard are well-implemented and address important issues. The enhancements to developer experience, such as the robust CLI installers in _common.sh and the simplified just dev command, are excellent additions. The code is clean, well-documented, and the accompanying tests are thorough. I have one minor suggestion in _common.sh to improve the robustness of the package manager detection. Overall, fantastic work!
27a3938 to
235cf60
Compare
…slate default Three fixes for the web-endpoints-hello sample: 1. Guard sentry_sdk import behind TYPE_CHECKING — the top-level import causes ModuleNotFoundError when sentry-sdk is not installed, even though setup_sentry() handles the missing package gracefully. 2. Fix dual-trace export to DevUI + Jaeger — the old code created a competing TracerProvider that replaced Genkit's provider, so traces went to either the DevUI or Jaeger but not both. Now uses genkit.core.tracing.add_custom_exporter to attach the OTLP exporter to Genkit's existing provider. When no DevUI is running (production), a provider with SERVICE_NAME is created first so Jaeger shows the correct service name. 3. Move Jaeger auto-start and --otel-endpoint wiring from justfile into run.sh so `./run.sh` alone gives the full dev experience with tracing. Pass --no-telemetry to skip Jaeger. 4. Add a rich default text for translate_text flow — a paragraph about the Northern Lights makes the DevUI demo more impressive.
235cf60 to
ceec4d3
Compare
Summary
Fixes and improvements for the
web-endpoints-hellosample and_common.sh.1. Guard sentry_sdk import behind
TYPE_CHECKINGThe top-level
from sentry_sdk.integrations import IntegrationcausesModuleNotFoundErrorwhensentry-sdkis not installed.2. Fix dual-trace export (DevUI + Jaeger)
Uses
genkit.core.tracing.add_custom_exporter()to attach the OTLPexporter to Genkit's existing
TracerProvider. Both DevUI and Jaegerreceive spans simultaneously. Sets
OTEL_SERVICE_NAMEenv var inrun.shso traces show the correct service name.3. Move Jaeger auto-start into
run.shrun.shnow handles Jaeger start +--otel-endpoint(previously onlyjust devdid this).just devsimplified to call./run.sh.4. gRPC OTel instrumentation
Added
opentelemetry-instrumentation-grpcfor auto-instrumentation ofgRPC spans with OTel semantic conventions (
rpc.system,rpc.service,rpc.method). gRPC and REST traces are distinguishable in Jaegerby filtering on these attributes.
5. Robust CLI installers in
_common.shunzipon Linux and installs it viaapt-get/dnf/yum if missing.
/etc/os-releaseand usesthe appropriate installer (Debian/Ubuntu, Fedora/RHEL, or pip fallback).
6. Rich default text for
translate_textflowTranslateInput.textdefaults to a paragraph about the Northern Lights.Files changed
src/sentry_init.pyTYPE_CHECKINGsrc/telemetry.pyadd_custom_exportersrc/grpc_server.pyGrpcAioInstrumentorServerinstrumentationsrc/schemas.pyrun.shOTEL_SERVICE_NAMEjustfiledevto callrun.shpyproject.tomlopentelemetry-instrumentation-grpcdeptests/telemetry_otel_test.pytests/schemas_test.pypy/pyproject.tomlpy/samples/_common.shTesting
bin/lintandjust lintpass with 0 errors