Repository navigation
Conversation
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 maps standard and debug OpenTelemetry SDK instruments to Cloud Monitoring descriptor names under the storage.googleapis.com/client/ namespace, configures latency and size histogram views for these metrics, and adds corresponding unit tests. Feedback on the new tests recommends recording metrics once before the retry loop rather than inside it to avoid non-deterministic behavior and potential test flakiness.
| for (int i = 0; i != 50 && export_count.load() == 0; ++i) { | ||
| latency->Record(1.0, opentelemetry::context::Context{}); | ||
| size->Record(1024.0, opentelemetry::context::Context{}); | ||
| std::this_thread::sleep_for(kExportInterval); | ||
| } |
There was a problem hiding this comment.
Recording metrics inside the loop can lead to non-deterministic test behavior and potential flakiness. If the background export thread is slow to wake up, multiple records will be accumulated, resulting in a histogram with more than one sample. If the matchers ExpectedLatencyHistogram() or ExpectedSizeHistogram() assert a specific count or sum, the test will fail.
To make the test deterministic and robust, record the metrics exactly once before entering the loop, and then simply sleep/wait in the loop until the export completes.
latency->Record(1.0, opentelemetry::context::Context{});
size->Record(1024.0, opentelemetry::context::Context{});
for (int i = 0; i != 50 && export_count.load() == 0; ++i) {
std::this_thread::sleep_for(kExportInterval);
}There was a problem hiding this comment.
Moved Record() outside the wait loop so each histogram is recorded once before waiting for the export.
Replied by cpriti-o11y-agent
…ram views Update `MetricNameFormatterOption` in `MetricsExporterOptions()` to map standard and debug OpenTelemetry SDK instrument names (`gcp.client.request.duration` and `gcp.storage.client.*`) to their Cloud Monitoring target descriptors under `storage.googleapis.com/client/`. Also register explicit latency (`s`) and size (`By`) histogram views for the Storage client instruments under the `gl-cpp` meter scope in `MakeGrpcMeterProvider()`. [Generated-by: AI]
631feab to
7fdb367
Compare
cpriti-os
changed the title
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #16638 +/- ##
==========================================
- Coverage 92.48% 92.45% -0.03%
==========================================
Files 2269 2269
Lines 222192 222233 +41
==========================================
- Hits 205487 205470 -17
- Misses 16705 16763 +58 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Extends the GCS OpenTelemetry metrics exporter and meter provider for the client-side OpenTelemetry metrics specification:
MetricsExporterOptions()metric name formatter to map standard and debug OpenTelemetry SDK instrument names (gcp.client.request.durationandgcp.storage.client.*) to their Cloud Monitoring target descriptors understorage.googleapis.com/client/.s) and size (By) histogram bucket views for the Storage client instruments under thegl-cppmeter scope inMakeGrpcMeterProvider().Created by cpriti-o11y-agent