From 13536ddd8c872685baba158a42a7b2ad37129b37 Mon Sep 17 00:00:00 2001 From: Alex Boten <223565+codeboten@users.noreply.github.com> Date: Thu, 17 Oct 2024 06:03:50 -0700 Subject: [PATCH] config: fix bug where WithResourceAsConstantLabels wasn't set (#6260) There was a bug in the config package where the WithResourceAsConstantLabels option wasn't appended to the list of options for the Prometheus reader. Fixed that issue and added a test to validate the correct number of options is set. --------- Signed-off-by: Alex Boten <223565+codeboten@users.noreply.github.com> --- CHANGELOG.md | 1 + config/metric.go | 42 ++++++++++++++++++++++++++---------------- config/metric_test.go | 41 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 68 insertions(+), 16 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 93bb4251ccc..d641da679c5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - Transform nil attribute values to `log.Value` zero value instead of panicking in `go.opentelemetry.io/contrib/bridges/otelzap`. (#6237) - Transform nil attribute values to `log.Value` zero value instead of `log.StringValue("")` in `go.opentelemetry.io/contrib/bridges/otelslog`. (#6246) - Fix `NewClientHandler` so that `rpc.client.request.*` metrics measure requests instead of responses and `rpc.client.responses.*` metrics measure responses instead of requests in `go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc`. (#6250) +- Fix issue in `go.opentelemetry.io/contrib/config` causing `otelprom.WithResourceAsConstantLabels` configuration to not be respected. (#6260) diff --git a/config/metric.go b/config/metric.go index 743d594456e..fb2dd23905c 100644 --- a/config/metric.go +++ b/config/metric.go @@ -298,28 +298,16 @@ func newIncludeExcludeFilter(lists *IncludeExclude) (attribute.Filter, error) { } func prometheusReader(ctx context.Context, prometheusConfig *Prometheus) (sdkmetric.Reader, error) { - var opts []otelprom.Option if prometheusConfig.Host == nil { return nil, fmt.Errorf("host must be specified") } if prometheusConfig.Port == nil { return nil, fmt.Errorf("port must be specified") } - if prometheusConfig.WithoutScopeInfo != nil && *prometheusConfig.WithoutScopeInfo { - opts = append(opts, otelprom.WithoutScopeInfo()) - } - if prometheusConfig.WithoutTypeSuffix != nil && *prometheusConfig.WithoutTypeSuffix { - opts = append(opts, otelprom.WithoutCounterSuffixes()) - } - if prometheusConfig.WithoutUnits != nil && *prometheusConfig.WithoutUnits { - opts = append(opts, otelprom.WithoutUnits()) - } - if prometheusConfig.WithResourceConstantLabels != nil { - f, err := newIncludeExcludeFilter(prometheusConfig.WithResourceConstantLabels) - if err != nil { - return nil, err - } - otelprom.WithResourceAsConstantLabels(f) + + opts, err := prometheusReaderOpts(prometheusConfig) + if err != nil { + return nil, err } reg := prometheus.NewRegistry() @@ -358,6 +346,28 @@ func prometheusReader(ctx context.Context, prometheusConfig *Prometheus) (sdkmet return readerWithServer{reader, &server}, nil } +func prometheusReaderOpts(prometheusConfig *Prometheus) ([]otelprom.Option, error) { + var opts []otelprom.Option + if prometheusConfig.WithoutScopeInfo != nil && *prometheusConfig.WithoutScopeInfo { + opts = append(opts, otelprom.WithoutScopeInfo()) + } + if prometheusConfig.WithoutTypeSuffix != nil && *prometheusConfig.WithoutTypeSuffix { + opts = append(opts, otelprom.WithoutCounterSuffixes()) + } + if prometheusConfig.WithoutUnits != nil && *prometheusConfig.WithoutUnits { + opts = append(opts, otelprom.WithoutUnits()) + } + if prometheusConfig.WithResourceConstantLabels != nil { + f, err := newIncludeExcludeFilter(prometheusConfig.WithResourceConstantLabels) + if err != nil { + return nil, err + } + opts = append(opts, otelprom.WithResourceAsConstantLabels(f)) + } + + return opts, nil +} + type readerWithServer struct { sdkmetric.Reader server *http.Server diff --git a/config/metric_test.go b/config/metric_test.go index 8d7ced508ca..3940e765b20 100644 --- a/config/metric_test.go +++ b/config/metric_test.go @@ -1138,3 +1138,44 @@ func TestNewIncludeExcludeFilterError(t *testing.T) { })) require.Equal(t, fmt.Errorf("attribute cannot be in both include and exclude list: foo"), err) } + +func TestPrometheusReaderOpts(t *testing.T) { + testCases := []struct { + name string + cfg Prometheus + wantOptions int + }{ + { + name: "no options", + cfg: Prometheus{}, + wantOptions: 0, + }, + { + name: "all set", + cfg: Prometheus{ + WithoutScopeInfo: ptr(true), + WithoutTypeSuffix: ptr(true), + WithoutUnits: ptr(true), + WithResourceConstantLabels: &IncludeExclude{}, + }, + wantOptions: 4, + }, + { + name: "all set false", + cfg: Prometheus{ + WithoutScopeInfo: ptr(false), + WithoutTypeSuffix: ptr(false), + WithoutUnits: ptr(false), + WithResourceConstantLabels: &IncludeExclude{}, + }, + wantOptions: 1, + }, + } + for _, tt := range testCases { + t.Run(tt.name, func(t *testing.T) { + opts, err := prometheusReaderOpts(&tt.cfg) + require.NoError(t, err) + require.Len(t, opts, tt.wantOptions) + }) + } +}