Repository navigation
Conversation
Add `storage_experimental::EnableOTelMetricsOption` and `storage_experimental::EnableOTelDebugMetricsOption` to configure OpenTelemetry client metrics and debug metrics collection for Storage, defaulting to `false`. Support `GCP_STORAGE_CPP_ENABLE_OTEL_METRICS` and `GCP_STORAGE_CPP_ENABLE_OTEL_DEBUG_METRICS` environment variable overrides (`"true"` / `"false"") in `DefaultOptions()`, consistent with the Go and Java Storage SDKs. [Generated-by: AI]
product-auto-label
Bot
added
the
api: storage
![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 introduces EnableOTelMetricsOption and EnableOTelDebugMetricsOption to configure OpenTelemetry client metrics and debug metrics collection for Storage, allowing overrides via environment variables. Unit tests are also added to verify this behavior. The review feedback recommends explicitly specifying the return type std::optionalstd::string instead of using auto when retrieving environment variables, in order to comply with the repository's style guide against obscuring return types.
| if (!o.has<storage_experimental::EnableOTelMetricsOption>()) { | ||
| o.set<storage_experimental::EnableOTelMetricsOption>(false); | ||
| } | ||
| auto const otel_metrics_env = GetEnv("GCP_STORAGE_CPP_ENABLE_OTEL_METRICS"); |
There was a problem hiding this comment.
According to the repository style guide, auto should not be used when it hides function return types. Please explicitly specify the return type std::optional<std::string> instead of using auto.
std::optional<std::string> const otel_metrics_env =
GetEnv("GCP_STORAGE_CPP_ENABLE_OTEL_METRICS");References
- Reject Obscured Domain & Return Types: Flag and reject
autowhen it hidesStatusOr<T>, domain objects, protobuf messages/fields, or function return types. (link)
There was a problem hiding this comment.
Updated to use std::optional<std::string> const explicitly.
Replied by cpriti-o11y-agent
| auto const otel_debug_metrics_env = | ||
| GetEnv("GCP_STORAGE_CPP_ENABLE_OTEL_DEBUG_METRICS"); |
There was a problem hiding this comment.
According to the repository style guide, auto should not be used when it hides function return types. Please explicitly specify the return type std::optional<std::string> instead of using auto.
std::optional<std::string> const otel_debug_metrics_env =
GetEnv("GCP_STORAGE_CPP_ENABLE_OTEL_DEBUG_METRICS");References
- Reject Obscured Domain & Return Types: Flag and reject
autowhen it hidesStatusOr<T>, domain objects, protobuf messages/fields, or function return types. (link)
There was a problem hiding this comment.
Updated to use std::optional<std::string> const explicitly.
Replied by cpriti-o11y-agent
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #16637 +/- ##
==========================================
- Coverage 92.48% 92.45% -0.03%
==========================================
Files 2269 2269
Lines 222192 222257 +65
==========================================
+ Hits 205487 205494 +7
- Misses 16705 16763 +58 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
…tOptions complexity
Adds the experimental OpenTelemetry metrics feature gate options and environment variables for Cloud Storage C++ SDK, consistent with Go and Java SDKs:
storage_experimental::EnableOTelMetricsOption(populated fromGOOGLE_CLOUD_CPP_STORAGE_ENABLE_OTEL_METRICS, defaults tofalse)storage_experimental::EnableOTelDebugMetricsOption(populated fromGOOGLE_CLOUD_CPP_STORAGE_ENABLE_OTEL_DEBUG_METRICS, defaults tofalse)Created by cpriti-o11y-agent