Reviewer comments

pull/35812/head
Yash Tibrewal 1 year ago
parent d2488f41c3
commit dc99764aea
  1. 15
      include/grpcpp/ext/csm_observability.h
  2. 1
      src/cpp/ext/csm/BUILD
  3. 3
      src/cpp/ext/csm/csm_observability.cc
  4. 2
      test/cpp/interop/xds_interop_client.cc
  5. 2
      test/cpp/interop/xds_interop_server.cc

@ -26,7 +26,7 @@
#include "absl/functional/any_invocable.h" #include "absl/functional/any_invocable.h"
#include "absl/status/statusor.h" #include "absl/status/statusor.h"
#include "absl/strings/string_view.h" #include "absl/strings/string_view.h"
#include "opentelemetry/sdk/metrics/meter_provider.h" #include "opentelemetry/metrics/meter_provider.h"
#include <grpcpp/ext/otel_plugin.h> #include <grpcpp/ext/otel_plugin.h>
@ -40,7 +40,15 @@ namespace experimental {
// This is a no-op at present, but in the future, this object would be useful // This is a no-op at present, but in the future, this object would be useful
// for performing cleanup. // for performing cleanup.
class CsmObservability {}; class CsmObservability {
public:
CsmObservability() = default;
// Disable copy constructor and copy-assignment operator.
CsmObservability(const CsmObservability&) = delete;
CsmObservability& operator=(const CsmObservability&) = delete;
CsmObservability(CsmObservability&&) = default;
CsmObservability& operator=(CsmObservability&&) = default;
};
// CsmObservabilityBuilder configures observability for all service mesh traffic // CsmObservabilityBuilder configures observability for all service mesh traffic
// for a binary running on CSM. // for a binary running on CSM.
@ -49,8 +57,7 @@ class CsmObservabilityBuilder {
CsmObservabilityBuilder(); CsmObservabilityBuilder();
~CsmObservabilityBuilder(); ~CsmObservabilityBuilder();
CsmObservabilityBuilder& SetMeterProvider( CsmObservabilityBuilder& SetMeterProvider(
std::shared_ptr<opentelemetry::sdk::metrics::MeterProvider> std::shared_ptr<opentelemetry::metrics::MeterProvider> meter_provider);
meter_provider);
// If set, \a target_attribute_filter is called per channel to decide whether // If set, \a target_attribute_filter is called per channel to decide whether
// to record the target attribute on client or to replace it with "other". // to record the target attribute on client or to replace it with "other".
// This helps reduce the cardinality on metrics in cases where many channels // This helps reduce the cardinality on metrics in cases where many channels

@ -46,6 +46,7 @@ grpc_cc_library(
"absl/types:optional", "absl/types:optional",
"absl/types:variant", "absl/types:variant",
"google_cloud_cpp:opentelemetry", "google_cloud_cpp:opentelemetry",
"otel/api",
"otel/sdk/src/metrics", "otel/sdk/src/metrics",
"otel/sdk:headers", "otel/sdk:headers",
"upb_base_lib", "upb_base_lib",

@ -107,8 +107,7 @@ CsmObservabilityBuilder::CsmObservabilityBuilder()
CsmObservabilityBuilder::~CsmObservabilityBuilder() = default; CsmObservabilityBuilder::~CsmObservabilityBuilder() = default;
CsmObservabilityBuilder& CsmObservabilityBuilder::SetMeterProvider( CsmObservabilityBuilder& CsmObservabilityBuilder::SetMeterProvider(
std::shared_ptr<opentelemetry::sdk::metrics::MeterProvider> std::shared_ptr<opentelemetry::metrics::MeterProvider> meter_provider) {
meter_provider) {
builder_->SetMeterProvider(meter_provider); builder_->SetMeterProvider(meter_provider);
return *this; return *this;
} }

@ -442,7 +442,7 @@ grpc::experimental::CsmObservability EnableCsmObservability() {
.SetMeterProvider(std::move(meter_provider)) .SetMeterProvider(std::move(meter_provider))
.BuildAndRegister(); .BuildAndRegister();
assert(observability.ok()); assert(observability.ok());
return *observability; return *std::move(observability);
} }
void RunServer(const int port, StatsWatchers* stats_watchers, void RunServer(const int port, StatsWatchers* stats_watchers,

@ -58,7 +58,7 @@ grpc::experimental::CsmObservability EnableCsmObservability() {
.SetMeterProvider(std::move(meter_provider)) .SetMeterProvider(std::move(meter_provider))
.BuildAndRegister(); .BuildAndRegister();
assert(observability.ok()); assert(observability.ok());
return *observability; return *std::move(observability);
} }
int main(int argc, char** argv) { int main(int argc, char** argv) {

Loading…
Cancel
Save