From d54ed9f378bee49ec9b34029393f62e13dc666c3 Mon Sep 17 00:00:00 2001 From: Gauri Kalra Date: Tue, 4 Aug 2026 10:23:47 +0000 Subject: [PATCH 1/4] feat(storage): add stream open latency metrics and trace annotations --- .../storage/internal/async/open_object.cc | 64 +++++++++++++++++++ .../storage/internal/async/open_object.h | 7 ++ 2 files changed, 71 insertions(+) diff --git a/google/cloud/storage/internal/async/open_object.cc b/google/cloud/storage/internal/async/open_object.cc index 1f3b87d805e3f..4f5aefd6b3ba6 100644 --- a/google/cloud/storage/internal/async/open_object.cc +++ b/google/cloud/storage/internal/async/open_object.cc @@ -15,6 +15,10 @@ #include "google/cloud/storage/internal/async/open_object.h" #include "google/cloud/internal/make_status.h" #include "absl/strings/str_cat.h" +#ifdef GOOGLE_CLOUD_CPP_STORAGE_WITH_OTEL_METRICS +#include "google/cloud/internal/opentelemetry.h" +#include +#endif #include namespace google { @@ -40,7 +44,39 @@ OpenObject::OpenObject(storage_internal::StorageStub& stub, CompletionQueue& cq, stub, cq, std::move(context), std::move(options), request))), initial_request_(std::move(request)) {} +#ifdef GOOGLE_CLOUD_CPP_STORAGE_WITH_OTEL_METRICS +struct StreamOpenMetrics { + opentelemetry::nostd::shared_ptr> + stream_open_latency; + opentelemetry::nostd::shared_ptr> + network_handshake; + opentelemetry::nostd::shared_ptr> + server_metadata_latency; + + static StreamOpenMetrics const& Instance() { + static auto const metrics = [] { + auto meter = + opentelemetry::metrics::Provider::GetMeterProvider()->GetMeter( + "storage", "v1"); + return StreamOpenMetrics{ + meter->CreateDoubleHistogram("gl-cpp.latency.stream_open", + "End-to-End Stream Open", "us"), + meter->CreateDoubleHistogram("gl-cpp.latency.network_handshake", + "Network Handshake", "us"), + meter->CreateDoubleHistogram("gl-cpp.latency.server_metadata", + "Server Metadata Latency", "us"), + }; + }(); + return metrics; + } +}; +#endif + future> OpenObject::Call() { + t0_ = std::chrono::steady_clock::now(); +#ifdef GOOGLE_CLOUD_CPP_STORAGE_WITH_OTEL_METRICS + span_ = opentelemetry::trace::Tracer::GetCurrentSpan(); +#endif auto future = promise_.get_future(); rpc_->Start().then([w = WeakFromThis()](auto f) { if (auto self = w.lock()) self->OnStart(f.get()); @@ -63,6 +99,7 @@ std::unique_ptr OpenObject::CreateRpc( } void OpenObject::OnStart(bool ok) { + t1_ = std::chrono::steady_clock::now(); if (!ok) return DoFinish(); rpc_->Write(initial_request_).then([w = WeakFromThis()](auto f) { if (auto self = w.lock()) self->OnWrite(f.get()); @@ -70,6 +107,7 @@ void OpenObject::OnStart(bool ok) { } void OpenObject::OnWrite(bool ok) { + t2_ = std::chrono::steady_clock::now(); if (!ok) return DoFinish(); rpc_->Read().then([w = WeakFromThis()](auto f) { if (auto self = w.lock()) self->OnRead(f.get()); @@ -78,6 +116,32 @@ void OpenObject::OnWrite(bool ok) { void OpenObject::OnRead( std::optional response) { + auto t3 = std::chrono::steady_clock::now(); +#ifdef GOOGLE_CLOUD_CPP_STORAGE_WITH_OTEL_METRICS + auto const& metrics = StreamOpenMetrics::Instance(); + + auto p1 = static_cast( + std::chrono::duration_cast(t1_ - t0_).count()); + auto p2 = static_cast( + std::chrono::duration_cast(t3 - t2_).count()); + auto p3 = static_cast( + std::chrono::duration_cast(t3 - t0_).count()); + + auto bucket = initial_request_.read_object_spec().bucket(); + metrics.network_handshake->Record(p1, {{"gcp.storage.bucket", bucket}}, + opentelemetry::context::Context{}); + metrics.server_metadata_latency->Record(p2, {{"gcp.storage.bucket", bucket}}, + opentelemetry::context::Context{}); + metrics.stream_open_latency->Record(p3, {{"gcp.storage.bucket", bucket}}, + opentelemetry::context::Context{}); + + if (span_ && span_->GetContext().IsValid()) { + span_->AddEvent("gl-cpp.open.read", + {{"gl-cpp.latency.network_handshake", p1}, + {"gl-cpp.latency.server_metadata", p2}, + {"gl-cpp.latency.stream_open", p3}}); + } +#endif if (!response) return DoFinish(); promise_.set_value(OpenStreamResult{std::move(rpc_), std::move(*response)}); } diff --git a/google/cloud/storage/internal/async/open_object.h b/google/cloud/storage/internal/async/open_object.h index 65f01dba4024c..36bd10df1a6c2 100644 --- a/google/cloud/storage/internal/async/open_object.h +++ b/google/cloud/storage/internal/async/open_object.h @@ -25,6 +25,7 @@ #include "google/cloud/version.h" #include "google/storage/v2/storage.pb.h" #include +#include #include #include @@ -107,6 +108,12 @@ class OpenObject : public std::enable_shared_from_this { std::shared_ptr rpc_; promise> promise_; google::storage::v2::BidiReadObjectRequest initial_request_; + std::chrono::steady_clock::time_point t0_; + std::chrono::steady_clock::time_point t1_; + std::chrono::steady_clock::time_point t2_; +#ifdef GOOGLE_CLOUD_CPP_STORAGE_WITH_OTEL_METRICS + opentelemetry::nostd::shared_ptr span_; +#endif }; GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_END From 73bfe502477db8e162408178cec32d2e27870c9c Mon Sep 17 00:00:00 2001 From: Gauri Kalra Date: Tue, 4 Aug 2026 11:28:56 +0000 Subject: [PATCH 2/4] Address feedback from code assistant --- google/cloud/storage/internal/async/open_object.cc | 8 ++++++-- google/cloud/storage/internal/async/open_object.h | 2 +- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/google/cloud/storage/internal/async/open_object.cc b/google/cloud/storage/internal/async/open_object.cc index 4f5aefd6b3ba6..49642f4ecf554 100644 --- a/google/cloud/storage/internal/async/open_object.cc +++ b/google/cloud/storage/internal/async/open_object.cc @@ -73,8 +73,8 @@ struct StreamOpenMetrics { #endif future> OpenObject::Call() { - t0_ = std::chrono::steady_clock::now(); #ifdef GOOGLE_CLOUD_CPP_STORAGE_WITH_OTEL_METRICS + t0_ = std::chrono::steady_clock::now(); span_ = opentelemetry::trace::Tracer::GetCurrentSpan(); #endif auto future = promise_.get_future(); @@ -99,7 +99,9 @@ std::unique_ptr OpenObject::CreateRpc( } void OpenObject::OnStart(bool ok) { +#ifdef GOOGLE_CLOUD_CPP_STORAGE_WITH_OTEL_METRICS t1_ = std::chrono::steady_clock::now(); +#endif if (!ok) return DoFinish(); rpc_->Write(initial_request_).then([w = WeakFromThis()](auto f) { if (auto self = w.lock()) self->OnWrite(f.get()); @@ -107,7 +109,9 @@ void OpenObject::OnStart(bool ok) { } void OpenObject::OnWrite(bool ok) { +#ifdef GOOGLE_CLOUD_CPP_STORAGE_WITH_OTEL_METRICS t2_ = std::chrono::steady_clock::now(); +#endif if (!ok) return DoFinish(); rpc_->Read().then([w = WeakFromThis()](auto f) { if (auto self = w.lock()) self->OnRead(f.get()); @@ -116,8 +120,8 @@ void OpenObject::OnWrite(bool ok) { void OpenObject::OnRead( std::optional response) { - auto t3 = std::chrono::steady_clock::now(); #ifdef GOOGLE_CLOUD_CPP_STORAGE_WITH_OTEL_METRICS + auto t3 = std::chrono::steady_clock::now(); auto const& metrics = StreamOpenMetrics::Instance(); auto p1 = static_cast( diff --git a/google/cloud/storage/internal/async/open_object.h b/google/cloud/storage/internal/async/open_object.h index 36bd10df1a6c2..18e9f4c3b7d8c 100644 --- a/google/cloud/storage/internal/async/open_object.h +++ b/google/cloud/storage/internal/async/open_object.h @@ -108,10 +108,10 @@ class OpenObject : public std::enable_shared_from_this { std::shared_ptr rpc_; promise> promise_; google::storage::v2::BidiReadObjectRequest initial_request_; +#ifdef GOOGLE_CLOUD_CPP_STORAGE_WITH_OTEL_METRICS std::chrono::steady_clock::time_point t0_; std::chrono::steady_clock::time_point t1_; std::chrono::steady_clock::time_point t2_; -#ifdef GOOGLE_CLOUD_CPP_STORAGE_WITH_OTEL_METRICS opentelemetry::nostd::shared_ptr span_; #endif }; From 68e279208dee7addda1c3fffcc5398396dfee951 Mon Sep 17 00:00:00 2001 From: Gauri Kalra Date: Thu, 6 Aug 2026 07:23:13 +0000 Subject: [PATCH 3/4] Address reviewer feedback about using a dedicated class for metrics --- .../storage/google_cloud_cpp_storage_grpc.bzl | 2 + .../google_cloud_cpp_storage_grpc.cmake | 2 + .../storage/internal/async/open_object.cc | 70 ++------- .../storage/internal/async/open_object.h | 7 +- .../internal/async/open_object_metrics.cc | 133 ++++++++++++++++++ .../internal/async/open_object_metrics.h | 61 ++++++++ 6 files changed, 210 insertions(+), 65 deletions(-) create mode 100644 google/cloud/storage/internal/async/open_object_metrics.cc create mode 100644 google/cloud/storage/internal/async/open_object_metrics.h diff --git a/google/cloud/storage/google_cloud_cpp_storage_grpc.bzl b/google/cloud/storage/google_cloud_cpp_storage_grpc.bzl index ad210011ea06f..69269f44fde53 100644 --- a/google/cloud/storage/google_cloud_cpp_storage_grpc.bzl +++ b/google/cloud/storage/google_cloud_cpp_storage_grpc.bzl @@ -50,6 +50,7 @@ google_cloud_cpp_storage_grpc_hdrs = [ "internal/async/object_descriptor_reader.h", "internal/async/object_descriptor_reader_tracing.h", "internal/async/open_object.h", + "internal/async/open_object_metrics.h", "internal/async/open_stream.h", "internal/async/partial_upload.h", "internal/async/read_payload_fwd.h", @@ -128,6 +129,7 @@ google_cloud_cpp_storage_grpc_srcs = [ "internal/async/object_descriptor_reader.cc", "internal/async/object_descriptor_reader_tracing.cc", "internal/async/open_object.cc", + "internal/async/open_object_metrics.cc", "internal/async/open_stream.cc", "internal/async/partial_upload.cc", "internal/async/read_range.cc", diff --git a/google/cloud/storage/google_cloud_cpp_storage_grpc.cmake b/google/cloud/storage/google_cloud_cpp_storage_grpc.cmake index 952d6dbef46d2..8d72e82151e26 100644 --- a/google/cloud/storage/google_cloud_cpp_storage_grpc.cmake +++ b/google/cloud/storage/google_cloud_cpp_storage_grpc.cmake @@ -123,6 +123,8 @@ add_library( internal/async/object_descriptor_reader_tracing.h internal/async/open_object.cc internal/async/open_object.h + internal/async/open_object_metrics.cc + internal/async/open_object_metrics.h internal/async/open_stream.cc internal/async/open_stream.h internal/async/partial_upload.cc diff --git a/google/cloud/storage/internal/async/open_object.cc b/google/cloud/storage/internal/async/open_object.cc index 49642f4ecf554..da59bc6f8a628 100644 --- a/google/cloud/storage/internal/async/open_object.cc +++ b/google/cloud/storage/internal/async/open_object.cc @@ -44,37 +44,9 @@ OpenObject::OpenObject(storage_internal::StorageStub& stub, CompletionQueue& cq, stub, cq, std::move(context), std::move(options), request))), initial_request_(std::move(request)) {} -#ifdef GOOGLE_CLOUD_CPP_STORAGE_WITH_OTEL_METRICS -struct StreamOpenMetrics { - opentelemetry::nostd::shared_ptr> - stream_open_latency; - opentelemetry::nostd::shared_ptr> - network_handshake; - opentelemetry::nostd::shared_ptr> - server_metadata_latency; - - static StreamOpenMetrics const& Instance() { - static auto const metrics = [] { - auto meter = - opentelemetry::metrics::Provider::GetMeterProvider()->GetMeter( - "storage", "v1"); - return StreamOpenMetrics{ - meter->CreateDoubleHistogram("gl-cpp.latency.stream_open", - "End-to-End Stream Open", "us"), - meter->CreateDoubleHistogram("gl-cpp.latency.network_handshake", - "Network Handshake", "us"), - meter->CreateDoubleHistogram("gl-cpp.latency.server_metadata", - "Server Metadata Latency", "us"), - }; - }(); - return metrics; - } -}; -#endif - future> OpenObject::Call() { -#ifdef GOOGLE_CLOUD_CPP_STORAGE_WITH_OTEL_METRICS - t0_ = std::chrono::steady_clock::now(); + metrics_.RecordCall(); +#ifdef GOOGLE_CLOUD_CPP_HAVE_OPENTELEMETRY span_ = opentelemetry::trace::Tracer::GetCurrentSpan(); #endif auto future = promise_.get_future(); @@ -99,9 +71,7 @@ std::unique_ptr OpenObject::CreateRpc( } void OpenObject::OnStart(bool ok) { -#ifdef GOOGLE_CLOUD_CPP_STORAGE_WITH_OTEL_METRICS - t1_ = std::chrono::steady_clock::now(); -#endif + metrics_.RecordStart(); if (!ok) return DoFinish(); rpc_->Write(initial_request_).then([w = WeakFromThis()](auto f) { if (auto self = w.lock()) self->OnWrite(f.get()); @@ -109,9 +79,7 @@ void OpenObject::OnStart(bool ok) { } void OpenObject::OnWrite(bool ok) { -#ifdef GOOGLE_CLOUD_CPP_STORAGE_WITH_OTEL_METRICS - t2_ = std::chrono::steady_clock::now(); -#endif + metrics_.RecordWrite(); if (!ok) return DoFinish(); rpc_->Read().then([w = WeakFromThis()](auto f) { if (auto self = w.lock()) self->OnRead(f.get()); @@ -120,32 +88,12 @@ void OpenObject::OnWrite(bool ok) { void OpenObject::OnRead( std::optional response) { -#ifdef GOOGLE_CLOUD_CPP_STORAGE_WITH_OTEL_METRICS - auto t3 = std::chrono::steady_clock::now(); - auto const& metrics = StreamOpenMetrics::Instance(); - - auto p1 = static_cast( - std::chrono::duration_cast(t1_ - t0_).count()); - auto p2 = static_cast( - std::chrono::duration_cast(t3 - t2_).count()); - auto p3 = static_cast( - std::chrono::duration_cast(t3 - t0_).count()); - - auto bucket = initial_request_.read_object_spec().bucket(); - metrics.network_handshake->Record(p1, {{"gcp.storage.bucket", bucket}}, - opentelemetry::context::Context{}); - metrics.server_metadata_latency->Record(p2, {{"gcp.storage.bucket", bucket}}, - opentelemetry::context::Context{}); - metrics.stream_open_latency->Record(p3, {{"gcp.storage.bucket", bucket}}, - opentelemetry::context::Context{}); - - if (span_ && span_->GetContext().IsValid()) { - span_->AddEvent("gl-cpp.open.read", - {{"gl-cpp.latency.network_handshake", p1}, - {"gl-cpp.latency.server_metadata", p2}, - {"gl-cpp.latency.stream_open", p3}}); - } +#ifdef GOOGLE_CLOUD_CPP_HAVE_OPENTELEMETRY + metrics_.RecordRead(initial_request_.read_object_spec().bucket(), span_); +#else + metrics_.RecordRead(initial_request_.read_object_spec().bucket()); #endif + if (!response) return DoFinish(); promise_.set_value(OpenStreamResult{std::move(rpc_), std::move(*response)}); } diff --git a/google/cloud/storage/internal/async/open_object.h b/google/cloud/storage/internal/async/open_object.h index 18e9f4c3b7d8c..5a980619d887b 100644 --- a/google/cloud/storage/internal/async/open_object.h +++ b/google/cloud/storage/internal/async/open_object.h @@ -15,6 +15,7 @@ #ifndef GOOGLE_CLOUD_CPP_GOOGLE_CLOUD_STORAGE_INTERNAL_ASYNC_OPEN_OBJECT_H #define GOOGLE_CLOUD_CPP_GOOGLE_CLOUD_STORAGE_INTERNAL_ASYNC_OPEN_OBJECT_H +#include "google/cloud/storage/internal/async/open_object_metrics.h" #include "google/cloud/storage/internal/async/open_stream.h" #include "google/cloud/storage/internal/storage_stub.h" #include "google/cloud/completion_queue.h" @@ -108,12 +109,10 @@ class OpenObject : public std::enable_shared_from_this { std::shared_ptr rpc_; promise> promise_; google::storage::v2::BidiReadObjectRequest initial_request_; -#ifdef GOOGLE_CLOUD_CPP_STORAGE_WITH_OTEL_METRICS - std::chrono::steady_clock::time_point t0_; - std::chrono::steady_clock::time_point t1_; - std::chrono::steady_clock::time_point t2_; +#ifdef GOOGLE_CLOUD_CPP_HAVE_OPENTELEMETRY opentelemetry::nostd::shared_ptr span_; #endif + OpenObjectMetrics metrics_; }; GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_END diff --git a/google/cloud/storage/internal/async/open_object_metrics.cc b/google/cloud/storage/internal/async/open_object_metrics.cc new file mode 100644 index 0000000000000..7a8d7f5c7eed1 --- /dev/null +++ b/google/cloud/storage/internal/async/open_object_metrics.cc @@ -0,0 +1,133 @@ +// Copyright 2024 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include "google/cloud/storage/internal/async/open_object_metrics.h" + +#ifdef GOOGLE_CLOUD_CPP_STORAGE_WITH_OTEL_METRICS +#include +#endif + +namespace google { +namespace cloud { +namespace storage_internal { +GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_BEGIN + +#ifdef GOOGLE_CLOUD_CPP_STORAGE_WITH_OTEL_METRICS +namespace { +struct StreamOpenMetrics { + opentelemetry::nostd::shared_ptr> + network_handshake; + opentelemetry::nostd::shared_ptr> + server_metadata_latency; + opentelemetry::nostd::shared_ptr> + stream_open_latency; + + static StreamOpenMetrics const& Instance() { + static auto const metrics = [] { + auto meter = + opentelemetry::metrics::Provider::GetMeterProvider()->GetMeter( + "storage", "v1"); + return StreamOpenMetrics{ + meter->CreateDoubleHistogram("gl-cpp.latency.network_handshake", + "Network Handshake", "us"), + meter->CreateDoubleHistogram("gl-cpp.latency.server_metadata", + "Server Metadata Latency", "us"), + meter->CreateDoubleHistogram("gl-cpp.latency.stream_open", + "End-to-End Stream Open", "us")}; + }(); + return metrics; + } +}; +} // namespace +#endif + +void OpenObjectMetrics::RecordCall() { +#ifdef GOOGLE_CLOUD_CPP_STORAGE_WITH_OTEL_METRICS + t0_ = std::chrono::steady_clock::now(); +#endif +} + +void OpenObjectMetrics::RecordStart() { +#ifdef GOOGLE_CLOUD_CPP_STORAGE_WITH_OTEL_METRICS + t1_ = std::chrono::steady_clock::now(); +#endif +} + +void OpenObjectMetrics::RecordWrite() { +#ifdef GOOGLE_CLOUD_CPP_STORAGE_WITH_OTEL_METRICS + t2_ = std::chrono::steady_clock::now(); +#endif +} + +#ifdef GOOGLE_CLOUD_CPP_STORAGE_WITH_OTEL_METRICS +void OpenObjectMetrics::RecordMetrics( + std::string const& bucket, std::chrono::steady_clock::time_point t3) { + auto const& metrics = StreamOpenMetrics::Instance(); + auto p1 = static_cast( + std::chrono::duration_cast(t1_ - t0_).count()); + auto p2 = static_cast( + std::chrono::duration_cast(t3 - t2_).count()); + auto p3 = static_cast( + std::chrono::duration_cast(t3 - t0_).count()); + + metrics.network_handshake->Record(p1, {{"gcp.storage.bucket", bucket}}, + opentelemetry::context::Context{}); + metrics.server_metadata_latency->Record(p2, {{"gcp.storage.bucket", bucket}}, + opentelemetry::context::Context{}); + metrics.stream_open_latency->Record(p3, {{"gcp.storage.bucket", bucket}}, + opentelemetry::context::Context{}); +} +#endif + +#ifdef GOOGLE_CLOUD_CPP_HAVE_OPENTELEMETRY +void OpenObjectMetrics::RecordRead( + std::string const& bucket, + opentelemetry::nostd::shared_ptr const& span) { +#ifdef GOOGLE_CLOUD_CPP_STORAGE_WITH_OTEL_METRICS + auto t3 = std::chrono::steady_clock::now(); + RecordMetrics(bucket, t3); + if (span && span->GetContext().IsValid()) { + auto p1 = static_cast( + std::chrono::duration_cast(t1_ - t0_) + .count()); + auto p2 = static_cast( + std::chrono::duration_cast(t3 - t2_) + .count()); + auto p3 = static_cast( + std::chrono::duration_cast(t3 - t0_) + .count()); + span->AddEvent("gl-cpp.stream_open.latency", + {{"gl-cpp.latency.network_handshake", p1}, + {"gl-cpp.latency.server_metadata", p2}, + {"gl-cpp.latency.stream_open", p3}}); + } +#else + (void)bucket; + (void)span; +#endif +} +#else +void OpenObjectMetrics::RecordRead(std::string const& bucket) { +#ifdef GOOGLE_CLOUD_CPP_STORAGE_WITH_OTEL_METRICS + RecordMetrics(bucket, std::chrono::steady_clock::now()); +#else + (void)bucket; +#endif +} +#endif + +GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_END +} // namespace storage_internal +} // namespace cloud +} // namespace google diff --git a/google/cloud/storage/internal/async/open_object_metrics.h b/google/cloud/storage/internal/async/open_object_metrics.h new file mode 100644 index 0000000000000..56d5bb79b0b74 --- /dev/null +++ b/google/cloud/storage/internal/async/open_object_metrics.h @@ -0,0 +1,61 @@ +// Copyright 2024 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#ifndef GOOGLE_CLOUD_CPP_GOOGLE_CLOUD_STORAGE_INTERNAL_ASYNC_OPEN_OBJECT_METRICS_H +#define GOOGLE_CLOUD_CPP_GOOGLE_CLOUD_STORAGE_INTERNAL_ASYNC_OPEN_OBJECT_METRICS_H + +#include "google/cloud/version.h" +#include +#include + +#ifdef GOOGLE_CLOUD_CPP_HAVE_OPENTELEMETRY +#include +#endif + +namespace google { +namespace cloud { +namespace storage_internal { +GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_BEGIN + +class OpenObjectMetrics { + public: + void RecordCall(); + void RecordStart(); + void RecordWrite(); + +#ifdef GOOGLE_CLOUD_CPP_HAVE_OPENTELEMETRY + void RecordRead( + std::string const& bucket, + opentelemetry::nostd::shared_ptr const& span); +#else + void RecordRead(std::string const& bucket); +#endif + + private: +#ifdef GOOGLE_CLOUD_CPP_STORAGE_WITH_OTEL_METRICS + void RecordMetrics(std::string const& bucket, + std::chrono::steady_clock::time_point t3); + + std::chrono::steady_clock::time_point t0_; + std::chrono::steady_clock::time_point t1_; + std::chrono::steady_clock::time_point t2_; +#endif +}; + +GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_END +} // namespace storage_internal +} // namespace cloud +} // namespace google + +#endif // GOOGLE_CLOUD_CPP_GOOGLE_CLOUD_STORAGE_INTERNAL_ASYNC_OPEN_OBJECT_METRICS_H From e3106e4466285de69bcdebfb5b52cd8ac965545c Mon Sep 17 00:00:00 2001 From: Gauri Kalra Date: Thu, 6 Aug 2026 08:40:21 +0000 Subject: [PATCH 4/4] Address feedback about decoupling tracing from metrics --- .../internal/async/open_object_metrics.cc | 19 +++++++++++-------- .../internal/async/open_object_metrics.h | 3 +++ 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/google/cloud/storage/internal/async/open_object_metrics.cc b/google/cloud/storage/internal/async/open_object_metrics.cc index 7a8d7f5c7eed1..3f989d9684cce 100644 --- a/google/cloud/storage/internal/async/open_object_metrics.cc +++ b/google/cloud/storage/internal/async/open_object_metrics.cc @@ -53,19 +53,22 @@ struct StreamOpenMetrics { #endif void OpenObjectMetrics::RecordCall() { -#ifdef GOOGLE_CLOUD_CPP_STORAGE_WITH_OTEL_METRICS +#if defined(GOOGLE_CLOUD_CPP_STORAGE_WITH_OTEL_METRICS) || \ + defined(GOOGLE_CLOUD_CPP_HAVE_OPENTELEMETRY) t0_ = std::chrono::steady_clock::now(); #endif } void OpenObjectMetrics::RecordStart() { -#ifdef GOOGLE_CLOUD_CPP_STORAGE_WITH_OTEL_METRICS +#if defined(GOOGLE_CLOUD_CPP_STORAGE_WITH_OTEL_METRICS) || \ + defined(GOOGLE_CLOUD_CPP_HAVE_OPENTELEMETRY) t1_ = std::chrono::steady_clock::now(); #endif } void OpenObjectMetrics::RecordWrite() { -#ifdef GOOGLE_CLOUD_CPP_STORAGE_WITH_OTEL_METRICS +#if defined(GOOGLE_CLOUD_CPP_STORAGE_WITH_OTEL_METRICS) || \ + defined(GOOGLE_CLOUD_CPP_HAVE_OPENTELEMETRY) t2_ = std::chrono::steady_clock::now(); #endif } @@ -94,9 +97,13 @@ void OpenObjectMetrics::RecordMetrics( void OpenObjectMetrics::RecordRead( std::string const& bucket, opentelemetry::nostd::shared_ptr const& span) { -#ifdef GOOGLE_CLOUD_CPP_STORAGE_WITH_OTEL_METRICS auto t3 = std::chrono::steady_clock::now(); +#ifdef GOOGLE_CLOUD_CPP_STORAGE_WITH_OTEL_METRICS RecordMetrics(bucket, t3); +#else + (void)bucket; +#endif + if (span && span->GetContext().IsValid()) { auto p1 = static_cast( std::chrono::duration_cast(t1_ - t0_) @@ -112,10 +119,6 @@ void OpenObjectMetrics::RecordRead( {"gl-cpp.latency.server_metadata", p2}, {"gl-cpp.latency.stream_open", p3}}); } -#else - (void)bucket; - (void)span; -#endif } #else void OpenObjectMetrics::RecordRead(std::string const& bucket) { diff --git a/google/cloud/storage/internal/async/open_object_metrics.h b/google/cloud/storage/internal/async/open_object_metrics.h index 56d5bb79b0b74..1d516e31318ed 100644 --- a/google/cloud/storage/internal/async/open_object_metrics.h +++ b/google/cloud/storage/internal/async/open_object_metrics.h @@ -46,7 +46,10 @@ class OpenObjectMetrics { #ifdef GOOGLE_CLOUD_CPP_STORAGE_WITH_OTEL_METRICS void RecordMetrics(std::string const& bucket, std::chrono::steady_clock::time_point t3); +#endif +#if defined(GOOGLE_CLOUD_CPP_STORAGE_WITH_OTEL_METRICS) || \ + defined(GOOGLE_CLOUD_CPP_HAVE_OPENTELEMETRY) std::chrono::steady_clock::time_point t0_; std::chrono::steady_clock::time_point t1_; std::chrono::steady_clock::time_point t2_;