Repository navigation
feat(secretmanager): add OpenTelemetry distributed tracing samples - #14647
chalmerlowe wants to merge 7 commits into
Conversation
Add standalone snippets and unit test suites demonstrating native OpenTelemetry distributed tracing for SecretManagerServiceClient and SecretManagerServiceAsyncClient: - Basic quickstart snippet using ConsoleSpanExporter and environment variable activation - Google Cloud Trace exporter sample using CloudTraceSpanExporter and BatchSpanProcessor - Programmatic configuration sample using ClientOptions(tracer_provider=...) - Asynchronous client sample using SecretManagerServiceAsyncClient - Mocked unit test suites with conftest fixture for isolated test execution
There was a problem hiding this comment.
Code Review
This pull request introduces new Python snippets and unit tests demonstrating OpenTelemetry distributed tracing with the Secret Manager client libraries, covering synchronous, asynchronous, programmatic, and Google Cloud Trace configurations. The feedback highlights a critical anti-pattern where the global TracerProvider is initialized and shut down inside reusable functions, which can cause silent failures on subsequent invocations. The reviewer recommends moving the TracerProvider lifecycle management to the main entry points, updating the unit tests to set the global provider directly, and importing the root secretmanager module in the async snippet for consistency.
…artext secret logging
…et_version snippet
…tures into conftest.py
|
Here is the summary of changes. You are about to add 4 region tags.
This comment is generated by snippet-bot.
|
| # applications (e.g., web servers), configure the TracerProvider once at | ||
| # application startup. | ||
| # | ||
| # 1. Initialize the OpenTelemetry TracerProvider with a Console exporter |
There was a problem hiding this comment.
Remove unnecessary numbering on the comments.
iennae
left a comment
There was a problem hiding this comment.
Overall, these look really great!
I just have one minor nit regarding the filenames. I recommend including the specific API action in the filenames (e.g., renaming secretmanager_trace.py to secretmanager_get_secret_version_trace.py). While I understand these are meant to be general Secret Manager tracing examples, making the action explicit aligns better with our standard naming conventions and makes it immediately clear what the sample is doing.
I'm curious about the choice of get_secret_version for the trace sample. Did you consider tracing an add_secret_version operation instead? I'm wondering if a write/mutate operation might produce a more interesting trace output for a user to see. I realize get is easier to run cleanly without mutating state, so I don't have a strong preference—just wondering if an add would show off the tracing value better!
Not blockers for merging, thank you for improving observability!
Thank you!
Add OpenTelemetry distributed tracing samples for Secret Manager.
There is a push to add OpenTelemetry (Observability) support to the GAPIC generated libraries.
This is a collection of samples that applies to Secret Manager, but is intended to help users of any GAPIC generated library to take advantage of the new Observability capabilities that are rolling out.
Note
For clarity:
secretmanager,translate, etc.