diff --git a/v2/telemetry.go b/v2/telemetry.go index dc0191d7..1e0320a2 100644 --- a/v2/telemetry.go +++ b/v2/telemetry.go @@ -31,6 +31,8 @@ package gax import ( "context" + "log/slog" + "sync" "go.opentelemetry.io/otel" "go.opentelemetry.io/otel/attribute" @@ -136,33 +138,38 @@ var defaultHistogramBoundaries = []float64{ // for a specific generated Google Cloud client library. // There should be exactly one ClientMetrics instance instantiated per generated client. type ClientMetrics struct { + get func() clientMetricsData +} + +type clientMetricsData struct { duration metric.Float64Histogram attr []attribute.KeyValue } -type clientMetricsOptions struct { +type telemetryOptions struct { provider metric.MeterProvider attributes map[string]string explicitBucketBoundaries []float64 + logger *slog.Logger } -// ClientMetricsOption is an option to configure a ClientMetrics instance. -// ClientMetricsOption works by modifying relevant fields of clientMetricsOptions. -type ClientMetricsOption interface { +// TelemetryOption is an option to configure a ClientMetrics instance. +// TelemetryOption works by modifying relevant fields of telemetryOptions. +type TelemetryOption interface { // Resolve applies the option by modifying opts. - Resolve(opts *clientMetricsOptions) + Resolve(opts *telemetryOptions) } type providerOpt struct { p metric.MeterProvider } -func (p providerOpt) Resolve(opts *clientMetricsOptions) { +func (p providerOpt) Resolve(opts *telemetryOptions) { opts.provider = p.p } // WithMeterProvider specifies the metric.MeterProvider to use for instruments. -func WithMeterProvider(p metric.MeterProvider) ClientMetricsOption { +func WithMeterProvider(p metric.MeterProvider) TelemetryOption { return &providerOpt{p: p} } @@ -170,12 +177,12 @@ type attrOpt struct { attrs map[string]string } -func (a attrOpt) Resolve(opts *clientMetricsOptions) { +func (a attrOpt) Resolve(opts *telemetryOptions) { opts.attributes = a.attrs } -// WithClientMetricsAttributes specifies the static attributes attachments. -func WithClientMetricsAttributes(attr map[string]string) ClientMetricsOption { +// WithTelemetryAttributes specifies the static attributes attachments. +func WithTelemetryAttributes(attr map[string]string) TelemetryOption { return &attrOpt{attrs: attr} } @@ -183,65 +190,106 @@ type boundariesOpt struct { boundaries []float64 } -func (b boundariesOpt) Resolve(opts *clientMetricsOptions) { +func (b boundariesOpt) Resolve(opts *telemetryOptions) { opts.explicitBucketBoundaries = b.boundaries } // WithExplicitBucketBoundaries overrides the default histogram bucket boundaries. -func WithExplicitBucketBoundaries(boundaries []float64) ClientMetricsOption { +func WithExplicitBucketBoundaries(boundaries []float64) TelemetryOption { return &boundariesOpt{boundaries: boundaries} } -// NewClientMetrics initializes and returns a new ClientMetrics instance. -// It is intended to be called once per generated client during initialization. -func NewClientMetrics(opts ...ClientMetricsOption) *ClientMetrics { - var config clientMetricsOptions - for _, opt := range opts { - opt.Resolve(&config) - } +type loggerOpt struct { + l *slog.Logger +} - provider := config.provider - if provider == nil { - provider = otel.GetMeterProvider() - } +func (l loggerOpt) Resolve(opts *telemetryOptions) { + opts.logger = l.l +} - var meterAttrs []attribute.KeyValue - if val, ok := config.attributes[ClientService]; ok { - meterAttrs = append(meterAttrs, attribute.KeyValue{Key: attribute.Key(keyGCPClientService), Value: attribute.StringValue(val)}) - } +// WithTelemetryLogger specifies a logger to record internal telemetry errors. +func WithTelemetryLogger(l *slog.Logger) TelemetryOption { + return &loggerOpt{l: l} +} - meterOpts := []metric.MeterOption{ - metric.WithInstrumentationVersion(config.attributes[ClientVersion]), - metric.WithSchemaURL(schemaURL), - } - if len(meterAttrs) > 0 { - meterOpts = append(meterOpts, metric.WithInstrumentationAttributes(meterAttrs...)) +func (config *telemetryOptions) meterProvider() metric.MeterProvider { + if config.provider != nil { + return config.provider } + return otel.GetMeterProvider() +} - meter := provider.Meter(config.attributes[ClientArtifact], meterOpts...) - - boundaries := config.explicitBucketBoundaries - if len(boundaries) == 0 { - boundaries = defaultHistogramBoundaries +func (config *telemetryOptions) bucketBoundaries() []float64 { + if len(config.explicitBucketBoundaries) > 0 { + return config.explicitBucketBoundaries } + return defaultHistogramBoundaries +} - duration, _ := meter.Float64Histogram( - metricName, - metric.WithDescription(metricDescription), - metric.WithUnit("s"), - metric.WithExplicitBucketBoundaries(boundaries...), - ) +// NewClientMetrics initializes and returns a new ClientMetrics instance. +// It is intended to be called once per generated client during initialization. +func NewClientMetrics(opts ...TelemetryOption) *ClientMetrics { + var config telemetryOptions + for _, opt := range opts { + opt.Resolve(&config) + } - var attr []attribute.KeyValue - if val, ok := config.attributes[URLDomain]; ok { - attr = append(attr, attribute.KeyValue{Key: attribute.Key(keyURLDomain), Value: attribute.StringValue(val)}) + return &ClientMetrics{ + get: sync.OnceValue(func() clientMetricsData { + provider := config.meterProvider() + + var meterAttrs []attribute.KeyValue + if val, ok := config.attributes[ClientService]; ok { + meterAttrs = append(meterAttrs, attribute.KeyValue{Key: attribute.Key(keyGCPClientService), Value: attribute.StringValue(val)}) + } + + meterOpts := []metric.MeterOption{ + metric.WithInstrumentationVersion(config.attributes[ClientVersion]), + metric.WithSchemaURL(schemaURL), + } + if len(meterAttrs) > 0 { + meterOpts = append(meterOpts, metric.WithInstrumentationAttributes(meterAttrs...)) + } + + meter := provider.Meter(config.attributes[ClientArtifact], meterOpts...) + + boundaries := config.bucketBoundaries() + + duration, err := meter.Float64Histogram( + metricName, + metric.WithDescription(metricDescription), + metric.WithUnit("s"), + metric.WithExplicitBucketBoundaries(boundaries...), + ) + if err != nil && config.logger != nil { + config.logger.Warn("failed to initialize OTel duration histogram", "error", err) + } + + var attr []attribute.KeyValue + if val, ok := config.attributes[URLDomain]; ok { + attr = append(attr, attribute.KeyValue{Key: attribute.Key(keyURLDomain), Value: attribute.StringValue(val)}) + } + if val, ok := config.attributes[RPCSystem]; ok { + attr = append(attr, attribute.KeyValue{Key: attribute.Key(keyRPCSystemName), Value: attribute.StringValue(val)}) + } + return clientMetricsData{ + duration: duration, + attr: attr, + } + }), } - if val, ok := config.attributes[RPCSystem]; ok { - attr = append(attr, attribute.KeyValue{Key: attribute.Key(keyRPCSystemName), Value: attribute.StringValue(val)}) +} + +func (cm *ClientMetrics) durationHistogram() metric.Float64Histogram { + if cm == nil || cm.get == nil { + return nil } + return cm.get().duration +} - return &ClientMetrics{ - duration: duration, - attr: attr, +func (cm *ClientMetrics) attributes() []attribute.KeyValue { + if cm == nil || cm.get == nil { + return nil } + return cm.get().attr } diff --git a/v2/telemetry_test.go b/v2/telemetry_test.go index fa2c7514..a74ba03b 100644 --- a/v2/telemetry_test.go +++ b/v2/telemetry_test.go @@ -30,7 +30,9 @@ package gax import ( + "bytes" "context" + "log/slog" "os/exec" "strings" "testing" @@ -47,15 +49,15 @@ func TestNewClientMetrics(t *testing.T) { tests := []struct { name string - opts []ClientMetricsOption + opts []TelemetryOption wantScopeAttr map[string]string wantDataAttr map[string]string useCustom bool }{ { name: "default boundaries and global provider", - opts: []ClientMetricsOption{ - WithClientMetricsAttributes(map[string]string{ + opts: []TelemetryOption{ + WithTelemetryAttributes(map[string]string{ ClientArtifact: "test-lib", ClientVersion: "v1.0.0", }), @@ -65,8 +67,8 @@ func TestNewClientMetrics(t *testing.T) { }, { name: "custom provider and custom boundaries", - opts: []ClientMetricsOption{ - WithClientMetricsAttributes(map[string]string{ + opts: []TelemetryOption{ + WithTelemetryAttributes(map[string]string{ ClientArtifact: "test-lib-2", ClientVersion: "v1.0.1", }), @@ -78,8 +80,8 @@ func TestNewClientMetrics(t *testing.T) { }, { name: "with static attributes", - opts: []ClientMetricsOption{ - WithClientMetricsAttributes(map[string]string{ + opts: []TelemetryOption{ + WithTelemetryAttributes(map[string]string{ ClientArtifact: "test-lib-3", ClientVersion: "v1.0.2", ClientService: "myservice", @@ -96,6 +98,30 @@ func TestNewClientMetrics(t *testing.T) { "url.domain": "test.domain", }, }, + { + name: "with logger", + opts: []TelemetryOption{ + WithTelemetryAttributes(map[string]string{ + ClientArtifact: "test-lib", + ClientVersion: "v1.0.0", + }), + WithTelemetryLogger(slog.Default()), + }, + wantScopeAttr: map[string]string{}, + wantDataAttr: map[string]string{}, + }, + { + name: "with nil logger", + opts: []TelemetryOption{ + WithTelemetryAttributes(map[string]string{ + ClientArtifact: "test-lib", + ClientVersion: "v1.0.0", + }), + WithTelemetryLogger(nil), + }, + wantScopeAttr: map[string]string{}, + wantDataAttr: map[string]string{}, + }, } for _, tt := range tests { @@ -104,7 +130,7 @@ func TestNewClientMetrics(t *testing.T) { if tt.useCustom { cmOpts = append(cmOpts, WithMeterProvider(customProvider)) cm := NewClientMetrics(cmOpts...) - if cm == nil || cm.duration == nil { + if cm == nil || cm.durationHistogram() == nil { t.Fatalf("expected initialized metrics") } return // we can't observe noop provider, so just verify it doesn't panic @@ -121,12 +147,12 @@ func TestNewClientMetrics(t *testing.T) { if cm == nil { t.Fatalf("NewClientMetrics returned nil") } - if cm.duration == nil { + if cm.durationHistogram() == nil { t.Fatalf("expected Float64Histogram to be initialized, got nil") } // Record a dummy value so we can collect the metrics and inspect Scope attributes - cm.duration.Record(context.Background(), 1.0, metric.WithAttributes(cm.attr...)) + cm.durationHistogram().Record(context.Background(), 1.0, metric.WithAttributes(cm.attributes()...)) var rm metricdata.ResourceMetrics if err := reader.Collect(context.Background(), &rm); err != nil { @@ -181,8 +207,8 @@ func TestNewClientMetrics(t *testing.T) { } func TestNewClientMetrics_GlobalFallback(t *testing.T) { - opts := []ClientMetricsOption{ - WithClientMetricsAttributes(map[string]string{ + opts := []TelemetryOption{ + WithTelemetryAttributes(map[string]string{ ClientArtifact: "test-global-fallback", }), } @@ -190,11 +216,63 @@ func TestNewClientMetrics_GlobalFallback(t *testing.T) { if cm == nil { t.Fatalf("expected non-nil ClientMetrics") } - if cm.duration == nil { + if cm.durationHistogram() == nil { t.Errorf("expected non-nil duration histogram") } } +func TestNewClientMetrics_InitializationError(t *testing.T) { + // Setup SDK + reader := sdkmetric.NewManualReader() + provider := sdkmetric.NewMeterProvider(sdkmetric.WithReader(reader)) + + t.Run("with logger", func(t *testing.T) { + // Capture log output + var buf bytes.Buffer + logger := slog.New(slog.NewTextHandler(&buf, nil)) + + opts := []TelemetryOption{ + WithMeterProvider(provider), + WithTelemetryAttributes(map[string]string{ + ClientArtifact: "test-error", + }), + WithExplicitBucketBoundaries([]float64{10.0, 5.0}), // Invalid boundaries trigger error + WithTelemetryLogger(logger), + } + + cm := NewClientMetrics(opts...) + if cm == nil { + t.Fatalf("expected non-nil ClientMetrics") + } + + // Trigger lazy initialization, which should fail and log + cm.durationHistogram() + + logOutput := buf.String() + if !strings.Contains(logOutput, "failed to initialize OTel duration histogram") { + t.Errorf("expected initialization error to be logged, got: %s", logOutput) + } + }) + + t.Run("without logger", func(t *testing.T) { + opts := []TelemetryOption{ + WithMeterProvider(provider), + WithTelemetryAttributes(map[string]string{ + ClientArtifact: "test-error", + }), + WithExplicitBucketBoundaries([]float64{10.0, 5.0}), // Invalid boundaries trigger error + } + + cm := NewClientMetrics(opts...) + if cm == nil { + t.Fatalf("expected non-nil ClientMetrics") + } + + // Trigger lazy initialization, which should fail but NOT panic + cm.durationHistogram() + }) +} + func TestTelemetryConfigKeys(t *testing.T) { // These keys are referenced by generated client code. They must not be changed, // otherwise generated code will fail to compile or pass attributes incorrectly.