From 7b310e0d597e563bdec423caa49eb40839c43e3d Mon Sep 17 00:00:00 2001 From: avilevy Date: Wed, 16 Nov 2022 19:37:58 +0000 Subject: [PATCH 1/2] Added the ability to disable automatic MetricDescriptor creation --- exporter/metric/metric.go | 7 +++- exporter/metric/metric_test.go | 75 ++++++++++++++++++++++++++++++++++ exporter/metric/option.go | 11 +++++ 3 files changed, 92 insertions(+), 1 deletion(-) diff --git a/exporter/metric/metric.go b/exporter/metric/metric.go index c7c3ae6d1..6fe045abe 100644 --- a/exporter/metric/metric.go +++ b/exporter/metric/metric.go @@ -148,6 +148,10 @@ func (me *metricExporter) exportMetricDescriptor(ctx context.Context, rm metricd } } + if me.o.disableCreateMetricDescriptors { + return nil + } + // TODO: This process is synchronous and blocks longer time if records in cps // have many different descriptors. In the cps.ForEach above, it should spawn // goroutines to send CreateMetricDescriptorRequest asynchronously in the case @@ -600,7 +604,8 @@ func numberDataPointToValue[N int64 | float64]( // > allowed. // > * Label name must start with a letter or digit. // > * The maximum length of a label name is 100 characters. -// Note: this does not truncate if a label is too long. +// +// Note: this does not truncate if a label is too long. func normalizeLabelKey(s string) string { if len(s) == 0 { return s diff --git a/exporter/metric/metric_test.go b/exporter/metric/metric_test.go index d911167f7..d6a54cfba 100644 --- a/exporter/metric/metric_test.go +++ b/exporter/metric/metric_test.go @@ -707,6 +707,81 @@ func (m *mock) CreateMetricDescriptor(ctx context.Context, req *monitoringpb.Cre return m.createMetricDescriptor(ctx, req) } +func TestExportWithDisableCreateMetricDescriptors(t *testing.T) { + for _, tc := range []struct { + desc string + disableCreateMetricsDescriptor bool + expectExportMetricDescriptor bool + }{ + { + desc: "default", + disableCreateMetricsDescriptor: false, + expectExportMetricDescriptor: true, + }, + { + desc: "Disable CreateMetricsDescriptor", + disableCreateMetricsDescriptor: true, + expectExportMetricDescriptor: false, + }, + } { + t.Run(tc.desc, func(t *testing.T) { + server := grpc.NewServer() + + flag := false + + m := mock{ + createTimeSeries: func(ctx context.Context, r *monitoringpb.CreateTimeSeriesRequest) (*emptypb.Empty, error) { + return &emptypb.Empty{}, nil + }, + createMetricDescriptor: func(ctx context.Context, req *monitoringpb.CreateMetricDescriptorRequest) (*googlemetricpb.MetricDescriptor, error) { + flag = true + return req.MetricDescriptor, nil + }, + } + + monitoringpb.RegisterMetricServiceServer(server, &m) + + lis, err := net.Listen("tcp", "127.0.0.1:0") + require.NoError(t, err) + go server.Serve(lis) + + clientOpts := []option.ClientOption{ + option.WithEndpoint(lis.Addr().String()), + option.WithoutAuthentication(), + option.WithGRPCDialOption(grpc.WithTransportCredentials(insecure.NewCredentials())), + } + res := &resource.Resource{} + ctx := context.Background() + + opts := []Option{ + WithProjectID("PROJECT_ID_NOT_REAL"), + WithMonitoringClientOptions(clientOpts...), + WithMetricDescriptorTypeFormatter(formatter), + WithDisableCreateMetricDescriptors(tc.disableCreateMetricsDescriptor), + } + + exporter, err := New(opts...) + if err != nil { + t.Errorf("Error occurred when creating exporter: %v", err) + } + provider := metric.NewMeterProvider( + metric.WithReader(metric.NewPeriodicReader(exporter)), + metric.WithResource(res), + ) + + meter := provider.Meter("test") + + counter, err := meter.SyncInt64().Counter("name.lastvalue") + require.NoError(t, err) + + counter.Add(ctx, 1) + require.NoError(t, provider.ForceFlush(ctx)) + server.Stop() + require.Equal(t, tc.expectExportMetricDescriptor, flag) + }) + } +} + func TestExportMetricsWithUserAgent(t *testing.T) { for _, tc := range []struct { desc string diff --git a/exporter/metric/option.go b/exporter/metric/option.go index d2bef7a4e..8fdb3d709 100644 --- a/exporter/metric/option.go +++ b/exporter/metric/option.go @@ -60,6 +60,9 @@ type options struct { // to the underlying Stackdriver Monitoring API client. // Optional. monitoringClientOptions []apioption.ClientOption + + // disableCreateMetricDescriptors disables automatic MetricDescriptor creation + disableCreateMetricDescriptors bool } // WithProjectID sets Google Cloud Platform project as projectID. @@ -116,3 +119,11 @@ func DefaultResourceAttributesFilter(kv attribute.KeyValue) bool { func NoAttributes(attribute.KeyValue) bool { return false } + +// WithDisableCreateMetricDescriptors will disable the automatic creation of +// MetricDescriptors when an unknown metric is set to be exported. +func WithDisableCreateMetricDescriptors(create bool) func(o *options) { + return func(o *options) { + o.disableCreateMetricDescriptors = create + } +} From 33257b31e3a9cec0824a4a00369e9f7a587ab8a8 Mon Sep 17 00:00:00 2001 From: avilevy Date: Wed, 16 Nov 2022 20:43:31 +0000 Subject: [PATCH 2/2] Addressed comments --- exporter/metric/metric.go | 8 ++++---- exporter/metric/metric_test.go | 11 +++++++---- exporter/metric/option.go | 4 ++-- 3 files changed, 13 insertions(+), 10 deletions(-) diff --git a/exporter/metric/metric.go b/exporter/metric/metric.go index 6fe045abe..8c35fadde 100644 --- a/exporter/metric/metric.go +++ b/exporter/metric/metric.go @@ -129,6 +129,10 @@ func (me *metricExporter) Export(ctx context.Context, rm metricdata.ResourceMetr // exportMetricDescriptor create MetricDescriptor from the record // if the descriptor is not registered in Cloud Monitoring yet. func (me *metricExporter) exportMetricDescriptor(ctx context.Context, rm metricdata.ResourceMetrics) error { + if me.o.disableCreateMetricDescriptors { + return nil + } + me.mdLock.Lock() defer me.mdLock.Unlock() mds := make(map[key]*googlemetricpb.MetricDescriptor) @@ -148,10 +152,6 @@ func (me *metricExporter) exportMetricDescriptor(ctx context.Context, rm metricd } } - if me.o.disableCreateMetricDescriptors { - return nil - } - // TODO: This process is synchronous and blocks longer time if records in cps // have many different descriptors. In the cps.ForEach above, it should spawn // goroutines to send CreateMetricDescriptorRequest asynchronously in the case diff --git a/exporter/metric/metric_test.go b/exporter/metric/metric_test.go index d6a54cfba..1227e18b7 100644 --- a/exporter/metric/metric_test.go +++ b/exporter/metric/metric_test.go @@ -727,14 +727,14 @@ func TestExportWithDisableCreateMetricDescriptors(t *testing.T) { t.Run(tc.desc, func(t *testing.T) { server := grpc.NewServer() - flag := false + metricDescriptorCreated := false m := mock{ createTimeSeries: func(ctx context.Context, r *monitoringpb.CreateTimeSeriesRequest) (*emptypb.Empty, error) { return &emptypb.Empty{}, nil }, createMetricDescriptor: func(ctx context.Context, req *monitoringpb.CreateMetricDescriptorRequest) (*googlemetricpb.MetricDescriptor, error) { - flag = true + metricDescriptorCreated = true return req.MetricDescriptor, nil }, } @@ -757,7 +757,10 @@ func TestExportWithDisableCreateMetricDescriptors(t *testing.T) { WithProjectID("PROJECT_ID_NOT_REAL"), WithMonitoringClientOptions(clientOpts...), WithMetricDescriptorTypeFormatter(formatter), - WithDisableCreateMetricDescriptors(tc.disableCreateMetricsDescriptor), + } + + if tc.disableCreateMetricsDescriptor { + opts = append(opts, WithDisableCreateMetricDescriptors()) } exporter, err := New(opts...) @@ -777,7 +780,7 @@ func TestExportWithDisableCreateMetricDescriptors(t *testing.T) { counter.Add(ctx, 1) require.NoError(t, provider.ForceFlush(ctx)) server.Stop() - require.Equal(t, tc.expectExportMetricDescriptor, flag) + require.Equal(t, tc.expectExportMetricDescriptor, metricDescriptorCreated) }) } } diff --git a/exporter/metric/option.go b/exporter/metric/option.go index 8fdb3d709..42527782e 100644 --- a/exporter/metric/option.go +++ b/exporter/metric/option.go @@ -122,8 +122,8 @@ func NoAttributes(attribute.KeyValue) bool { // WithDisableCreateMetricDescriptors will disable the automatic creation of // MetricDescriptors when an unknown metric is set to be exported. -func WithDisableCreateMetricDescriptors(create bool) func(o *options) { +func WithDisableCreateMetricDescriptors() func(o *options) { return func(o *options) { - o.disableCreateMetricDescriptors = create + o.disableCreateMetricDescriptors = true } }