Skip to content

Commit

Permalink
config: fix bug where WithResourceAsConstantLabels wasn't set
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
codeboten committed Oct 16, 2024
1 parent d9c9bbe commit 336b2ac
Show file tree
Hide file tree
Showing 2 changed files with 59 additions and 8 deletions.
26 changes: 18 additions & 8 deletions config/metric.go
Original file line number Diff line number Diff line change
Expand Up @@ -297,14 +297,8 @@ func newIncludeExcludeFilter(lists *IncludeExclude) (attribute.Filter, error) {
}, nil
}

func prometheusReader(ctx context.Context, prometheusConfig *Prometheus) (sdkmetric.Reader, error) {
func prometheusReaderOpts(prometheusConfig *Prometheus) ([]otelprom.Option, 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())
}
Expand All @@ -319,7 +313,23 @@ func prometheusReader(ctx context.Context, prometheusConfig *Prometheus) (sdkmet
if err != nil {
return nil, err
}
otelprom.WithResourceAsConstantLabels(f)
opts = append(opts, otelprom.WithResourceAsConstantLabels(f))
}

return opts, nil
}

func prometheusReader(ctx context.Context, prometheusConfig *Prometheus) (sdkmetric.Reader, error) {
if prometheusConfig.Host == nil {
return nil, fmt.Errorf("host must be specified")
}
if prometheusConfig.Port == nil {
return nil, fmt.Errorf("port must be specified")
}

opts, err := prometheusReaderOpts(prometheusConfig)
if err != nil {
return nil, err
}

reg := prometheus.NewRegistry()
Expand Down
41 changes: 41 additions & 0 deletions config/metric_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
}
}

0 comments on commit 336b2ac

Please sign in to comment.