From ce982830b2b0f0c15ae6b3c59f1553585c288e89 Mon Sep 17 00:00:00 2001 From: Zach Reyes Date: Thu, 4 Aug 2022 20:12:34 -0400 Subject: [PATCH 1/4] Rewrote observability tests for exporters to use global func --- gcp/observability/observability.go | 2 +- gcp/observability/observability_test.go | 26 +++++++++++------ gcp/observability/opencensus.go | 39 +++++++++++++++++-------- 3 files changed, 45 insertions(+), 22 deletions(-) diff --git a/gcp/observability/observability.go b/gcp/observability/observability.go index 40242692e8d2..1ab20d4cab5b 100644 --- a/gcp/observability/observability.go +++ b/gcp/observability/observability.go @@ -64,7 +64,7 @@ func Start(ctx context.Context) error { } // Enabling tracing and metrics via OpenCensus - if err := startOpenCensus(config, nil); err != nil { + if err := startOpenCensus(config); err != nil { return fmt.Errorf("failed to instrument OpenCensus: %v", err) } diff --git a/gcp/observability/observability_test.go b/gcp/observability/observability_test.go index c5fa59c6648f..18e3fd0f88fc 100644 --- a/gcp/observability/observability_test.go +++ b/gcp/observability/observability_test.go @@ -129,7 +129,6 @@ func (fle *fakeLoggingExporter) Close() error { type test struct { t *testing.T fle *fakeLoggingExporter - fe *fakeOpenCensusExporter testServer testgrpc.TestServiceServer // nil means none // srv and srvAddr are set once startServer is called. @@ -229,8 +228,7 @@ func (te *test) enableOpenCensus() { EnableCloudMonitoring: true, GlobalTraceSamplingRate: 1.0, } - te.fe = &fakeOpenCensusExporter{SeenViews: make(map[string]string), t: te.t} - startOpenCensus(config, te.fe) + startOpenCensus(config) } func checkEventCommon(t *testing.T, seen *grpclogrecordpb.GrpcLogRecord) { @@ -783,6 +781,16 @@ func (s) TestNoEnvSet(t *testing.T) { func (s) TestOpenCensusIntegration(t *testing.T) { te := newTest(t) defer te.tearDown() + fe := &fakeOpenCensusExporter{SeenViews: make(map[string]string), t: te.t} + + defer func(ne func(config *configpb.ObservabilityConfig) (tracingMetricsExporter, error)) { + newExporter = ne + }(newExporter) + + newExporter = func(config *configpb.ObservabilityConfig) (tracingMetricsExporter, error) { + return fe, nil + } + te.enableOpenCensus() te.startServer(&testServer{}) tc := testgrpc.NewTestServiceClient(te.clientConn()) @@ -807,17 +815,17 @@ func (s) TestOpenCensusIntegration(t *testing.T) { defer cancel() for ctx.Err() == nil { errs = nil - te.fe.mu.RLock() - if value := te.fe.SeenViews["grpc.io/client/completed_rpcs"]; value != TypeOpenCensusViewCount { + fe.mu.RLock() + if value := fe.SeenViews["grpc.io/client/completed_rpcs"]; value != TypeOpenCensusViewCount { errs = append(errs, fmt.Errorf("unexpected type for grpc.io/client/completed_rpcs: %s != %s", value, TypeOpenCensusViewCount)) } - if value := te.fe.SeenViews["grpc.io/server/completed_rpcs"]; value != TypeOpenCensusViewCount { + if value := fe.SeenViews["grpc.io/server/completed_rpcs"]; value != TypeOpenCensusViewCount { errs = append(errs, fmt.Errorf("unexpected type for grpc.io/server/completed_rpcs: %s != %s", value, TypeOpenCensusViewCount)) } - if te.fe.SeenSpans <= 0 { - errs = append(errs, fmt.Errorf("unexpected number of seen spans: %v <= 0", te.fe.SeenSpans)) + if fe.SeenSpans <= 0 { + errs = append(errs, fmt.Errorf("unexpected number of seen spans: %v <= 0", fe.SeenSpans)) } - te.fe.mu.RUnlock() + fe.mu.RUnlock() if len(errs) == 0 { break } diff --git a/gcp/observability/opencensus.go b/gcp/observability/opencensus.go index c303eb87f76b..410b90a1bc64 100644 --- a/gcp/observability/opencensus.go +++ b/gcp/observability/opencensus.go @@ -38,26 +38,41 @@ var ( defaultMetricsReportingInterval = time.Second * 30 ) +type tracingMetricsExporter interface { + trace.Exporter + view.Exporter +} + +// globals to stub out in tests +var newExporter = newStackdriverExporter + +func newStackdriverExporter(config *configpb.ObservabilityConfig) (tracingMetricsExporter, error) { + // Create the Stackdriver exporter, which is shared between tracing and stats + mr := monitoredresource.Autodetect() + logger.Infof("Detected MonitoredResource:: %+v", mr) + var err error + exporter, err := stackdriver.NewExporter(stackdriver.Options{ + ProjectID: config.DestinationProjectId, + MonitoredResource: mr, + }) + if err != nil { + return nil, fmt.Errorf("failed to create Stackdriver exporter: %v", err) + } + return exporter, nil +} + // This method accepts config and exporter; the exporter argument is exposed to // assist unit testing of the OpenCensus behavior. -func startOpenCensus(config *configpb.ObservabilityConfig, exporter interface{}) error { +func startOpenCensus(config *configpb.ObservabilityConfig) error { // If both tracing and metrics are disabled, there's no point inject default // StatsHandler. if config == nil || (!config.EnableCloudTrace && !config.EnableCloudMonitoring) { return nil } - if exporter == nil { - // Create the Stackdriver exporter, which is shared between tracing and stats - mr := monitoredresource.Autodetect() - logger.Infof("Detected MonitoredResource:: %+v", mr) - var err error - if exporter, err = stackdriver.NewExporter(stackdriver.Options{ - ProjectID: config.DestinationProjectId, - MonitoredResource: mr, - }); err != nil { - return fmt.Errorf("failed to create Stackdriver exporter: %v", err) - } + exporter, err := newExporter(config) + if err != nil { + return err } var so trace.StartOptions From a930d0b0399e3ad47e2bfdcf97efbdec68aa6f88 Mon Sep 17 00:00:00 2001 From: Zach Reyes Date: Thu, 4 Aug 2022 21:59:01 -0400 Subject: [PATCH 2/4] Added support for custom tags and deleted old implementation --- gcp/observability/exporting.go | 15 +- .../internal/config/config.pb.go | 193 ++++++++++-------- .../internal/config/config.proto | 3 + gcp/observability/logging.go | 2 +- gcp/observability/observability_test.go | 67 +++++- gcp/observability/opencensus.go | 24 ++- gcp/observability/tags.go | 46 ----- gcp/observability/tags_test.go | 64 ------ 8 files changed, 197 insertions(+), 217 deletions(-) delete mode 100644 gcp/observability/tags.go delete mode 100644 gcp/observability/tags_test.go diff --git a/gcp/observability/exporting.go b/gcp/observability/exporting.go index cf95383726c3..ac4d6ea7d065 100644 --- a/gcp/observability/exporting.go +++ b/gcp/observability/exporting.go @@ -22,9 +22,9 @@ import ( "context" "encoding/json" "fmt" - "os" gcplogging "cloud.google.com/go/logging" + configpb "google.golang.org/grpc/gcp/observability/internal/config" grpclogrecordpb "google.golang.org/grpc/gcp/observability/internal/logging" "google.golang.org/protobuf/encoding/protojson" ) @@ -45,20 +45,19 @@ type cloudLoggingExporter struct { logger *gcplogging.Logger } -func newCloudLoggingExporter(ctx context.Context, projectID string) (*cloudLoggingExporter, error) { - c, err := gcplogging.NewClient(ctx, fmt.Sprintf("projects/%v", projectID)) +func newCloudLoggingExporter(ctx context.Context, config *configpb.ObservabilityConfig) (*cloudLoggingExporter, error) { + c, err := gcplogging.NewClient(ctx, fmt.Sprintf("projects/%v", config.DestinationProjectId)) if err != nil { return nil, fmt.Errorf("failed to create cloudLoggingExporter: %v", err) } defer logger.Infof("Successfully created cloudLoggingExporter") - customTags := getCustomTags(os.Environ()) - if len(customTags) != 0 { - logger.Infof("Adding custom tags: %+v", customTags) + if len(config.CustomTags) != 0 { + logger.Infof("Adding custom tags: %+v", config.CustomTags) } return &cloudLoggingExporter{ - projectID: projectID, + projectID: config.DestinationProjectId, client: c, - logger: c.Logger("microservices.googleapis.com/observability/grpc", gcplogging.CommonLabels(customTags)), + logger: c.Logger("microservices.googleapis.com/observability/grpc", gcplogging.CommonLabels(config.CustomTags)), }, nil } diff --git a/gcp/observability/internal/config/config.pb.go b/gcp/observability/internal/config/config.pb.go index a60269c984d3..1c52da52457d 100644 --- a/gcp/observability/internal/config/config.pb.go +++ b/gcp/observability/internal/config/config.pb.go @@ -22,14 +22,13 @@ // Code generated by protoc-gen-go. DO NOT EDIT. // versions: -// protoc-gen-go v1.25.0 -// protoc v3.14.0 -// source: gcp/observability/internal/config/config.proto +// protoc-gen-go v1.28.0 +// protoc v3.15.3 +// source: internal/config/config.proto package config import ( - proto "github.com/golang/protobuf/proto" protoreflect "google.golang.org/protobuf/reflect/protoreflect" protoimpl "google.golang.org/protobuf/runtime/protoimpl" reflect "reflect" @@ -43,10 +42,6 @@ const ( _ = protoimpl.EnforceVersion(protoimpl.MaxVersion - 20) ) -// This is a compile-time assertion that a sufficiently up-to-date version -// of the legacy proto package is being used. -const _ = proto.ProtoPackageIsVersion4 - // Configuration for observability behaviors. By default, no configuration is // required for tracing/metrics/logging to function. This config captures the // most common knobs for gRPC users. It's always possible to override with @@ -76,12 +71,14 @@ type ObservabilityConfig struct { // For example, 0.05 means there is a 5% chance for a RPC to be traced, 1.0 // means trace every call, 0 means don’t start new traces. GlobalTraceSamplingRate float64 `protobuf:"fixed64,6,opt,name=global_trace_sampling_rate,json=globalTraceSamplingRate,proto3" json:"global_trace_sampling_rate,omitempty"` + // A list of custom tags that will be attached to every log entry. + CustomTags map[string]string `protobuf:"bytes,7,rep,name=custom_tags,json=customTags,proto3" json:"custom_tags,omitempty" protobuf_key:"bytes,1,opt,name=key,proto3" protobuf_val:"bytes,2,opt,name=value,proto3"` } func (x *ObservabilityConfig) Reset() { *x = ObservabilityConfig{} if protoimpl.UnsafeEnabled { - mi := &file_gcp_observability_internal_config_config_proto_msgTypes[0] + mi := &file_internal_config_config_proto_msgTypes[0] ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) ms.StoreMessageInfo(mi) } @@ -94,7 +91,7 @@ func (x *ObservabilityConfig) String() string { func (*ObservabilityConfig) ProtoMessage() {} func (x *ObservabilityConfig) ProtoReflect() protoreflect.Message { - mi := &file_gcp_observability_internal_config_config_proto_msgTypes[0] + mi := &file_internal_config_config_proto_msgTypes[0] if protoimpl.UnsafeEnabled && x != nil { ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) if ms.LoadMessageInfo() == nil { @@ -107,7 +104,7 @@ func (x *ObservabilityConfig) ProtoReflect() protoreflect.Message { // Deprecated: Use ObservabilityConfig.ProtoReflect.Descriptor instead. func (*ObservabilityConfig) Descriptor() ([]byte, []int) { - return file_gcp_observability_internal_config_config_proto_rawDescGZIP(), []int{0} + return file_internal_config_config_proto_rawDescGZIP(), []int{0} } func (x *ObservabilityConfig) GetEnableCloudTrace() bool { @@ -152,6 +149,13 @@ func (x *ObservabilityConfig) GetGlobalTraceSamplingRate() float64 { return 0 } +func (x *ObservabilityConfig) GetCustomTags() map[string]string { + if x != nil { + return x.CustomTags + } + return nil +} + type ObservabilityConfig_LogFilter struct { state protoimpl.MessageState sizeCache protoimpl.SizeCache @@ -186,7 +190,7 @@ type ObservabilityConfig_LogFilter struct { func (x *ObservabilityConfig_LogFilter) Reset() { *x = ObservabilityConfig_LogFilter{} if protoimpl.UnsafeEnabled { - mi := &file_gcp_observability_internal_config_config_proto_msgTypes[1] + mi := &file_internal_config_config_proto_msgTypes[1] ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) ms.StoreMessageInfo(mi) } @@ -199,7 +203,7 @@ func (x *ObservabilityConfig_LogFilter) String() string { func (*ObservabilityConfig_LogFilter) ProtoMessage() {} func (x *ObservabilityConfig_LogFilter) ProtoReflect() protoreflect.Message { - mi := &file_gcp_observability_internal_config_config_proto_msgTypes[1] + mi := &file_internal_config_config_proto_msgTypes[1] if protoimpl.UnsafeEnabled && x != nil { ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) if ms.LoadMessageInfo() == nil { @@ -212,7 +216,7 @@ func (x *ObservabilityConfig_LogFilter) ProtoReflect() protoreflect.Message { // Deprecated: Use ObservabilityConfig_LogFilter.ProtoReflect.Descriptor instead. func (*ObservabilityConfig_LogFilter) Descriptor() ([]byte, []int) { - return file_gcp_observability_internal_config_config_proto_rawDescGZIP(), []int{0, 0} + return file_internal_config_config_proto_rawDescGZIP(), []int{0, 0} } func (x *ObservabilityConfig_LogFilter) GetPattern() string { @@ -236,89 +240,100 @@ func (x *ObservabilityConfig_LogFilter) GetMessageBytes() int32 { return 0 } -var File_gcp_observability_internal_config_config_proto protoreflect.FileDescriptor - -var file_gcp_observability_internal_config_config_proto_rawDesc = []byte{ - 0x0a, 0x2e, 0x67, 0x63, 0x70, 0x2f, 0x6f, 0x62, 0x73, 0x65, 0x72, 0x76, 0x61, 0x62, 0x69, 0x6c, - 0x69, 0x74, 0x79, 0x2f, 0x69, 0x6e, 0x74, 0x65, 0x72, 0x6e, 0x61, 0x6c, 0x2f, 0x63, 0x6f, 0x6e, - 0x66, 0x69, 0x67, 0x2f, 0x63, 0x6f, 0x6e, 0x66, 0x69, 0x67, 0x2e, 0x70, 0x72, 0x6f, 0x74, 0x6f, - 0x12, 0x21, 0x67, 0x72, 0x70, 0x63, 0x2e, 0x6f, 0x62, 0x73, 0x65, 0x72, 0x76, 0x61, 0x62, 0x69, - 0x6c, 0x69, 0x74, 0x79, 0x2e, 0x63, 0x6f, 0x6e, 0x66, 0x69, 0x67, 0x2e, 0x76, 0x31, 0x61, 0x6c, - 0x70, 0x68, 0x61, 0x22, 0xf2, 0x03, 0x0a, 0x13, 0x4f, 0x62, 0x73, 0x65, 0x72, 0x76, 0x61, 0x62, - 0x69, 0x6c, 0x69, 0x74, 0x79, 0x43, 0x6f, 0x6e, 0x66, 0x69, 0x67, 0x12, 0x2c, 0x0a, 0x12, 0x65, - 0x6e, 0x61, 0x62, 0x6c, 0x65, 0x5f, 0x63, 0x6c, 0x6f, 0x75, 0x64, 0x5f, 0x74, 0x72, 0x61, 0x63, - 0x65, 0x18, 0x04, 0x20, 0x01, 0x28, 0x08, 0x52, 0x10, 0x65, 0x6e, 0x61, 0x62, 0x6c, 0x65, 0x43, - 0x6c, 0x6f, 0x75, 0x64, 0x54, 0x72, 0x61, 0x63, 0x65, 0x12, 0x36, 0x0a, 0x17, 0x65, 0x6e, 0x61, - 0x62, 0x6c, 0x65, 0x5f, 0x63, 0x6c, 0x6f, 0x75, 0x64, 0x5f, 0x6d, 0x6f, 0x6e, 0x69, 0x74, 0x6f, - 0x72, 0x69, 0x6e, 0x67, 0x18, 0x05, 0x20, 0x01, 0x28, 0x08, 0x52, 0x15, 0x65, 0x6e, 0x61, 0x62, - 0x6c, 0x65, 0x43, 0x6c, 0x6f, 0x75, 0x64, 0x4d, 0x6f, 0x6e, 0x69, 0x74, 0x6f, 0x72, 0x69, 0x6e, - 0x67, 0x12, 0x30, 0x0a, 0x14, 0x65, 0x6e, 0x61, 0x62, 0x6c, 0x65, 0x5f, 0x63, 0x6c, 0x6f, 0x75, - 0x64, 0x5f, 0x6c, 0x6f, 0x67, 0x67, 0x69, 0x6e, 0x67, 0x18, 0x01, 0x20, 0x01, 0x28, 0x08, 0x52, - 0x12, 0x65, 0x6e, 0x61, 0x62, 0x6c, 0x65, 0x43, 0x6c, 0x6f, 0x75, 0x64, 0x4c, 0x6f, 0x67, 0x67, - 0x69, 0x6e, 0x67, 0x12, 0x34, 0x0a, 0x16, 0x64, 0x65, 0x73, 0x74, 0x69, 0x6e, 0x61, 0x74, 0x69, - 0x6f, 0x6e, 0x5f, 0x70, 0x72, 0x6f, 0x6a, 0x65, 0x63, 0x74, 0x5f, 0x69, 0x64, 0x18, 0x02, 0x20, - 0x01, 0x28, 0x09, 0x52, 0x14, 0x64, 0x65, 0x73, 0x74, 0x69, 0x6e, 0x61, 0x74, 0x69, 0x6f, 0x6e, - 0x50, 0x72, 0x6f, 0x6a, 0x65, 0x63, 0x74, 0x49, 0x64, 0x12, 0x61, 0x0a, 0x0b, 0x6c, 0x6f, 0x67, - 0x5f, 0x66, 0x69, 0x6c, 0x74, 0x65, 0x72, 0x73, 0x18, 0x03, 0x20, 0x03, 0x28, 0x0b, 0x32, 0x40, - 0x2e, 0x67, 0x72, 0x70, 0x63, 0x2e, 0x6f, 0x62, 0x73, 0x65, 0x72, 0x76, 0x61, 0x62, 0x69, 0x6c, - 0x69, 0x74, 0x79, 0x2e, 0x63, 0x6f, 0x6e, 0x66, 0x69, 0x67, 0x2e, 0x76, 0x31, 0x61, 0x6c, 0x70, - 0x68, 0x61, 0x2e, 0x4f, 0x62, 0x73, 0x65, 0x72, 0x76, 0x61, 0x62, 0x69, 0x6c, 0x69, 0x74, 0x79, - 0x43, 0x6f, 0x6e, 0x66, 0x69, 0x67, 0x2e, 0x4c, 0x6f, 0x67, 0x46, 0x69, 0x6c, 0x74, 0x65, 0x72, - 0x52, 0x0a, 0x6c, 0x6f, 0x67, 0x46, 0x69, 0x6c, 0x74, 0x65, 0x72, 0x73, 0x12, 0x3b, 0x0a, 0x1a, - 0x67, 0x6c, 0x6f, 0x62, 0x61, 0x6c, 0x5f, 0x74, 0x72, 0x61, 0x63, 0x65, 0x5f, 0x73, 0x61, 0x6d, - 0x70, 0x6c, 0x69, 0x6e, 0x67, 0x5f, 0x72, 0x61, 0x74, 0x65, 0x18, 0x06, 0x20, 0x01, 0x28, 0x01, - 0x52, 0x17, 0x67, 0x6c, 0x6f, 0x62, 0x61, 0x6c, 0x54, 0x72, 0x61, 0x63, 0x65, 0x53, 0x61, 0x6d, - 0x70, 0x6c, 0x69, 0x6e, 0x67, 0x52, 0x61, 0x74, 0x65, 0x1a, 0x6d, 0x0a, 0x09, 0x4c, 0x6f, 0x67, - 0x46, 0x69, 0x6c, 0x74, 0x65, 0x72, 0x12, 0x18, 0x0a, 0x07, 0x70, 0x61, 0x74, 0x74, 0x65, 0x72, - 0x6e, 0x18, 0x01, 0x20, 0x01, 0x28, 0x09, 0x52, 0x07, 0x70, 0x61, 0x74, 0x74, 0x65, 0x72, 0x6e, - 0x12, 0x21, 0x0a, 0x0c, 0x68, 0x65, 0x61, 0x64, 0x65, 0x72, 0x5f, 0x62, 0x79, 0x74, 0x65, 0x73, - 0x18, 0x02, 0x20, 0x01, 0x28, 0x05, 0x52, 0x0b, 0x68, 0x65, 0x61, 0x64, 0x65, 0x72, 0x42, 0x79, - 0x74, 0x65, 0x73, 0x12, 0x23, 0x0a, 0x0d, 0x6d, 0x65, 0x73, 0x73, 0x61, 0x67, 0x65, 0x5f, 0x62, - 0x79, 0x74, 0x65, 0x73, 0x18, 0x03, 0x20, 0x01, 0x28, 0x05, 0x52, 0x0c, 0x6d, 0x65, 0x73, 0x73, - 0x61, 0x67, 0x65, 0x42, 0x79, 0x74, 0x65, 0x73, 0x42, 0x74, 0x0a, 0x1c, 0x69, 0x6f, 0x2e, 0x67, +var File_internal_config_config_proto protoreflect.FileDescriptor + +var file_internal_config_config_proto_rawDesc = []byte{ + 0x0a, 0x1c, 0x69, 0x6e, 0x74, 0x65, 0x72, 0x6e, 0x61, 0x6c, 0x2f, 0x63, 0x6f, 0x6e, 0x66, 0x69, + 0x67, 0x2f, 0x63, 0x6f, 0x6e, 0x66, 0x69, 0x67, 0x2e, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x12, 0x21, + 0x67, 0x72, 0x70, 0x63, 0x2e, 0x6f, 0x62, 0x73, 0x65, 0x72, 0x76, 0x61, 0x62, 0x69, 0x6c, 0x69, + 0x74, 0x79, 0x2e, 0x63, 0x6f, 0x6e, 0x66, 0x69, 0x67, 0x2e, 0x76, 0x31, 0x61, 0x6c, 0x70, 0x68, + 0x61, 0x22, 0x9a, 0x05, 0x0a, 0x13, 0x4f, 0x62, 0x73, 0x65, 0x72, 0x76, 0x61, 0x62, 0x69, 0x6c, + 0x69, 0x74, 0x79, 0x43, 0x6f, 0x6e, 0x66, 0x69, 0x67, 0x12, 0x2c, 0x0a, 0x12, 0x65, 0x6e, 0x61, + 0x62, 0x6c, 0x65, 0x5f, 0x63, 0x6c, 0x6f, 0x75, 0x64, 0x5f, 0x74, 0x72, 0x61, 0x63, 0x65, 0x18, + 0x04, 0x20, 0x01, 0x28, 0x08, 0x52, 0x10, 0x65, 0x6e, 0x61, 0x62, 0x6c, 0x65, 0x43, 0x6c, 0x6f, + 0x75, 0x64, 0x54, 0x72, 0x61, 0x63, 0x65, 0x12, 0x36, 0x0a, 0x17, 0x65, 0x6e, 0x61, 0x62, 0x6c, + 0x65, 0x5f, 0x63, 0x6c, 0x6f, 0x75, 0x64, 0x5f, 0x6d, 0x6f, 0x6e, 0x69, 0x74, 0x6f, 0x72, 0x69, + 0x6e, 0x67, 0x18, 0x05, 0x20, 0x01, 0x28, 0x08, 0x52, 0x15, 0x65, 0x6e, 0x61, 0x62, 0x6c, 0x65, + 0x43, 0x6c, 0x6f, 0x75, 0x64, 0x4d, 0x6f, 0x6e, 0x69, 0x74, 0x6f, 0x72, 0x69, 0x6e, 0x67, 0x12, + 0x30, 0x0a, 0x14, 0x65, 0x6e, 0x61, 0x62, 0x6c, 0x65, 0x5f, 0x63, 0x6c, 0x6f, 0x75, 0x64, 0x5f, + 0x6c, 0x6f, 0x67, 0x67, 0x69, 0x6e, 0x67, 0x18, 0x01, 0x20, 0x01, 0x28, 0x08, 0x52, 0x12, 0x65, + 0x6e, 0x61, 0x62, 0x6c, 0x65, 0x43, 0x6c, 0x6f, 0x75, 0x64, 0x4c, 0x6f, 0x67, 0x67, 0x69, 0x6e, + 0x67, 0x12, 0x34, 0x0a, 0x16, 0x64, 0x65, 0x73, 0x74, 0x69, 0x6e, 0x61, 0x74, 0x69, 0x6f, 0x6e, + 0x5f, 0x70, 0x72, 0x6f, 0x6a, 0x65, 0x63, 0x74, 0x5f, 0x69, 0x64, 0x18, 0x02, 0x20, 0x01, 0x28, + 0x09, 0x52, 0x14, 0x64, 0x65, 0x73, 0x74, 0x69, 0x6e, 0x61, 0x74, 0x69, 0x6f, 0x6e, 0x50, 0x72, + 0x6f, 0x6a, 0x65, 0x63, 0x74, 0x49, 0x64, 0x12, 0x61, 0x0a, 0x0b, 0x6c, 0x6f, 0x67, 0x5f, 0x66, + 0x69, 0x6c, 0x74, 0x65, 0x72, 0x73, 0x18, 0x03, 0x20, 0x03, 0x28, 0x0b, 0x32, 0x40, 0x2e, 0x67, + 0x72, 0x70, 0x63, 0x2e, 0x6f, 0x62, 0x73, 0x65, 0x72, 0x76, 0x61, 0x62, 0x69, 0x6c, 0x69, 0x74, + 0x79, 0x2e, 0x63, 0x6f, 0x6e, 0x66, 0x69, 0x67, 0x2e, 0x76, 0x31, 0x61, 0x6c, 0x70, 0x68, 0x61, + 0x2e, 0x4f, 0x62, 0x73, 0x65, 0x72, 0x76, 0x61, 0x62, 0x69, 0x6c, 0x69, 0x74, 0x79, 0x43, 0x6f, + 0x6e, 0x66, 0x69, 0x67, 0x2e, 0x4c, 0x6f, 0x67, 0x46, 0x69, 0x6c, 0x74, 0x65, 0x72, 0x52, 0x0a, + 0x6c, 0x6f, 0x67, 0x46, 0x69, 0x6c, 0x74, 0x65, 0x72, 0x73, 0x12, 0x3b, 0x0a, 0x1a, 0x67, 0x6c, + 0x6f, 0x62, 0x61, 0x6c, 0x5f, 0x74, 0x72, 0x61, 0x63, 0x65, 0x5f, 0x73, 0x61, 0x6d, 0x70, 0x6c, + 0x69, 0x6e, 0x67, 0x5f, 0x72, 0x61, 0x74, 0x65, 0x18, 0x06, 0x20, 0x01, 0x28, 0x01, 0x52, 0x17, + 0x67, 0x6c, 0x6f, 0x62, 0x61, 0x6c, 0x54, 0x72, 0x61, 0x63, 0x65, 0x53, 0x61, 0x6d, 0x70, 0x6c, + 0x69, 0x6e, 0x67, 0x52, 0x61, 0x74, 0x65, 0x12, 0x67, 0x0a, 0x0b, 0x63, 0x75, 0x73, 0x74, 0x6f, + 0x6d, 0x5f, 0x74, 0x61, 0x67, 0x73, 0x18, 0x07, 0x20, 0x03, 0x28, 0x0b, 0x32, 0x46, 0x2e, 0x67, 0x72, 0x70, 0x63, 0x2e, 0x6f, 0x62, 0x73, 0x65, 0x72, 0x76, 0x61, 0x62, 0x69, 0x6c, 0x69, 0x74, - 0x79, 0x2e, 0x63, 0x6f, 0x6e, 0x66, 0x69, 0x67, 0x42, 0x18, 0x4f, 0x62, 0x73, 0x65, 0x72, 0x76, - 0x61, 0x62, 0x69, 0x6c, 0x69, 0x74, 0x79, 0x43, 0x6f, 0x6e, 0x66, 0x69, 0x67, 0x50, 0x72, 0x6f, - 0x74, 0x6f, 0x50, 0x01, 0x5a, 0x38, 0x67, 0x6f, 0x6f, 0x67, 0x6c, 0x65, 0x2e, 0x67, 0x6f, 0x6c, - 0x61, 0x6e, 0x67, 0x2e, 0x6f, 0x72, 0x67, 0x2f, 0x67, 0x72, 0x70, 0x63, 0x2f, 0x67, 0x63, 0x70, - 0x2f, 0x6f, 0x62, 0x73, 0x65, 0x72, 0x76, 0x61, 0x62, 0x69, 0x6c, 0x69, 0x74, 0x79, 0x2f, 0x69, - 0x6e, 0x74, 0x65, 0x72, 0x6e, 0x61, 0x6c, 0x2f, 0x63, 0x6f, 0x6e, 0x66, 0x69, 0x67, 0x62, 0x06, - 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x33, + 0x79, 0x2e, 0x63, 0x6f, 0x6e, 0x66, 0x69, 0x67, 0x2e, 0x76, 0x31, 0x61, 0x6c, 0x70, 0x68, 0x61, + 0x2e, 0x4f, 0x62, 0x73, 0x65, 0x72, 0x76, 0x61, 0x62, 0x69, 0x6c, 0x69, 0x74, 0x79, 0x43, 0x6f, + 0x6e, 0x66, 0x69, 0x67, 0x2e, 0x43, 0x75, 0x73, 0x74, 0x6f, 0x6d, 0x54, 0x61, 0x67, 0x73, 0x45, + 0x6e, 0x74, 0x72, 0x79, 0x52, 0x0a, 0x63, 0x75, 0x73, 0x74, 0x6f, 0x6d, 0x54, 0x61, 0x67, 0x73, + 0x1a, 0x6d, 0x0a, 0x09, 0x4c, 0x6f, 0x67, 0x46, 0x69, 0x6c, 0x74, 0x65, 0x72, 0x12, 0x18, 0x0a, + 0x07, 0x70, 0x61, 0x74, 0x74, 0x65, 0x72, 0x6e, 0x18, 0x01, 0x20, 0x01, 0x28, 0x09, 0x52, 0x07, + 0x70, 0x61, 0x74, 0x74, 0x65, 0x72, 0x6e, 0x12, 0x21, 0x0a, 0x0c, 0x68, 0x65, 0x61, 0x64, 0x65, + 0x72, 0x5f, 0x62, 0x79, 0x74, 0x65, 0x73, 0x18, 0x02, 0x20, 0x01, 0x28, 0x05, 0x52, 0x0b, 0x68, + 0x65, 0x61, 0x64, 0x65, 0x72, 0x42, 0x79, 0x74, 0x65, 0x73, 0x12, 0x23, 0x0a, 0x0d, 0x6d, 0x65, + 0x73, 0x73, 0x61, 0x67, 0x65, 0x5f, 0x62, 0x79, 0x74, 0x65, 0x73, 0x18, 0x03, 0x20, 0x01, 0x28, + 0x05, 0x52, 0x0c, 0x6d, 0x65, 0x73, 0x73, 0x61, 0x67, 0x65, 0x42, 0x79, 0x74, 0x65, 0x73, 0x1a, + 0x3d, 0x0a, 0x0f, 0x43, 0x75, 0x73, 0x74, 0x6f, 0x6d, 0x54, 0x61, 0x67, 0x73, 0x45, 0x6e, 0x74, + 0x72, 0x79, 0x12, 0x10, 0x0a, 0x03, 0x6b, 0x65, 0x79, 0x18, 0x01, 0x20, 0x01, 0x28, 0x09, 0x52, + 0x03, 0x6b, 0x65, 0x79, 0x12, 0x14, 0x0a, 0x05, 0x76, 0x61, 0x6c, 0x75, 0x65, 0x18, 0x02, 0x20, + 0x01, 0x28, 0x09, 0x52, 0x05, 0x76, 0x61, 0x6c, 0x75, 0x65, 0x3a, 0x02, 0x38, 0x01, 0x42, 0x74, + 0x0a, 0x1c, 0x69, 0x6f, 0x2e, 0x67, 0x72, 0x70, 0x63, 0x2e, 0x6f, 0x62, 0x73, 0x65, 0x72, 0x76, + 0x61, 0x62, 0x69, 0x6c, 0x69, 0x74, 0x79, 0x2e, 0x63, 0x6f, 0x6e, 0x66, 0x69, 0x67, 0x42, 0x18, + 0x4f, 0x62, 0x73, 0x65, 0x72, 0x76, 0x61, 0x62, 0x69, 0x6c, 0x69, 0x74, 0x79, 0x43, 0x6f, 0x6e, + 0x66, 0x69, 0x67, 0x50, 0x72, 0x6f, 0x74, 0x6f, 0x50, 0x01, 0x5a, 0x38, 0x67, 0x6f, 0x6f, 0x67, + 0x6c, 0x65, 0x2e, 0x67, 0x6f, 0x6c, 0x61, 0x6e, 0x67, 0x2e, 0x6f, 0x72, 0x67, 0x2f, 0x67, 0x72, + 0x70, 0x63, 0x2f, 0x67, 0x63, 0x70, 0x2f, 0x6f, 0x62, 0x73, 0x65, 0x72, 0x76, 0x61, 0x62, 0x69, + 0x6c, 0x69, 0x74, 0x79, 0x2f, 0x69, 0x6e, 0x74, 0x65, 0x72, 0x6e, 0x61, 0x6c, 0x2f, 0x63, 0x6f, + 0x6e, 0x66, 0x69, 0x67, 0x62, 0x06, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x33, } var ( - file_gcp_observability_internal_config_config_proto_rawDescOnce sync.Once - file_gcp_observability_internal_config_config_proto_rawDescData = file_gcp_observability_internal_config_config_proto_rawDesc + file_internal_config_config_proto_rawDescOnce sync.Once + file_internal_config_config_proto_rawDescData = file_internal_config_config_proto_rawDesc ) -func file_gcp_observability_internal_config_config_proto_rawDescGZIP() []byte { - file_gcp_observability_internal_config_config_proto_rawDescOnce.Do(func() { - file_gcp_observability_internal_config_config_proto_rawDescData = protoimpl.X.CompressGZIP(file_gcp_observability_internal_config_config_proto_rawDescData) +func file_internal_config_config_proto_rawDescGZIP() []byte { + file_internal_config_config_proto_rawDescOnce.Do(func() { + file_internal_config_config_proto_rawDescData = protoimpl.X.CompressGZIP(file_internal_config_config_proto_rawDescData) }) - return file_gcp_observability_internal_config_config_proto_rawDescData + return file_internal_config_config_proto_rawDescData } -var file_gcp_observability_internal_config_config_proto_msgTypes = make([]protoimpl.MessageInfo, 2) -var file_gcp_observability_internal_config_config_proto_goTypes = []interface{}{ +var file_internal_config_config_proto_msgTypes = make([]protoimpl.MessageInfo, 3) +var file_internal_config_config_proto_goTypes = []interface{}{ (*ObservabilityConfig)(nil), // 0: grpc.observability.config.v1alpha.ObservabilityConfig (*ObservabilityConfig_LogFilter)(nil), // 1: grpc.observability.config.v1alpha.ObservabilityConfig.LogFilter + nil, // 2: grpc.observability.config.v1alpha.ObservabilityConfig.CustomTagsEntry } -var file_gcp_observability_internal_config_config_proto_depIdxs = []int32{ +var file_internal_config_config_proto_depIdxs = []int32{ 1, // 0: grpc.observability.config.v1alpha.ObservabilityConfig.log_filters:type_name -> grpc.observability.config.v1alpha.ObservabilityConfig.LogFilter - 1, // [1:1] is the sub-list for method output_type - 1, // [1:1] is the sub-list for method input_type - 1, // [1:1] is the sub-list for extension type_name - 1, // [1:1] is the sub-list for extension extendee - 0, // [0:1] is the sub-list for field type_name + 2, // 1: grpc.observability.config.v1alpha.ObservabilityConfig.custom_tags:type_name -> grpc.observability.config.v1alpha.ObservabilityConfig.CustomTagsEntry + 2, // [2:2] is the sub-list for method output_type + 2, // [2:2] is the sub-list for method input_type + 2, // [2:2] is the sub-list for extension type_name + 2, // [2:2] is the sub-list for extension extendee + 0, // [0:2] is the sub-list for field type_name } -func init() { file_gcp_observability_internal_config_config_proto_init() } -func file_gcp_observability_internal_config_config_proto_init() { - if File_gcp_observability_internal_config_config_proto != nil { +func init() { file_internal_config_config_proto_init() } +func file_internal_config_config_proto_init() { + if File_internal_config_config_proto != nil { return } if !protoimpl.UnsafeEnabled { - file_gcp_observability_internal_config_config_proto_msgTypes[0].Exporter = func(v interface{}, i int) interface{} { + file_internal_config_config_proto_msgTypes[0].Exporter = func(v interface{}, i int) interface{} { switch v := v.(*ObservabilityConfig); i { case 0: return &v.state @@ -330,7 +345,7 @@ func file_gcp_observability_internal_config_config_proto_init() { return nil } } - file_gcp_observability_internal_config_config_proto_msgTypes[1].Exporter = func(v interface{}, i int) interface{} { + file_internal_config_config_proto_msgTypes[1].Exporter = func(v interface{}, i int) interface{} { switch v := v.(*ObservabilityConfig_LogFilter); i { case 0: return &v.state @@ -347,18 +362,18 @@ func file_gcp_observability_internal_config_config_proto_init() { out := protoimpl.TypeBuilder{ File: protoimpl.DescBuilder{ GoPackagePath: reflect.TypeOf(x{}).PkgPath(), - RawDescriptor: file_gcp_observability_internal_config_config_proto_rawDesc, + RawDescriptor: file_internal_config_config_proto_rawDesc, NumEnums: 0, - NumMessages: 2, + NumMessages: 3, NumExtensions: 0, NumServices: 0, }, - GoTypes: file_gcp_observability_internal_config_config_proto_goTypes, - DependencyIndexes: file_gcp_observability_internal_config_config_proto_depIdxs, - MessageInfos: file_gcp_observability_internal_config_config_proto_msgTypes, + GoTypes: file_internal_config_config_proto_goTypes, + DependencyIndexes: file_internal_config_config_proto_depIdxs, + MessageInfos: file_internal_config_config_proto_msgTypes, }.Build() - File_gcp_observability_internal_config_config_proto = out.File - file_gcp_observability_internal_config_config_proto_rawDesc = nil - file_gcp_observability_internal_config_config_proto_goTypes = nil - file_gcp_observability_internal_config_config_proto_depIdxs = nil + File_internal_config_config_proto = out.File + file_internal_config_config_proto_rawDesc = nil + file_internal_config_config_proto_goTypes = nil + file_internal_config_config_proto_depIdxs = nil } diff --git a/gcp/observability/internal/config/config.proto b/gcp/observability/internal/config/config.proto index 2c108bfa2abf..685bf2d84eaf 100644 --- a/gcp/observability/internal/config/config.proto +++ b/gcp/observability/internal/config/config.proto @@ -86,4 +86,7 @@ message ObservabilityConfig { // For example, 0.05 means there is a 5% chance for a RPC to be traced, 1.0 // means trace every call, 0 means don’t start new traces. double global_trace_sampling_rate = 6; + + // A list of custom tags that will be attached to every log entry. + map custom_tags = 7; } diff --git a/gcp/observability/logging.go b/gcp/observability/logging.go index ed7e76d74c04..711dc74f8672 100644 --- a/gcp/observability/logging.go +++ b/gcp/observability/logging.go @@ -325,7 +325,7 @@ func (l *binaryLogger) Start(ctx context.Context, config *configpb.Observability if config.GetDestinationProjectId() == "" { return fmt.Errorf("failed to enable CloudLogging: empty destination_project_id") } - exporter, err := newCloudLoggingExporter(ctx, config.DestinationProjectId) + exporter, err := newCloudLoggingExporter(ctx, config) if err != nil { return fmt.Errorf("unable to create CloudLogging exporter: %v", err) } diff --git a/gcp/observability/observability_test.go b/gcp/observability/observability_test.go index 18e3fd0f88fc..962f56f1d120 100644 --- a/gcp/observability/observability_test.go +++ b/gcp/observability/observability_test.go @@ -36,6 +36,7 @@ import ( "google.golang.org/grpc/credentials/insecure" configpb "google.golang.org/grpc/gcp/observability/internal/config" grpclogrecordpb "google.golang.org/grpc/gcp/observability/internal/logging" + "google.golang.org/grpc/internal" iblog "google.golang.org/grpc/internal/binarylog" "google.golang.org/grpc/internal/grpctest" "google.golang.org/grpc/internal/leakcheck" @@ -687,7 +688,7 @@ func (s) TestRefuseStartWithInvalidPatterns(t *testing.T) { // place in the temporary portion of the file system dependent on system. It // also sets the environment variable GRPC_CONFIG_OBSERVABILITY_JSON to point to // this created config. -func createTmpConfigInFileSystem(rawJSON string) (*os.File, error) { +func createTmpConfigInFileSystem(rawJSON string) (func(), error) { configJSONFile, err := ioutil.TempFile(os.TempDir(), "configJSON-") if err != nil { return nil, fmt.Errorf("cannot create file %v: %v", configJSONFile.Name(), err) @@ -697,7 +698,10 @@ func createTmpConfigInFileSystem(rawJSON string) (*os.File, error) { return nil, fmt.Errorf("cannot write marshalled JSON: %v", err) } os.Setenv(envObservabilityConfigJSON, configJSONFile.Name()) - return configJSONFile, nil + return func() { + configJSONFile.Close() + os.Setenv(envObservabilityConfigJSON, "") + }, nil } // TestJSONEnvVarSet tests a valid observability configuration specified by the @@ -708,8 +712,9 @@ func (s) TestJSONEnvVarSet(t *testing.T) { "destinationProjectId": "fake", "logFilters":[{"pattern":"*","headerBytes":1073741824,"messageBytes":1073741824}] }` - configJSONFile, err := createTmpConfigInFileSystem(configJSON) - defer configJSONFile.Close() + cleanup, err := createTmpConfigInFileSystem(configJSON) + defer cleanup() + if err != nil { t.Fatalf("failed to create config in file system: %v", err) } @@ -731,8 +736,8 @@ func (s) TestBothConfigEnvVarsSet(t *testing.T) { "destinationProjectId":"fake", "logFilters":[{"pattern":":-)"}, {"pattern":"*"}] }` - configJSONFile, err := createTmpConfigInFileSystem(configJSON) - defer configJSONFile.Close() + cleanup, err := createTmpConfigInFileSystem(configJSON) + defer cleanup() if err != nil { t.Fatalf("failed to create config in file system: %v", err) } @@ -764,6 +769,7 @@ func (s) TestBothConfigEnvVarsSet(t *testing.T) { // a file (or valid configuration). func (s) TestErrInFileSystemEnvVar(t *testing.T) { os.Setenv(envObservabilityConfigJSON, "/this-file/does-not-exist") + defer os.Setenv(envObservabilityConfigJSON, "") if err := Start(context.Background()); err == nil { t.Fatalf("Invalid file system path not triggering error") } @@ -835,3 +841,52 @@ func (s) TestOpenCensusIntegration(t *testing.T) { t.Fatalf("Invalid OpenCensus export data: %v", errs) } } + +// TestCustomTagsTracingMetrics verifies that the custom tags defined in our +// observability configuration and set to two hardcoded values are passed to the +// function to create an exporter. +func (s) TestCustomTagsTracingMetrics(t *testing.T) { + defer func(ne func(config *configpb.ObservabilityConfig) (tracingMetricsExporter, error)) { + newExporter = ne + }(newExporter) + fe := &fakeOpenCensusExporter{SeenViews: make(map[string]string), t: t} + newExporter = func(config *configpb.ObservabilityConfig) (tracingMetricsExporter, error) { + ct := config.GetCustomTags() + if len(ct) < 1 { + t.Fatalf("less than 2 custom tags sent in") + } + if val, ok := ct["customtag1"]; !ok || val != "wow" { + t.Fatalf("incorrect custom tag: got %v, want %v", val, "wow") + } + if val, ok := ct["customtag2"]; !ok || val != "nice" { + t.Fatalf("incorrect custom tag: got %v, want %v", val, "nice") + } + return fe, nil + } + + // This configuration present in file system and it's defined custom tags should make it + // to the created exporter. + configJSON := `{ + "destinationProjectId": "fake", + "enableCloudTrace": true, + "enableCloudMonitoring": true, + "globalTraceSamplingRate": 1.0, + "customTags":{"customtag1":"wow","customtag2":"nice"} + }` + cleanup, err := createTmpConfigInFileSystem(configJSON) + defer cleanup() + + // To clear globally registered tracing and metrics exporters. + defer func() { + internal.ClearExtraDialOptions() + internal.ClearExtraServerOptions() + }() + + ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout) + defer cancel() + err = Start(ctx) + defer End() + if err != nil { + t.Fatalf("Start() failed with err: %v", err) + } +} diff --git a/gcp/observability/opencensus.go b/gcp/observability/opencensus.go index 410b90a1bc64..df0eba95dfd9 100644 --- a/gcp/observability/opencensus.go +++ b/gcp/observability/opencensus.go @@ -38,12 +38,28 @@ var ( defaultMetricsReportingInterval = time.Second * 30 ) +func convertTagsToMonitoringLabels(tags map[string]string) *stackdriver.Labels { + labels := &stackdriver.Labels{} + for k, v := range tags { + labels.Set(k, v, "") + } + return labels +} + +func convertTagsToTraceAttributes(tags map[string]string) map[string]interface{} { + ta := make(map[string]interface{}, len(tags)) + for k, v := range tags { + ta[k] = v + } + return ta +} + type tracingMetricsExporter interface { trace.Exporter view.Exporter } -// globals to stub out in tests +// global to stub out in tests var newExporter = newStackdriverExporter func newStackdriverExporter(config *configpb.ObservabilityConfig) (tracingMetricsExporter, error) { @@ -52,8 +68,10 @@ func newStackdriverExporter(config *configpb.ObservabilityConfig) (tracingMetric logger.Infof("Detected MonitoredResource:: %+v", mr) var err error exporter, err := stackdriver.NewExporter(stackdriver.Options{ - ProjectID: config.DestinationProjectId, - MonitoredResource: mr, + ProjectID: config.DestinationProjectId, + MonitoredResource: mr, + DefaultMonitoringLabels: convertTagsToMonitoringLabels(config.CustomTags), + DefaultTraceAttributes: convertTagsToTraceAttributes(config.CustomTags), }) if err != nil { return nil, fmt.Errorf("failed to create Stackdriver exporter: %v", err) diff --git a/gcp/observability/tags.go b/gcp/observability/tags.go deleted file mode 100644 index c9a900970ea9..000000000000 --- a/gcp/observability/tags.go +++ /dev/null @@ -1,46 +0,0 @@ -/* - * - * Copyright 2022 gRPC authors. - * - * 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 - * - * http://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. - * - */ - -package observability - -import ( - "strings" -) - -const ( - envPrefixCustomTags = "GRPC_OBSERVABILITY_" - envPrefixLen = len(envPrefixCustomTags) -) - -func getCustomTags(envs []string) map[string]string { - m := make(map[string]string) - for _, e := range envs { - if !strings.HasPrefix(e, envPrefixCustomTags) { - continue - } - tokens := strings.SplitN(e, "=", 2) - if len(tokens) == 2 { - if len(tokens[0]) == envPrefixLen { - // Empty key is not allowed - continue - } - m[tokens[0][envPrefixLen:]] = tokens[1] - } - } - return m -} diff --git a/gcp/observability/tags_test.go b/gcp/observability/tags_test.go deleted file mode 100644 index 5a0353a03087..000000000000 --- a/gcp/observability/tags_test.go +++ /dev/null @@ -1,64 +0,0 @@ -/* - * - * Copyright 2022 gRPC authors. - * - * 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 - * - * http://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. - * - */ - -package observability - -import ( - "reflect" - "testing" -) - -// TestGetCustomTags tests the normal tags parsing -func (s) TestGetCustomTags(t *testing.T) { - var ( - input = []string{ - "GRPC_OBSERVABILITY_APP_NAME=app1", - "GRPC_OBSERVABILITY_DATACENTER=us-west1-a", - "GRPC_OBSERVABILITY_smallcase=OK", - } - expect = map[string]string{ - "APP_NAME": "app1", - "DATACENTER": "us-west1-a", - "smallcase": "OK", - } - ) - result := getCustomTags(input) - if !reflect.DeepEqual(result, expect) { - t.Errorf("result [%+v] != expect [%+v]", result, expect) - } -} - -// TestGetCustomTagsInvalid tests the invalid cases of tags parsing -func (s) TestGetCustomTagsInvalid(t *testing.T) { - var ( - input = []string{ - "GRPC_OBSERVABILITY_APP_NAME=app1", - "GRPC_OBSERVABILITY=foo", - "GRPC_OBSERVABILITY_=foo", // Users should not set "" as key name - "GRPC_STUFF=foo", - "STUFF_GRPC_OBSERVABILITY_=foo", - } - expect = map[string]string{ - "APP_NAME": "app1", - } - ) - result := getCustomTags(input) - if !reflect.DeepEqual(result, expect) { - t.Errorf("result [%+v] != expect [%+v]", result, expect) - } -} From 054cddafa2eb2309671506cd58023b7b72a26d9c Mon Sep 17 00:00:00 2001 From: Zach Reyes Date: Mon, 8 Aug 2022 20:15:22 -0400 Subject: [PATCH 3/4] Switch internal configuration representation from proto to struct --- gcp/observability/config.go | 87 +++- gcp/observability/exporting.go | 7 +- .../internal/config/config.pb.go | 379 ------------------ .../internal/config/config.proto | 92 ----- gcp/observability/logging.go | 15 +- gcp/observability/observability_test.go | 69 ++-- gcp/observability/opencensus.go | 7 +- 7 files changed, 121 insertions(+), 535 deletions(-) delete mode 100644 gcp/observability/internal/config/config.pb.go delete mode 100644 gcp/observability/internal/config/config.proto diff --git a/gcp/observability/config.go b/gcp/observability/config.go index ecda5b230073..923d8fe30d8c 100644 --- a/gcp/observability/config.go +++ b/gcp/observability/config.go @@ -28,8 +28,6 @@ import ( gcplogging "cloud.google.com/go/logging" "golang.org/x/oauth2/google" - configpb "google.golang.org/grpc/gcp/observability/internal/config" - "google.golang.org/protobuf/encoding/protojson" ) const ( @@ -41,6 +39,69 @@ const ( var logFilterPatternRegexp = regexp.MustCompile(logFilterPatternRegexpStr) +// LogFilter represents a method logging configuration. +type LogFilter struct { + // Pattern is a string which can select a group of method names. By + // default, the Pattern is an empty string, matching no methods. + // + // Only "*" Wildcard is accepted for Pattern. A Pattern is in the form + // of / or just a character "*" . + // + // If the Pattern is "*", it specifies the defaults for all the + // services; If the Pattern is /*, it specifies the defaults + // for all methods in the specified service ; If the Pattern is + // */, this is not supported. + // + // Examples: + // - "Foo/Bar" selects only the method "Bar" from service "Foo" + // - "Foo/*" selects all methods from service "Foo" + // - "*" selects all methods from all services. + Pattern string `json:"pattern,omitempty"` + // HeaderBytes is the number of bytes of each header to log. If the size of + // the header is greater than the defined limit, content past the limit will + // be truncated. The default value is 0. + HeaderBytes int32 `json:"header_bytes,omitempty"` + // MessageBytes is the number of bytes of each message to log. If the size + // of the message is greater than the defined limit, content pass the limit + // will be truncated. The default value is 0. + MessageBytes int32 `json:"message_bytes,omitempty"` +} + +// ObvConfig is configuration for observability behaviors. By default, no +// configuration is required for tracing/metrics/logging to function. This +// config captures the most common knobs for gRPC users. It's always possible to +// override with explicit config in code. +type ObvConfig struct { + // EnableCloudTrace represents whether the tracing data upload to + // CloudTrace should be enabled or not. + EnableCloudTrace bool `json:"enable_cloud_trace,omitempty"` + // EnableCloudMonitoring represents whether the metrics data upload to + // CloudMonitoring should be enabled or not. + EnableCloudMonitoring bool `json:"enable_cloud_monitoring,omitempty"` + // EnableCloudLogging represents Whether the logging data upload to + // CloudLogging should be enabled or not. + EnableCloudLogging bool `json:"enable_cloud_logging,omitempty"` + // DestinationProjectID is the destination GCP project identifier for the + // uploading log entries. If empty, the gRPC Observability plugin will + // attempt to fetch the project_id from the GCP environment variables, or + // from the default credentials. + DestinationProjectID string `json:"destination_project_id,omitempty"` + // LogFilters is a list of method config. The order matters here - the first + // Pattern which matches the current method will apply the associated config + // options in the LogFilter. Any other LogFilter that also matches that + // comes later will be ignored. So a LogFilter of "*/*" should appear last + // in this list. + LogFilters []LogFilter `json:"log_filters,omitempty"` + // GlobalTraceSamplingRate is the global setting that controls the + // probability of a RPC being traced. For example, 0.05 means there is a 5% + // chance for a RPC to be traced, 1.0 means trace every call, 0 means don’t + // start new traces. + GlobalTraceSamplingRate float64 `json:"global_trace_sampling_rate,omitempty"` + // CustomTags a list of custom tags that will be attached to every log + // entry. + CustomTags map[string]string `json:"custom_tags,omitempty"` +} + // fetchDefaultProjectID fetches the default GCP project id from environment. func fetchDefaultProjectID(ctx context.Context) string { // Step 1: Check ENV var @@ -62,25 +123,25 @@ func fetchDefaultProjectID(ctx context.Context) string { return credentials.ProjectID } -func validateFilters(config *configpb.ObservabilityConfig) error { - for _, filter := range config.GetLogFilters() { +func validateFilters(config *ObvConfig) error { + for _, filter := range config.LogFilters { if filter.Pattern == "*" { continue } match := logFilterPatternRegexp.FindStringSubmatch(filter.Pattern) if match == nil { - return fmt.Errorf("invalid log filter pattern: %v", filter.Pattern) + return fmt.Errorf("invalid log filter Pattern: %v", filter.Pattern) } } return nil } // unmarshalAndVerifyConfig unmarshals a json string representing an -// observability config into its protobuf format, and also verifies the +// observability config into its internal go format, and also verifies the // configuration's fields for validity. -func unmarshalAndVerifyConfig(rawJSON json.RawMessage) (*configpb.ObservabilityConfig, error) { - var config configpb.ObservabilityConfig - if err := protojson.Unmarshal(rawJSON, &config); err != nil { +func unmarshalAndVerifyConfig(rawJSON json.RawMessage) (*ObvConfig, error) { + var config ObvConfig + if err := json.Unmarshal(rawJSON, &config); err != nil { return nil, fmt.Errorf("error parsing observability config: %v", err) } if err := validateFilters(&config); err != nil { @@ -93,7 +154,7 @@ func unmarshalAndVerifyConfig(rawJSON json.RawMessage) (*configpb.ObservabilityC return &config, nil } -func parseObservabilityConfig() (*configpb.ObservabilityConfig, error) { +func parseObservabilityConfig() (*ObvConfig, error) { if fileSystemPath := os.Getenv(envObservabilityConfigJSON); fileSystemPath != "" { content, err := ioutil.ReadFile(fileSystemPath) // TODO: Switch to os.ReadFile once dropped support for go 1.15 if err != nil { @@ -107,14 +168,14 @@ func parseObservabilityConfig() (*configpb.ObservabilityConfig, error) { return nil, nil } -func ensureProjectIDInObservabilityConfig(ctx context.Context, config *configpb.ObservabilityConfig) error { - if config.GetDestinationProjectId() == "" { +func ensureProjectIDInObservabilityConfig(ctx context.Context, config *ObvConfig) error { + if config.DestinationProjectID == "" { // Try to fetch the GCP project id projectID := fetchDefaultProjectID(ctx) if projectID == "" { return fmt.Errorf("empty destination project ID") } - config.DestinationProjectId = projectID + config.DestinationProjectID = projectID } return nil } diff --git a/gcp/observability/exporting.go b/gcp/observability/exporting.go index ac4d6ea7d065..310a68398e3b 100644 --- a/gcp/observability/exporting.go +++ b/gcp/observability/exporting.go @@ -24,7 +24,6 @@ import ( "fmt" gcplogging "cloud.google.com/go/logging" - configpb "google.golang.org/grpc/gcp/observability/internal/config" grpclogrecordpb "google.golang.org/grpc/gcp/observability/internal/logging" "google.golang.org/protobuf/encoding/protojson" ) @@ -45,8 +44,8 @@ type cloudLoggingExporter struct { logger *gcplogging.Logger } -func newCloudLoggingExporter(ctx context.Context, config *configpb.ObservabilityConfig) (*cloudLoggingExporter, error) { - c, err := gcplogging.NewClient(ctx, fmt.Sprintf("projects/%v", config.DestinationProjectId)) +func newCloudLoggingExporter(ctx context.Context, config *ObvConfig) (*cloudLoggingExporter, error) { + c, err := gcplogging.NewClient(ctx, fmt.Sprintf("projects/%v", config.DestinationProjectID)) if err != nil { return nil, fmt.Errorf("failed to create cloudLoggingExporter: %v", err) } @@ -55,7 +54,7 @@ func newCloudLoggingExporter(ctx context.Context, config *configpb.Observability logger.Infof("Adding custom tags: %+v", config.CustomTags) } return &cloudLoggingExporter{ - projectID: config.DestinationProjectId, + projectID: config.DestinationProjectID, client: c, logger: c.Logger("microservices.googleapis.com/observability/grpc", gcplogging.CommonLabels(config.CustomTags)), }, nil diff --git a/gcp/observability/internal/config/config.pb.go b/gcp/observability/internal/config/config.pb.go deleted file mode 100644 index 1c52da52457d..000000000000 --- a/gcp/observability/internal/config/config.pb.go +++ /dev/null @@ -1,379 +0,0 @@ -// Copyright 2022 The gRPC Authors -// -// 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 -// -// http://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. - -// Observability Config is used by gRPC Observability plugin to control provided -// observability features. It contains parameters to enable/disable certain -// features, or fine tune the verbosity. -// -// Note that gRPC may use this config in JSON form, not in protobuf form. This -// proto definition is intended to help document the schema but might not -// actually be used directly by gRPC. - -// Code generated by protoc-gen-go. DO NOT EDIT. -// versions: -// protoc-gen-go v1.28.0 -// protoc v3.15.3 -// source: internal/config/config.proto - -package config - -import ( - protoreflect "google.golang.org/protobuf/reflect/protoreflect" - protoimpl "google.golang.org/protobuf/runtime/protoimpl" - reflect "reflect" - sync "sync" -) - -const ( - // Verify that this generated code is sufficiently up-to-date. - _ = protoimpl.EnforceVersion(20 - protoimpl.MinVersion) - // Verify that runtime/protoimpl is sufficiently up-to-date. - _ = protoimpl.EnforceVersion(protoimpl.MaxVersion - 20) -) - -// Configuration for observability behaviors. By default, no configuration is -// required for tracing/metrics/logging to function. This config captures the -// most common knobs for gRPC users. It's always possible to override with -// explicit config in code. -type ObservabilityConfig struct { - state protoimpl.MessageState - sizeCache protoimpl.SizeCache - unknownFields protoimpl.UnknownFields - - // Whether the tracing data upload to CloudTrace should be enabled or not. - EnableCloudTrace bool `protobuf:"varint,4,opt,name=enable_cloud_trace,json=enableCloudTrace,proto3" json:"enable_cloud_trace,omitempty"` - // Whether the metrics data upload to CloudMonitoring should be enabled or - // not. - EnableCloudMonitoring bool `protobuf:"varint,5,opt,name=enable_cloud_monitoring,json=enableCloudMonitoring,proto3" json:"enable_cloud_monitoring,omitempty"` - // Whether the logging data upload to CloudLogging should be enabled or not. - EnableCloudLogging bool `protobuf:"varint,1,opt,name=enable_cloud_logging,json=enableCloudLogging,proto3" json:"enable_cloud_logging,omitempty"` - // The destination GCP project identifier for the uploading log entries. If - // empty, the gRPC Observability plugin will attempt to fetch the project_id - // from the GCP environment variables, or from the default credentials. - DestinationProjectId string `protobuf:"bytes,2,opt,name=destination_project_id,json=destinationProjectId,proto3" json:"destination_project_id,omitempty"` - // A list of method config. The order matters here - the first pattern which - // matches the current method will apply the associated config options in - // the LogFilter. Any other LogFilter that also matches that comes later - // will be ignored. So a LogFilter of "*/*" should appear last in this list. - LogFilters []*ObservabilityConfig_LogFilter `protobuf:"bytes,3,rep,name=log_filters,json=logFilters,proto3" json:"log_filters,omitempty"` - // The global setting that controls the probability of a RPC being traced. - // For example, 0.05 means there is a 5% chance for a RPC to be traced, 1.0 - // means trace every call, 0 means don’t start new traces. - GlobalTraceSamplingRate float64 `protobuf:"fixed64,6,opt,name=global_trace_sampling_rate,json=globalTraceSamplingRate,proto3" json:"global_trace_sampling_rate,omitempty"` - // A list of custom tags that will be attached to every log entry. - CustomTags map[string]string `protobuf:"bytes,7,rep,name=custom_tags,json=customTags,proto3" json:"custom_tags,omitempty" protobuf_key:"bytes,1,opt,name=key,proto3" protobuf_val:"bytes,2,opt,name=value,proto3"` -} - -func (x *ObservabilityConfig) Reset() { - *x = ObservabilityConfig{} - if protoimpl.UnsafeEnabled { - mi := &file_internal_config_config_proto_msgTypes[0] - ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) - ms.StoreMessageInfo(mi) - } -} - -func (x *ObservabilityConfig) String() string { - return protoimpl.X.MessageStringOf(x) -} - -func (*ObservabilityConfig) ProtoMessage() {} - -func (x *ObservabilityConfig) ProtoReflect() protoreflect.Message { - mi := &file_internal_config_config_proto_msgTypes[0] - if protoimpl.UnsafeEnabled && x != nil { - ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) - if ms.LoadMessageInfo() == nil { - ms.StoreMessageInfo(mi) - } - return ms - } - return mi.MessageOf(x) -} - -// Deprecated: Use ObservabilityConfig.ProtoReflect.Descriptor instead. -func (*ObservabilityConfig) Descriptor() ([]byte, []int) { - return file_internal_config_config_proto_rawDescGZIP(), []int{0} -} - -func (x *ObservabilityConfig) GetEnableCloudTrace() bool { - if x != nil { - return x.EnableCloudTrace - } - return false -} - -func (x *ObservabilityConfig) GetEnableCloudMonitoring() bool { - if x != nil { - return x.EnableCloudMonitoring - } - return false -} - -func (x *ObservabilityConfig) GetEnableCloudLogging() bool { - if x != nil { - return x.EnableCloudLogging - } - return false -} - -func (x *ObservabilityConfig) GetDestinationProjectId() string { - if x != nil { - return x.DestinationProjectId - } - return "" -} - -func (x *ObservabilityConfig) GetLogFilters() []*ObservabilityConfig_LogFilter { - if x != nil { - return x.LogFilters - } - return nil -} - -func (x *ObservabilityConfig) GetGlobalTraceSamplingRate() float64 { - if x != nil { - return x.GlobalTraceSamplingRate - } - return 0 -} - -func (x *ObservabilityConfig) GetCustomTags() map[string]string { - if x != nil { - return x.CustomTags - } - return nil -} - -type ObservabilityConfig_LogFilter struct { - state protoimpl.MessageState - sizeCache protoimpl.SizeCache - unknownFields protoimpl.UnknownFields - - // Pattern is a string which can select a group of method names. By - // default, the pattern is an empty string, matching no methods. - // - // Only "*" Wildcard is accepted for pattern. A pattern is in the form - // of / or just a character "*" . - // - // If the pattern is "*", it specifies the defaults for all the - // services; If the pattern is /*, it specifies the defaults - // for all methods in the specified service ; If the pattern is - // */, this is not supported. - // - // Examples: - // - "Foo/Bar" selects only the method "Bar" from service "Foo" - // - "Foo/*" selects all methods from service "Foo" - // - "*" selects all methods from all services. - Pattern string `protobuf:"bytes,1,opt,name=pattern,proto3" json:"pattern,omitempty"` - // Number of bytes of each header to log. If the size of the header is - // greater than the defined limit, content pass the limit will be - // truncated. The default value is 0. - HeaderBytes int32 `protobuf:"varint,2,opt,name=header_bytes,json=headerBytes,proto3" json:"header_bytes,omitempty"` - // Number of bytes of each message to log. If the size of the message is - // greater than the defined limit, content pass the limit will be - // truncated. The default value is 0. - MessageBytes int32 `protobuf:"varint,3,opt,name=message_bytes,json=messageBytes,proto3" json:"message_bytes,omitempty"` -} - -func (x *ObservabilityConfig_LogFilter) Reset() { - *x = ObservabilityConfig_LogFilter{} - if protoimpl.UnsafeEnabled { - mi := &file_internal_config_config_proto_msgTypes[1] - ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) - ms.StoreMessageInfo(mi) - } -} - -func (x *ObservabilityConfig_LogFilter) String() string { - return protoimpl.X.MessageStringOf(x) -} - -func (*ObservabilityConfig_LogFilter) ProtoMessage() {} - -func (x *ObservabilityConfig_LogFilter) ProtoReflect() protoreflect.Message { - mi := &file_internal_config_config_proto_msgTypes[1] - if protoimpl.UnsafeEnabled && x != nil { - ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) - if ms.LoadMessageInfo() == nil { - ms.StoreMessageInfo(mi) - } - return ms - } - return mi.MessageOf(x) -} - -// Deprecated: Use ObservabilityConfig_LogFilter.ProtoReflect.Descriptor instead. -func (*ObservabilityConfig_LogFilter) Descriptor() ([]byte, []int) { - return file_internal_config_config_proto_rawDescGZIP(), []int{0, 0} -} - -func (x *ObservabilityConfig_LogFilter) GetPattern() string { - if x != nil { - return x.Pattern - } - return "" -} - -func (x *ObservabilityConfig_LogFilter) GetHeaderBytes() int32 { - if x != nil { - return x.HeaderBytes - } - return 0 -} - -func (x *ObservabilityConfig_LogFilter) GetMessageBytes() int32 { - if x != nil { - return x.MessageBytes - } - return 0 -} - -var File_internal_config_config_proto protoreflect.FileDescriptor - -var file_internal_config_config_proto_rawDesc = []byte{ - 0x0a, 0x1c, 0x69, 0x6e, 0x74, 0x65, 0x72, 0x6e, 0x61, 0x6c, 0x2f, 0x63, 0x6f, 0x6e, 0x66, 0x69, - 0x67, 0x2f, 0x63, 0x6f, 0x6e, 0x66, 0x69, 0x67, 0x2e, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x12, 0x21, - 0x67, 0x72, 0x70, 0x63, 0x2e, 0x6f, 0x62, 0x73, 0x65, 0x72, 0x76, 0x61, 0x62, 0x69, 0x6c, 0x69, - 0x74, 0x79, 0x2e, 0x63, 0x6f, 0x6e, 0x66, 0x69, 0x67, 0x2e, 0x76, 0x31, 0x61, 0x6c, 0x70, 0x68, - 0x61, 0x22, 0x9a, 0x05, 0x0a, 0x13, 0x4f, 0x62, 0x73, 0x65, 0x72, 0x76, 0x61, 0x62, 0x69, 0x6c, - 0x69, 0x74, 0x79, 0x43, 0x6f, 0x6e, 0x66, 0x69, 0x67, 0x12, 0x2c, 0x0a, 0x12, 0x65, 0x6e, 0x61, - 0x62, 0x6c, 0x65, 0x5f, 0x63, 0x6c, 0x6f, 0x75, 0x64, 0x5f, 0x74, 0x72, 0x61, 0x63, 0x65, 0x18, - 0x04, 0x20, 0x01, 0x28, 0x08, 0x52, 0x10, 0x65, 0x6e, 0x61, 0x62, 0x6c, 0x65, 0x43, 0x6c, 0x6f, - 0x75, 0x64, 0x54, 0x72, 0x61, 0x63, 0x65, 0x12, 0x36, 0x0a, 0x17, 0x65, 0x6e, 0x61, 0x62, 0x6c, - 0x65, 0x5f, 0x63, 0x6c, 0x6f, 0x75, 0x64, 0x5f, 0x6d, 0x6f, 0x6e, 0x69, 0x74, 0x6f, 0x72, 0x69, - 0x6e, 0x67, 0x18, 0x05, 0x20, 0x01, 0x28, 0x08, 0x52, 0x15, 0x65, 0x6e, 0x61, 0x62, 0x6c, 0x65, - 0x43, 0x6c, 0x6f, 0x75, 0x64, 0x4d, 0x6f, 0x6e, 0x69, 0x74, 0x6f, 0x72, 0x69, 0x6e, 0x67, 0x12, - 0x30, 0x0a, 0x14, 0x65, 0x6e, 0x61, 0x62, 0x6c, 0x65, 0x5f, 0x63, 0x6c, 0x6f, 0x75, 0x64, 0x5f, - 0x6c, 0x6f, 0x67, 0x67, 0x69, 0x6e, 0x67, 0x18, 0x01, 0x20, 0x01, 0x28, 0x08, 0x52, 0x12, 0x65, - 0x6e, 0x61, 0x62, 0x6c, 0x65, 0x43, 0x6c, 0x6f, 0x75, 0x64, 0x4c, 0x6f, 0x67, 0x67, 0x69, 0x6e, - 0x67, 0x12, 0x34, 0x0a, 0x16, 0x64, 0x65, 0x73, 0x74, 0x69, 0x6e, 0x61, 0x74, 0x69, 0x6f, 0x6e, - 0x5f, 0x70, 0x72, 0x6f, 0x6a, 0x65, 0x63, 0x74, 0x5f, 0x69, 0x64, 0x18, 0x02, 0x20, 0x01, 0x28, - 0x09, 0x52, 0x14, 0x64, 0x65, 0x73, 0x74, 0x69, 0x6e, 0x61, 0x74, 0x69, 0x6f, 0x6e, 0x50, 0x72, - 0x6f, 0x6a, 0x65, 0x63, 0x74, 0x49, 0x64, 0x12, 0x61, 0x0a, 0x0b, 0x6c, 0x6f, 0x67, 0x5f, 0x66, - 0x69, 0x6c, 0x74, 0x65, 0x72, 0x73, 0x18, 0x03, 0x20, 0x03, 0x28, 0x0b, 0x32, 0x40, 0x2e, 0x67, - 0x72, 0x70, 0x63, 0x2e, 0x6f, 0x62, 0x73, 0x65, 0x72, 0x76, 0x61, 0x62, 0x69, 0x6c, 0x69, 0x74, - 0x79, 0x2e, 0x63, 0x6f, 0x6e, 0x66, 0x69, 0x67, 0x2e, 0x76, 0x31, 0x61, 0x6c, 0x70, 0x68, 0x61, - 0x2e, 0x4f, 0x62, 0x73, 0x65, 0x72, 0x76, 0x61, 0x62, 0x69, 0x6c, 0x69, 0x74, 0x79, 0x43, 0x6f, - 0x6e, 0x66, 0x69, 0x67, 0x2e, 0x4c, 0x6f, 0x67, 0x46, 0x69, 0x6c, 0x74, 0x65, 0x72, 0x52, 0x0a, - 0x6c, 0x6f, 0x67, 0x46, 0x69, 0x6c, 0x74, 0x65, 0x72, 0x73, 0x12, 0x3b, 0x0a, 0x1a, 0x67, 0x6c, - 0x6f, 0x62, 0x61, 0x6c, 0x5f, 0x74, 0x72, 0x61, 0x63, 0x65, 0x5f, 0x73, 0x61, 0x6d, 0x70, 0x6c, - 0x69, 0x6e, 0x67, 0x5f, 0x72, 0x61, 0x74, 0x65, 0x18, 0x06, 0x20, 0x01, 0x28, 0x01, 0x52, 0x17, - 0x67, 0x6c, 0x6f, 0x62, 0x61, 0x6c, 0x54, 0x72, 0x61, 0x63, 0x65, 0x53, 0x61, 0x6d, 0x70, 0x6c, - 0x69, 0x6e, 0x67, 0x52, 0x61, 0x74, 0x65, 0x12, 0x67, 0x0a, 0x0b, 0x63, 0x75, 0x73, 0x74, 0x6f, - 0x6d, 0x5f, 0x74, 0x61, 0x67, 0x73, 0x18, 0x07, 0x20, 0x03, 0x28, 0x0b, 0x32, 0x46, 0x2e, 0x67, - 0x72, 0x70, 0x63, 0x2e, 0x6f, 0x62, 0x73, 0x65, 0x72, 0x76, 0x61, 0x62, 0x69, 0x6c, 0x69, 0x74, - 0x79, 0x2e, 0x63, 0x6f, 0x6e, 0x66, 0x69, 0x67, 0x2e, 0x76, 0x31, 0x61, 0x6c, 0x70, 0x68, 0x61, - 0x2e, 0x4f, 0x62, 0x73, 0x65, 0x72, 0x76, 0x61, 0x62, 0x69, 0x6c, 0x69, 0x74, 0x79, 0x43, 0x6f, - 0x6e, 0x66, 0x69, 0x67, 0x2e, 0x43, 0x75, 0x73, 0x74, 0x6f, 0x6d, 0x54, 0x61, 0x67, 0x73, 0x45, - 0x6e, 0x74, 0x72, 0x79, 0x52, 0x0a, 0x63, 0x75, 0x73, 0x74, 0x6f, 0x6d, 0x54, 0x61, 0x67, 0x73, - 0x1a, 0x6d, 0x0a, 0x09, 0x4c, 0x6f, 0x67, 0x46, 0x69, 0x6c, 0x74, 0x65, 0x72, 0x12, 0x18, 0x0a, - 0x07, 0x70, 0x61, 0x74, 0x74, 0x65, 0x72, 0x6e, 0x18, 0x01, 0x20, 0x01, 0x28, 0x09, 0x52, 0x07, - 0x70, 0x61, 0x74, 0x74, 0x65, 0x72, 0x6e, 0x12, 0x21, 0x0a, 0x0c, 0x68, 0x65, 0x61, 0x64, 0x65, - 0x72, 0x5f, 0x62, 0x79, 0x74, 0x65, 0x73, 0x18, 0x02, 0x20, 0x01, 0x28, 0x05, 0x52, 0x0b, 0x68, - 0x65, 0x61, 0x64, 0x65, 0x72, 0x42, 0x79, 0x74, 0x65, 0x73, 0x12, 0x23, 0x0a, 0x0d, 0x6d, 0x65, - 0x73, 0x73, 0x61, 0x67, 0x65, 0x5f, 0x62, 0x79, 0x74, 0x65, 0x73, 0x18, 0x03, 0x20, 0x01, 0x28, - 0x05, 0x52, 0x0c, 0x6d, 0x65, 0x73, 0x73, 0x61, 0x67, 0x65, 0x42, 0x79, 0x74, 0x65, 0x73, 0x1a, - 0x3d, 0x0a, 0x0f, 0x43, 0x75, 0x73, 0x74, 0x6f, 0x6d, 0x54, 0x61, 0x67, 0x73, 0x45, 0x6e, 0x74, - 0x72, 0x79, 0x12, 0x10, 0x0a, 0x03, 0x6b, 0x65, 0x79, 0x18, 0x01, 0x20, 0x01, 0x28, 0x09, 0x52, - 0x03, 0x6b, 0x65, 0x79, 0x12, 0x14, 0x0a, 0x05, 0x76, 0x61, 0x6c, 0x75, 0x65, 0x18, 0x02, 0x20, - 0x01, 0x28, 0x09, 0x52, 0x05, 0x76, 0x61, 0x6c, 0x75, 0x65, 0x3a, 0x02, 0x38, 0x01, 0x42, 0x74, - 0x0a, 0x1c, 0x69, 0x6f, 0x2e, 0x67, 0x72, 0x70, 0x63, 0x2e, 0x6f, 0x62, 0x73, 0x65, 0x72, 0x76, - 0x61, 0x62, 0x69, 0x6c, 0x69, 0x74, 0x79, 0x2e, 0x63, 0x6f, 0x6e, 0x66, 0x69, 0x67, 0x42, 0x18, - 0x4f, 0x62, 0x73, 0x65, 0x72, 0x76, 0x61, 0x62, 0x69, 0x6c, 0x69, 0x74, 0x79, 0x43, 0x6f, 0x6e, - 0x66, 0x69, 0x67, 0x50, 0x72, 0x6f, 0x74, 0x6f, 0x50, 0x01, 0x5a, 0x38, 0x67, 0x6f, 0x6f, 0x67, - 0x6c, 0x65, 0x2e, 0x67, 0x6f, 0x6c, 0x61, 0x6e, 0x67, 0x2e, 0x6f, 0x72, 0x67, 0x2f, 0x67, 0x72, - 0x70, 0x63, 0x2f, 0x67, 0x63, 0x70, 0x2f, 0x6f, 0x62, 0x73, 0x65, 0x72, 0x76, 0x61, 0x62, 0x69, - 0x6c, 0x69, 0x74, 0x79, 0x2f, 0x69, 0x6e, 0x74, 0x65, 0x72, 0x6e, 0x61, 0x6c, 0x2f, 0x63, 0x6f, - 0x6e, 0x66, 0x69, 0x67, 0x62, 0x06, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x33, -} - -var ( - file_internal_config_config_proto_rawDescOnce sync.Once - file_internal_config_config_proto_rawDescData = file_internal_config_config_proto_rawDesc -) - -func file_internal_config_config_proto_rawDescGZIP() []byte { - file_internal_config_config_proto_rawDescOnce.Do(func() { - file_internal_config_config_proto_rawDescData = protoimpl.X.CompressGZIP(file_internal_config_config_proto_rawDescData) - }) - return file_internal_config_config_proto_rawDescData -} - -var file_internal_config_config_proto_msgTypes = make([]protoimpl.MessageInfo, 3) -var file_internal_config_config_proto_goTypes = []interface{}{ - (*ObservabilityConfig)(nil), // 0: grpc.observability.config.v1alpha.ObservabilityConfig - (*ObservabilityConfig_LogFilter)(nil), // 1: grpc.observability.config.v1alpha.ObservabilityConfig.LogFilter - nil, // 2: grpc.observability.config.v1alpha.ObservabilityConfig.CustomTagsEntry -} -var file_internal_config_config_proto_depIdxs = []int32{ - 1, // 0: grpc.observability.config.v1alpha.ObservabilityConfig.log_filters:type_name -> grpc.observability.config.v1alpha.ObservabilityConfig.LogFilter - 2, // 1: grpc.observability.config.v1alpha.ObservabilityConfig.custom_tags:type_name -> grpc.observability.config.v1alpha.ObservabilityConfig.CustomTagsEntry - 2, // [2:2] is the sub-list for method output_type - 2, // [2:2] is the sub-list for method input_type - 2, // [2:2] is the sub-list for extension type_name - 2, // [2:2] is the sub-list for extension extendee - 0, // [0:2] is the sub-list for field type_name -} - -func init() { file_internal_config_config_proto_init() } -func file_internal_config_config_proto_init() { - if File_internal_config_config_proto != nil { - return - } - if !protoimpl.UnsafeEnabled { - file_internal_config_config_proto_msgTypes[0].Exporter = func(v interface{}, i int) interface{} { - switch v := v.(*ObservabilityConfig); i { - case 0: - return &v.state - case 1: - return &v.sizeCache - case 2: - return &v.unknownFields - default: - return nil - } - } - file_internal_config_config_proto_msgTypes[1].Exporter = func(v interface{}, i int) interface{} { - switch v := v.(*ObservabilityConfig_LogFilter); i { - case 0: - return &v.state - case 1: - return &v.sizeCache - case 2: - return &v.unknownFields - default: - return nil - } - } - } - type x struct{} - out := protoimpl.TypeBuilder{ - File: protoimpl.DescBuilder{ - GoPackagePath: reflect.TypeOf(x{}).PkgPath(), - RawDescriptor: file_internal_config_config_proto_rawDesc, - NumEnums: 0, - NumMessages: 3, - NumExtensions: 0, - NumServices: 0, - }, - GoTypes: file_internal_config_config_proto_goTypes, - DependencyIndexes: file_internal_config_config_proto_depIdxs, - MessageInfos: file_internal_config_config_proto_msgTypes, - }.Build() - File_internal_config_config_proto = out.File - file_internal_config_config_proto_rawDesc = nil - file_internal_config_config_proto_goTypes = nil - file_internal_config_config_proto_depIdxs = nil -} diff --git a/gcp/observability/internal/config/config.proto b/gcp/observability/internal/config/config.proto deleted file mode 100644 index 685bf2d84eaf..000000000000 --- a/gcp/observability/internal/config/config.proto +++ /dev/null @@ -1,92 +0,0 @@ -// Copyright 2022 The gRPC Authors -// -// 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 -// -// http://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. - -// Observability Config is used by gRPC Observability plugin to control provided -// observability features. It contains parameters to enable/disable certain -// features, or fine tune the verbosity. -// -// Note that gRPC may use this config in JSON form, not in protobuf form. This -// proto definition is intended to help document the schema but might not -// actually be used directly by gRPC. - -syntax = "proto3"; - -package grpc.observability.config.v1alpha; - -option java_package = "io.grpc.observability.config"; -option java_multiple_files = true; -option java_outer_classname = "ObservabilityConfigProto"; -option go_package = "google.golang.org/grpc/gcp/observability/internal/config"; - -// Configuration for observability behaviors. By default, no configuration is -// required for tracing/metrics/logging to function. This config captures the -// most common knobs for gRPC users. It's always possible to override with -// explicit config in code. -message ObservabilityConfig { - // Whether the tracing data upload to CloudTrace should be enabled or not. - bool enable_cloud_trace = 4; - - // Whether the metrics data upload to CloudMonitoring should be enabled or - // not. - bool enable_cloud_monitoring = 5; - - // Whether the logging data upload to CloudLogging should be enabled or not. - bool enable_cloud_logging = 1; - - // The destination GCP project identifier for the uploading log entries. If - // empty, the gRPC Observability plugin will attempt to fetch the project_id - // from the GCP environment variables, or from the default credentials. - string destination_project_id = 2; - - message LogFilter { - // Pattern is a string which can select a group of method names. By - // default, the pattern is an empty string, matching no methods. - // - // Only "*" Wildcard is accepted for pattern. A pattern is in the form - // of / or just a character "*" . - // - // If the pattern is "*", it specifies the defaults for all the - // services; If the pattern is /*, it specifies the defaults - // for all methods in the specified service ; If the pattern is - // */, this is not supported. - // - // Examples: - // - "Foo/Bar" selects only the method "Bar" from service "Foo" - // - "Foo/*" selects all methods from service "Foo" - // - "*" selects all methods from all services. - string pattern = 1; - // Number of bytes of each header to log. If the size of the header is - // greater than the defined limit, content pass the limit will be - // truncated. The default value is 0. - int32 header_bytes = 2; - // Number of bytes of each message to log. If the size of the message is - // greater than the defined limit, content pass the limit will be - // truncated. The default value is 0. - int32 message_bytes = 3; - } - - // A list of method config. The order matters here - the first pattern which - // matches the current method will apply the associated config options in - // the LogFilter. Any other LogFilter that also matches that comes later - // will be ignored. So a LogFilter of "*/*" should appear last in this list. - repeated LogFilter log_filters = 3; - - // The global setting that controls the probability of a RPC being traced. - // For example, 0.05 means there is a 5% chance for a RPC to be traced, 1.0 - // means trace every call, 0 means don’t start new traces. - double global_trace_sampling_rate = 6; - - // A list of custom tags that will be attached to every log entry. - map custom_tags = 7; -} diff --git a/gcp/observability/logging.go b/gcp/observability/logging.go index 711dc74f8672..8bab110de68d 100644 --- a/gcp/observability/logging.go +++ b/gcp/observability/logging.go @@ -27,7 +27,6 @@ import ( "github.com/google/uuid" binlogpb "google.golang.org/grpc/binarylog/grpc_binarylog_v1" - configpb "google.golang.org/grpc/gcp/observability/internal/config" grpclogrecordpb "google.golang.org/grpc/gcp/observability/internal/logging" iblog "google.golang.org/grpc/internal/binarylog" ) @@ -248,7 +247,7 @@ func (l *binaryLogger) Close() { } } -func validateExistingMethodLoggerConfig(existing *iblog.MethodLoggerConfig, filter *configpb.ObservabilityConfig_LogFilter) bool { +func validateExistingMethodLoggerConfig(existing *iblog.MethodLoggerConfig, filter LogFilter) bool { // In future, we could add more validations. Currently, we only check if the // new filter configs are different than the existing one, if so, we log a // warning. @@ -258,7 +257,7 @@ func validateExistingMethodLoggerConfig(existing *iblog.MethodLoggerConfig, filt return existing == nil } -func createBinaryLoggerConfig(filters []*configpb.ObservabilityConfig_LogFilter) iblog.LoggerConfig { +func createBinaryLoggerConfig(filters []LogFilter) iblog.LoggerConfig { config := iblog.LoggerConfig{ Services: make(map[string]*iblog.MethodLoggerConfig), Methods: make(map[string]*iblog.MethodLoggerConfig), @@ -296,8 +295,8 @@ func createBinaryLoggerConfig(filters []*configpb.ObservabilityConfig_LogFilter) // start is the core logic for setting up the custom binary logging logger, and // it's also useful for testing. -func (l *binaryLogger) start(config *configpb.ObservabilityConfig, exporter loggingExporter) error { - filters := config.GetLogFilters() +func (l *binaryLogger) start(config *ObvConfig, exporter loggingExporter) error { + filters := config.LogFilters if len(filters) == 0 || exporter == nil { // Doing nothing is allowed if exporter != nil { @@ -318,11 +317,11 @@ func (l *binaryLogger) start(config *configpb.ObservabilityConfig, exporter logg return nil } -func (l *binaryLogger) Start(ctx context.Context, config *configpb.ObservabilityConfig) error { - if config == nil || !config.GetEnableCloudLogging() { +func (l *binaryLogger) Start(ctx context.Context, config *ObvConfig) error { + if config == nil || !config.EnableCloudLogging { return nil } - if config.GetDestinationProjectId() == "" { + if config.DestinationProjectID == "" { return fmt.Errorf("failed to enable CloudLogging: empty destination_project_id") } exporter, err := newCloudLoggingExporter(ctx, config) diff --git a/gcp/observability/observability_test.go b/gcp/observability/observability_test.go index 962f56f1d120..97c1cd3d2d85 100644 --- a/gcp/observability/observability_test.go +++ b/gcp/observability/observability_test.go @@ -34,7 +34,6 @@ import ( "go.opencensus.io/trace" "google.golang.org/grpc" "google.golang.org/grpc/credentials/insecure" - configpb "google.golang.org/grpc/gcp/observability/internal/config" grpclogrecordpb "google.golang.org/grpc/gcp/observability/internal/logging" "google.golang.org/grpc/internal" iblog "google.golang.org/grpc/internal/binarylog" @@ -197,7 +196,7 @@ func (te *test) clientConn() *grpc.ClientConn { return te.cc } -func (te *test) enablePluginWithConfig(config *configpb.ObservabilityConfig) { +func (te *test) enablePluginWithConfig(config *ObvConfig) { // Injects the fake exporter for testing purposes te.fle = &fakeLoggingExporter{t: te.t} defaultLogger = newBinaryLogger(nil) @@ -208,10 +207,10 @@ func (te *test) enablePluginWithConfig(config *configpb.ObservabilityConfig) { } func (te *test) enablePluginWithCaptureAll() { - te.enablePluginWithConfig(&configpb.ObservabilityConfig{ + te.enablePluginWithConfig(&ObvConfig{ EnableCloudLogging: true, - DestinationProjectId: "fake", - LogFilters: []*configpb.ObservabilityConfig_LogFilter{ + DestinationProjectID: "fake", + LogFilters: []LogFilter{ { Pattern: "*", HeaderBytes: infinitySizeBytes, @@ -223,7 +222,7 @@ func (te *test) enablePluginWithCaptureAll() { func (te *test) enableOpenCensus() { defaultMetricsReportingInterval = time.Millisecond * 100 - config := &configpb.ObservabilityConfig{ + config := &ObvConfig{ EnableCloudLogging: true, EnableCloudTrace: true, EnableCloudMonitoring: true, @@ -521,7 +520,7 @@ func (s) TestLoggingForErrorCall(t *testing.T) { func (s) TestEmptyConfig(t *testing.T) { te := newTest(t) defer te.tearDown() - te.enablePluginWithConfig(&configpb.ObservabilityConfig{}) + te.enablePluginWithConfig(&ObvConfig{}) te.startServer(&testServer{}) tc := testgrpc.NewTestServiceClient(te.clientConn()) @@ -553,10 +552,10 @@ func (s) TestOverrideConfig(t *testing.T) { // most specific one. The third filter allows message payload logging, and // others disabling the message payload logging. We should observe this // behavior latter. - te.enablePluginWithConfig(&configpb.ObservabilityConfig{ + te.enablePluginWithConfig(&ObvConfig{ EnableCloudLogging: true, - DestinationProjectId: "fake", - LogFilters: []*configpb.ObservabilityConfig_LogFilter{ + DestinationProjectID: "fake", + LogFilters: []LogFilter{ { Pattern: "wont/match", MessageBytes: 0, @@ -620,10 +619,10 @@ func (s) TestNoMatch(t *testing.T) { // Setting 3 filters, expected to use the second filter. The second filter // allows message payload logging, and others disabling the message payload // logging. We should observe this behavior latter. - te.enablePluginWithConfig(&configpb.ObservabilityConfig{ + te.enablePluginWithConfig(&ObvConfig{ EnableCloudLogging: true, - DestinationProjectId: "fake", - LogFilters: []*configpb.ObservabilityConfig_LogFilter{ + DestinationProjectID: "fake", + LogFilters: []LogFilter{ { Pattern: "wont/match", MessageBytes: 0, @@ -660,10 +659,10 @@ func (s) TestNoMatch(t *testing.T) { } func (s) TestRefuseStartWithInvalidPatterns(t *testing.T) { - config := &configpb.ObservabilityConfig{ + config := &ObvConfig{ EnableCloudLogging: true, - DestinationProjectId: "fake", - LogFilters: []*configpb.ObservabilityConfig_LogFilter{ + DestinationProjectID: "fake", + LogFilters: []LogFilter{ { Pattern: ":-)", }, @@ -672,7 +671,7 @@ func (s) TestRefuseStartWithInvalidPatterns(t *testing.T) { }, }, } - configJSON, err := protojson.Marshal(config) + configJSON, err := json.Marshal(config) if err != nil { t.Fatalf("failed to convert config to JSON: %v", err) } @@ -709,8 +708,8 @@ func createTmpConfigInFileSystem(rawJSON string) (func(), error) { // file path pointing to a JSON encoded config. func (s) TestJSONEnvVarSet(t *testing.T) { configJSON := `{ - "destinationProjectId": "fake", - "logFilters":[{"pattern":"*","headerBytes":1073741824,"messageBytes":1073741824}] + "destination_project_id": "fake", + "log_filters":[{"pattern":"*","header_bytes":1073741824,"message_bytes":1073741824}] }` cleanup, err := createTmpConfigInFileSystem(configJSON) defer cleanup() @@ -733,8 +732,8 @@ func (s) TestJSONEnvVarSet(t *testing.T) { // variable is set and valid. func (s) TestBothConfigEnvVarsSet(t *testing.T) { configJSON := `{ - "destinationProjectId":"fake", - "logFilters":[{"pattern":":-)"}, {"pattern":"*"}] + "destination_project_id":"fake", + "log_filters":[{"pattern":":-)"}, {"pattern_string":"*"}] }` cleanup, err := createTmpConfigInFileSystem(configJSON) defer cleanup() @@ -742,10 +741,10 @@ func (s) TestBothConfigEnvVarsSet(t *testing.T) { t.Fatalf("failed to create config in file system: %v", err) } // This configuration should be ignored, as precedence 2. - validConfig := &configpb.ObservabilityConfig{ + validConfig := &ObvConfig{ EnableCloudLogging: true, - DestinationProjectId: "fake", - LogFilters: []*configpb.ObservabilityConfig_LogFilter{ + DestinationProjectID: "fake", + LogFilters: []LogFilter{ { Pattern: "*", HeaderBytes: infinitySizeBytes, @@ -753,7 +752,7 @@ func (s) TestBothConfigEnvVarsSet(t *testing.T) { }, }, } - validConfigJSON, err := protojson.Marshal(validConfig) + validConfigJSON, err := json.Marshal(validConfig) if err != nil { t.Fatalf("failed to convert config to JSON: %v", err) } @@ -789,11 +788,11 @@ func (s) TestOpenCensusIntegration(t *testing.T) { defer te.tearDown() fe := &fakeOpenCensusExporter{SeenViews: make(map[string]string), t: te.t} - defer func(ne func(config *configpb.ObservabilityConfig) (tracingMetricsExporter, error)) { + defer func(ne func(config *ObvConfig) (tracingMetricsExporter, error)) { newExporter = ne }(newExporter) - newExporter = func(config *configpb.ObservabilityConfig) (tracingMetricsExporter, error) { + newExporter = func(config *ObvConfig) (tracingMetricsExporter, error) { return fe, nil } @@ -846,12 +845,12 @@ func (s) TestOpenCensusIntegration(t *testing.T) { // observability configuration and set to two hardcoded values are passed to the // function to create an exporter. func (s) TestCustomTagsTracingMetrics(t *testing.T) { - defer func(ne func(config *configpb.ObservabilityConfig) (tracingMetricsExporter, error)) { + defer func(ne func(config *ObvConfig) (tracingMetricsExporter, error)) { newExporter = ne }(newExporter) fe := &fakeOpenCensusExporter{SeenViews: make(map[string]string), t: t} - newExporter = func(config *configpb.ObservabilityConfig) (tracingMetricsExporter, error) { - ct := config.GetCustomTags() + newExporter = func(config *ObvConfig) (tracingMetricsExporter, error) { + ct := config.CustomTags if len(ct) < 1 { t.Fatalf("less than 2 custom tags sent in") } @@ -867,11 +866,11 @@ func (s) TestCustomTagsTracingMetrics(t *testing.T) { // This configuration present in file system and it's defined custom tags should make it // to the created exporter. configJSON := `{ - "destinationProjectId": "fake", - "enableCloudTrace": true, - "enableCloudMonitoring": true, - "globalTraceSamplingRate": 1.0, - "customTags":{"customtag1":"wow","customtag2":"nice"} + "destination_project_id": "fake", + "enable_cloud_trace": true, + "enable_cloud_monitoring": true, + "global_trace_sampling_rate": 1.0, + "custom_tags":{"customtag1":"wow","customtag2":"nice"} }` cleanup, err := createTmpConfigInFileSystem(configJSON) defer cleanup() diff --git a/gcp/observability/opencensus.go b/gcp/observability/opencensus.go index df0eba95dfd9..6880799daf19 100644 --- a/gcp/observability/opencensus.go +++ b/gcp/observability/opencensus.go @@ -29,7 +29,6 @@ import ( "go.opencensus.io/stats/view" "go.opencensus.io/trace" "google.golang.org/grpc" - configpb "google.golang.org/grpc/gcp/observability/internal/config" "google.golang.org/grpc/internal" ) @@ -62,13 +61,13 @@ type tracingMetricsExporter interface { // global to stub out in tests var newExporter = newStackdriverExporter -func newStackdriverExporter(config *configpb.ObservabilityConfig) (tracingMetricsExporter, error) { +func newStackdriverExporter(config *ObvConfig) (tracingMetricsExporter, error) { // Create the Stackdriver exporter, which is shared between tracing and stats mr := monitoredresource.Autodetect() logger.Infof("Detected MonitoredResource:: %+v", mr) var err error exporter, err := stackdriver.NewExporter(stackdriver.Options{ - ProjectID: config.DestinationProjectId, + ProjectID: config.DestinationProjectID, MonitoredResource: mr, DefaultMonitoringLabels: convertTagsToMonitoringLabels(config.CustomTags), DefaultTraceAttributes: convertTagsToTraceAttributes(config.CustomTags), @@ -81,7 +80,7 @@ func newStackdriverExporter(config *configpb.ObservabilityConfig) (tracingMetric // This method accepts config and exporter; the exporter argument is exposed to // assist unit testing of the OpenCensus behavior. -func startOpenCensus(config *configpb.ObservabilityConfig) error { +func startOpenCensus(config *ObvConfig) error { // If both tracing and metrics are disabled, there's no point inject default // StatsHandler. if config == nil || (!config.EnableCloudTrace && !config.EnableCloudMonitoring) { From b44b856f8d5a09ff578f4f71cd7acb12d2f1d6fc Mon Sep 17 00:00:00 2001 From: Zach Reyes Date: Thu, 11 Aug 2022 13:31:54 -0400 Subject: [PATCH 4/4] Responded to Doug's comments --- gcp/observability/config.go | 24 ++++++++--------- gcp/observability/exporting.go | 2 +- gcp/observability/logging.go | 8 +++--- gcp/observability/observability_test.go | 34 ++++++++++++------------- gcp/observability/opencensus.go | 12 ++++----- 5 files changed, 40 insertions(+), 40 deletions(-) diff --git a/gcp/observability/config.go b/gcp/observability/config.go index 923d8fe30d8c..1a9f718e81b1 100644 --- a/gcp/observability/config.go +++ b/gcp/observability/config.go @@ -39,8 +39,8 @@ const ( var logFilterPatternRegexp = regexp.MustCompile(logFilterPatternRegexpStr) -// LogFilter represents a method logging configuration. -type LogFilter struct { +// logFilter represents a method logging configuration. +type logFilter struct { // Pattern is a string which can select a group of method names. By // default, the Pattern is an empty string, matching no methods. // @@ -67,11 +67,11 @@ type LogFilter struct { MessageBytes int32 `json:"message_bytes,omitempty"` } -// ObvConfig is configuration for observability behaviors. By default, no +// config is configuration for observability behaviors. By default, no // configuration is required for tracing/metrics/logging to function. This // config captures the most common knobs for gRPC users. It's always possible to // override with explicit config in code. -type ObvConfig struct { +type config struct { // EnableCloudTrace represents whether the tracing data upload to // CloudTrace should be enabled or not. EnableCloudTrace bool `json:"enable_cloud_trace,omitempty"` @@ -88,10 +88,10 @@ type ObvConfig struct { DestinationProjectID string `json:"destination_project_id,omitempty"` // LogFilters is a list of method config. The order matters here - the first // Pattern which matches the current method will apply the associated config - // options in the LogFilter. Any other LogFilter that also matches that - // comes later will be ignored. So a LogFilter of "*/*" should appear last + // options in the logFilter. Any other logFilter that also matches that + // comes later will be ignored. So a logFilter of "*/*" should appear last // in this list. - LogFilters []LogFilter `json:"log_filters,omitempty"` + LogFilters []logFilter `json:"log_filters,omitempty"` // GlobalTraceSamplingRate is the global setting that controls the // probability of a RPC being traced. For example, 0.05 means there is a 5% // chance for a RPC to be traced, 1.0 means trace every call, 0 means don’t @@ -123,7 +123,7 @@ func fetchDefaultProjectID(ctx context.Context) string { return credentials.ProjectID } -func validateFilters(config *ObvConfig) error { +func validateFilters(config *config) error { for _, filter := range config.LogFilters { if filter.Pattern == "*" { continue @@ -139,8 +139,8 @@ func validateFilters(config *ObvConfig) error { // unmarshalAndVerifyConfig unmarshals a json string representing an // observability config into its internal go format, and also verifies the // configuration's fields for validity. -func unmarshalAndVerifyConfig(rawJSON json.RawMessage) (*ObvConfig, error) { - var config ObvConfig +func unmarshalAndVerifyConfig(rawJSON json.RawMessage) (*config, error) { + var config config if err := json.Unmarshal(rawJSON, &config); err != nil { return nil, fmt.Errorf("error parsing observability config: %v", err) } @@ -154,7 +154,7 @@ func unmarshalAndVerifyConfig(rawJSON json.RawMessage) (*ObvConfig, error) { return &config, nil } -func parseObservabilityConfig() (*ObvConfig, error) { +func parseObservabilityConfig() (*config, error) { if fileSystemPath := os.Getenv(envObservabilityConfigJSON); fileSystemPath != "" { content, err := ioutil.ReadFile(fileSystemPath) // TODO: Switch to os.ReadFile once dropped support for go 1.15 if err != nil { @@ -168,7 +168,7 @@ func parseObservabilityConfig() (*ObvConfig, error) { return nil, nil } -func ensureProjectIDInObservabilityConfig(ctx context.Context, config *ObvConfig) error { +func ensureProjectIDInObservabilityConfig(ctx context.Context, config *config) error { if config.DestinationProjectID == "" { // Try to fetch the GCP project id projectID := fetchDefaultProjectID(ctx) diff --git a/gcp/observability/exporting.go b/gcp/observability/exporting.go index 310a68398e3b..0c566f4b183c 100644 --- a/gcp/observability/exporting.go +++ b/gcp/observability/exporting.go @@ -44,7 +44,7 @@ type cloudLoggingExporter struct { logger *gcplogging.Logger } -func newCloudLoggingExporter(ctx context.Context, config *ObvConfig) (*cloudLoggingExporter, error) { +func newCloudLoggingExporter(ctx context.Context, config *config) (*cloudLoggingExporter, error) { c, err := gcplogging.NewClient(ctx, fmt.Sprintf("projects/%v", config.DestinationProjectID)) if err != nil { return nil, fmt.Errorf("failed to create cloudLoggingExporter: %v", err) diff --git a/gcp/observability/logging.go b/gcp/observability/logging.go index 8bab110de68d..f48af56997f4 100644 --- a/gcp/observability/logging.go +++ b/gcp/observability/logging.go @@ -247,7 +247,7 @@ func (l *binaryLogger) Close() { } } -func validateExistingMethodLoggerConfig(existing *iblog.MethodLoggerConfig, filter LogFilter) bool { +func validateExistingMethodLoggerConfig(existing *iblog.MethodLoggerConfig, filter logFilter) bool { // In future, we could add more validations. Currently, we only check if the // new filter configs are different than the existing one, if so, we log a // warning. @@ -257,7 +257,7 @@ func validateExistingMethodLoggerConfig(existing *iblog.MethodLoggerConfig, filt return existing == nil } -func createBinaryLoggerConfig(filters []LogFilter) iblog.LoggerConfig { +func createBinaryLoggerConfig(filters []logFilter) iblog.LoggerConfig { config := iblog.LoggerConfig{ Services: make(map[string]*iblog.MethodLoggerConfig), Methods: make(map[string]*iblog.MethodLoggerConfig), @@ -295,7 +295,7 @@ func createBinaryLoggerConfig(filters []LogFilter) iblog.LoggerConfig { // start is the core logic for setting up the custom binary logging logger, and // it's also useful for testing. -func (l *binaryLogger) start(config *ObvConfig, exporter loggingExporter) error { +func (l *binaryLogger) start(config *config, exporter loggingExporter) error { filters := config.LogFilters if len(filters) == 0 || exporter == nil { // Doing nothing is allowed @@ -317,7 +317,7 @@ func (l *binaryLogger) start(config *ObvConfig, exporter loggingExporter) error return nil } -func (l *binaryLogger) Start(ctx context.Context, config *ObvConfig) error { +func (l *binaryLogger) Start(ctx context.Context, config *config) error { if config == nil || !config.EnableCloudLogging { return nil } diff --git a/gcp/observability/observability_test.go b/gcp/observability/observability_test.go index 97c1cd3d2d85..3e8f1d704bda 100644 --- a/gcp/observability/observability_test.go +++ b/gcp/observability/observability_test.go @@ -196,7 +196,7 @@ func (te *test) clientConn() *grpc.ClientConn { return te.cc } -func (te *test) enablePluginWithConfig(config *ObvConfig) { +func (te *test) enablePluginWithConfig(config *config) { // Injects the fake exporter for testing purposes te.fle = &fakeLoggingExporter{t: te.t} defaultLogger = newBinaryLogger(nil) @@ -207,10 +207,10 @@ func (te *test) enablePluginWithConfig(config *ObvConfig) { } func (te *test) enablePluginWithCaptureAll() { - te.enablePluginWithConfig(&ObvConfig{ + te.enablePluginWithConfig(&config{ EnableCloudLogging: true, DestinationProjectID: "fake", - LogFilters: []LogFilter{ + LogFilters: []logFilter{ { Pattern: "*", HeaderBytes: infinitySizeBytes, @@ -222,7 +222,7 @@ func (te *test) enablePluginWithCaptureAll() { func (te *test) enableOpenCensus() { defaultMetricsReportingInterval = time.Millisecond * 100 - config := &ObvConfig{ + config := &config{ EnableCloudLogging: true, EnableCloudTrace: true, EnableCloudMonitoring: true, @@ -520,7 +520,7 @@ func (s) TestLoggingForErrorCall(t *testing.T) { func (s) TestEmptyConfig(t *testing.T) { te := newTest(t) defer te.tearDown() - te.enablePluginWithConfig(&ObvConfig{}) + te.enablePluginWithConfig(&config{}) te.startServer(&testServer{}) tc := testgrpc.NewTestServiceClient(te.clientConn()) @@ -552,10 +552,10 @@ func (s) TestOverrideConfig(t *testing.T) { // most specific one. The third filter allows message payload logging, and // others disabling the message payload logging. We should observe this // behavior latter. - te.enablePluginWithConfig(&ObvConfig{ + te.enablePluginWithConfig(&config{ EnableCloudLogging: true, DestinationProjectID: "fake", - LogFilters: []LogFilter{ + LogFilters: []logFilter{ { Pattern: "wont/match", MessageBytes: 0, @@ -619,10 +619,10 @@ func (s) TestNoMatch(t *testing.T) { // Setting 3 filters, expected to use the second filter. The second filter // allows message payload logging, and others disabling the message payload // logging. We should observe this behavior latter. - te.enablePluginWithConfig(&ObvConfig{ + te.enablePluginWithConfig(&config{ EnableCloudLogging: true, DestinationProjectID: "fake", - LogFilters: []LogFilter{ + LogFilters: []logFilter{ { Pattern: "wont/match", MessageBytes: 0, @@ -659,10 +659,10 @@ func (s) TestNoMatch(t *testing.T) { } func (s) TestRefuseStartWithInvalidPatterns(t *testing.T) { - config := &ObvConfig{ + config := &config{ EnableCloudLogging: true, DestinationProjectID: "fake", - LogFilters: []LogFilter{ + LogFilters: []logFilter{ { Pattern: ":-)", }, @@ -741,10 +741,10 @@ func (s) TestBothConfigEnvVarsSet(t *testing.T) { t.Fatalf("failed to create config in file system: %v", err) } // This configuration should be ignored, as precedence 2. - validConfig := &ObvConfig{ + validConfig := &config{ EnableCloudLogging: true, DestinationProjectID: "fake", - LogFilters: []LogFilter{ + LogFilters: []logFilter{ { Pattern: "*", HeaderBytes: infinitySizeBytes, @@ -788,11 +788,11 @@ func (s) TestOpenCensusIntegration(t *testing.T) { defer te.tearDown() fe := &fakeOpenCensusExporter{SeenViews: make(map[string]string), t: te.t} - defer func(ne func(config *ObvConfig) (tracingMetricsExporter, error)) { + defer func(ne func(config *config) (tracingMetricsExporter, error)) { newExporter = ne }(newExporter) - newExporter = func(config *ObvConfig) (tracingMetricsExporter, error) { + newExporter = func(config *config) (tracingMetricsExporter, error) { return fe, nil } @@ -845,11 +845,11 @@ func (s) TestOpenCensusIntegration(t *testing.T) { // observability configuration and set to two hardcoded values are passed to the // function to create an exporter. func (s) TestCustomTagsTracingMetrics(t *testing.T) { - defer func(ne func(config *ObvConfig) (tracingMetricsExporter, error)) { + defer func(ne func(config *config) (tracingMetricsExporter, error)) { newExporter = ne }(newExporter) fe := &fakeOpenCensusExporter{SeenViews: make(map[string]string), t: t} - newExporter = func(config *ObvConfig) (tracingMetricsExporter, error) { + newExporter = func(config *config) (tracingMetricsExporter, error) { ct := config.CustomTags if len(ct) < 1 { t.Fatalf("less than 2 custom tags sent in") diff --git a/gcp/observability/opencensus.go b/gcp/observability/opencensus.go index 6880799daf19..7d297f90bfc3 100644 --- a/gcp/observability/opencensus.go +++ b/gcp/observability/opencensus.go @@ -37,7 +37,7 @@ var ( defaultMetricsReportingInterval = time.Second * 30 ) -func convertTagsToMonitoringLabels(tags map[string]string) *stackdriver.Labels { +func tagsToMonitoringLabels(tags map[string]string) *stackdriver.Labels { labels := &stackdriver.Labels{} for k, v := range tags { labels.Set(k, v, "") @@ -45,7 +45,7 @@ func convertTagsToMonitoringLabels(tags map[string]string) *stackdriver.Labels { return labels } -func convertTagsToTraceAttributes(tags map[string]string) map[string]interface{} { +func tagsToTraceAttributes(tags map[string]string) map[string]interface{} { ta := make(map[string]interface{}, len(tags)) for k, v := range tags { ta[k] = v @@ -61,7 +61,7 @@ type tracingMetricsExporter interface { // global to stub out in tests var newExporter = newStackdriverExporter -func newStackdriverExporter(config *ObvConfig) (tracingMetricsExporter, error) { +func newStackdriverExporter(config *config) (tracingMetricsExporter, error) { // Create the Stackdriver exporter, which is shared between tracing and stats mr := monitoredresource.Autodetect() logger.Infof("Detected MonitoredResource:: %+v", mr) @@ -69,8 +69,8 @@ func newStackdriverExporter(config *ObvConfig) (tracingMetricsExporter, error) { exporter, err := stackdriver.NewExporter(stackdriver.Options{ ProjectID: config.DestinationProjectID, MonitoredResource: mr, - DefaultMonitoringLabels: convertTagsToMonitoringLabels(config.CustomTags), - DefaultTraceAttributes: convertTagsToTraceAttributes(config.CustomTags), + DefaultMonitoringLabels: tagsToMonitoringLabels(config.CustomTags), + DefaultTraceAttributes: tagsToTraceAttributes(config.CustomTags), }) if err != nil { return nil, fmt.Errorf("failed to create Stackdriver exporter: %v", err) @@ -80,7 +80,7 @@ func newStackdriverExporter(config *ObvConfig) (tracingMetricsExporter, error) { // This method accepts config and exporter; the exporter argument is exposed to // assist unit testing of the OpenCensus behavior. -func startOpenCensus(config *ObvConfig) error { +func startOpenCensus(config *config) error { // If both tracing and metrics are disabled, there's no point inject default // StatsHandler. if config == nil || (!config.EnableCloudTrace && !config.EnableCloudMonitoring) {