Skip to content

Commit 295293c

Browse files
authored
Cleanup the Prometheus config (jaegertracing#5720)
## Which problem is this PR solving? - Part of jaegertracing#5632 ## Description of the changes - Cleaned up the Prometheus config - Removed namespaceConfig as its not needed for a metrics storage ## How was this change tested? - `make test` ## Checklist - [x] I have read https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md - [x] I have signed all commits <s> - [ ] I have added unit tests for the new functionality </s> - [x] I have run lint and test steps successfully - for `jaeger`: `make lint test` - for `jaeger-ui`: `yarn lint` and `yarn test` --------- Signed-off-by: FlamingSaint <[email protected]>
1 parent ea2ec18 commit 295293c

File tree

4 files changed

+43
-52
lines changed

4 files changed

+43
-52
lines changed

pkg/version/build_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -43,4 +43,4 @@ func TestString(t *testing.T) {
4343
}
4444
expectedOutput := "git-commit=foobar, git-version=v1.2.3, build-date=2024-01-04"
4545
assert.Equal(t, expectedOutput, test.String())
46-
}
46+
}

plugin/metrics/prometheus/factory.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ type Factory struct {
4040
func NewFactory() *Factory {
4141
return &Factory{
4242
tracer: otel.GetTracerProvider(),
43-
options: NewOptions("prometheus"),
43+
options: NewOptions(),
4444
}
4545
}
4646

@@ -64,5 +64,5 @@ func (f *Factory) Initialize(logger *zap.Logger) error {
6464

6565
// CreateMetricsReader implements storage.MetricsFactory.
6666
func (f *Factory) CreateMetricsReader() (metricsstore.Reader, error) {
67-
return prometheusstore.NewMetricsReader(f.options.Primary.Configuration, f.logger, f.tracer)
67+
return prometheusstore.NewMetricsReader(f.options.Configuration, f.logger, f.tracer)
6868
}

plugin/metrics/prometheus/factory_test.go

+13-14
Original file line numberDiff line numberDiff line change
@@ -34,14 +34,13 @@ func TestPrometheusFactory(t *testing.T) {
3434
f := NewFactory()
3535
require.NoError(t, f.Initialize(zap.NewNop()))
3636
assert.NotNil(t, f.logger)
37-
assert.Equal(t, "prometheus", f.options.Primary.namespace)
3837

3938
listener, err := net.Listen("tcp", "localhost:")
4039
require.NoError(t, err)
4140
assert.NotNil(t, listener)
4241
defer listener.Close()
4342

44-
f.options.Primary.ServerURL = "http://" + listener.Addr().String()
43+
f.options.ServerURL = "http://" + listener.Addr().String()
4544
reader, err := f.CreateMetricsReader()
4645

4746
require.NoError(t, err)
@@ -50,11 +49,11 @@ func TestPrometheusFactory(t *testing.T) {
5049

5150
func TestWithDefaultConfiguration(t *testing.T) {
5251
f := NewFactory()
53-
assert.Equal(t, "http://localhost:9090", f.options.Primary.ServerURL)
54-
assert.Equal(t, 30*time.Second, f.options.Primary.ConnectTimeout)
52+
assert.Equal(t, "http://localhost:9090", f.options.ServerURL)
53+
assert.Equal(t, 30*time.Second, f.options.ConnectTimeout)
5554

56-
assert.Empty(t, f.options.Primary.MetricNamespace)
57-
assert.Equal(t, "ms", f.options.Primary.LatencyUnit)
55+
assert.Empty(t, f.options.MetricNamespace)
56+
assert.Equal(t, "ms", f.options.LatencyUnit)
5857
}
5958

6059
func TestWithConfiguration(t *testing.T) {
@@ -69,10 +68,10 @@ func TestWithConfiguration(t *testing.T) {
6968
})
7069
require.NoError(t, err)
7170
f.InitFromViper(v, zap.NewNop())
72-
assert.Equal(t, "http://localhost:1234", f.options.Primary.ServerURL)
73-
assert.Equal(t, 5*time.Second, f.options.Primary.ConnectTimeout)
74-
assert.Equal(t, "test/test_file.txt", f.options.Primary.TokenFilePath)
75-
assert.False(t, f.options.Primary.TokenOverrideFromContext)
71+
assert.Equal(t, "http://localhost:1234", f.options.ServerURL)
72+
assert.Equal(t, 5*time.Second, f.options.ConnectTimeout)
73+
assert.Equal(t, "test/test_file.txt", f.options.TokenFilePath)
74+
assert.False(t, f.options.TokenOverrideFromContext)
7675
})
7776
t.Run("with space in token file path", func(t *testing.T) {
7877
f := NewFactory()
@@ -82,7 +81,7 @@ func TestWithConfiguration(t *testing.T) {
8281
})
8382
require.NoError(t, err)
8483
f.InitFromViper(v, zap.NewNop())
85-
assert.Equal(t, "test/ test file.txt", f.options.Primary.TokenFilePath)
84+
assert.Equal(t, "test/ test file.txt", f.options.TokenFilePath)
8685
})
8786
t.Run("with custom configuration of prometheus.query", func(t *testing.T) {
8887
f := NewFactory()
@@ -93,8 +92,8 @@ func TestWithConfiguration(t *testing.T) {
9392
})
9493
require.NoError(t, err)
9594
f.InitFromViper(v, zap.NewNop())
96-
assert.Equal(t, "mynamespace", f.options.Primary.MetricNamespace)
97-
assert.Equal(t, "ms", f.options.Primary.LatencyUnit)
95+
assert.Equal(t, "mynamespace", f.options.MetricNamespace)
96+
assert.Equal(t, "ms", f.options.LatencyUnit)
9897
})
9998
t.Run("with invalid prometheus.query.duration-unit", func(t *testing.T) {
10099
defer func() {
@@ -110,7 +109,7 @@ func TestWithConfiguration(t *testing.T) {
110109
})
111110
require.NoError(t, err)
112111
f.InitFromViper(v, zap.NewNop())
113-
require.Empty(t, f.options.Primary.LatencyUnit)
112+
require.Empty(t, f.options.LatencyUnit)
114113
})
115114
}
116115

plugin/metrics/prometheus/options.go

+27-35
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,8 @@ import (
2727
)
2828

2929
const (
30+
prefix = "prometheus"
31+
3032
suffixServerURL = ".server-url"
3133
suffixConnectTimeout = ".connect-timeout"
3234
suffixTokenFilePath = ".token-file"
@@ -48,18 +50,13 @@ const (
4850
defaultNormalizeDuration = false
4951
)
5052

51-
type namespaceConfig struct {
52-
config.Configuration `mapstructure:",squash"`
53-
namespace string
54-
}
55-
5653
// Options stores the configuration entries for this storage.
5754
type Options struct {
58-
Primary namespaceConfig `mapstructure:",squash"`
55+
config.Configuration `mapstructure:",squash"`
5956
}
6057

6158
// NewOptions creates a new Options struct.
62-
func NewOptions(primaryNamespace string) *Options {
59+
func NewOptions() *Options {
6360
defaultConfig := config.Configuration{
6461
ServerURL: defaultServerURL,
6562
ConnectTimeout: defaultConnectTimeout,
@@ -71,75 +68,70 @@ func NewOptions(primaryNamespace string) *Options {
7168
}
7269

7370
return &Options{
74-
Primary: namespaceConfig{
75-
Configuration: defaultConfig,
76-
namespace: primaryNamespace,
77-
},
71+
Configuration: defaultConfig,
7872
}
7973
}
8074

8175
// AddFlags from this storage to the CLI.
8276
func (opt *Options) AddFlags(flagSet *flag.FlagSet) {
83-
nsConfig := &opt.Primary
84-
flagSet.String(nsConfig.namespace+suffixServerURL, defaultServerURL,
77+
flagSet.String(prefix+suffixServerURL, defaultServerURL,
8578
"The Prometheus server's URL, must include the protocol scheme e.g. http://localhost:9090")
86-
flagSet.Duration(nsConfig.namespace+suffixConnectTimeout, defaultConnectTimeout,
79+
flagSet.Duration(prefix+suffixConnectTimeout, defaultConnectTimeout,
8780
"The period to wait for a connection to Prometheus when executing queries.")
88-
flagSet.String(nsConfig.namespace+suffixTokenFilePath, defaultTokenFilePath,
81+
flagSet.String(prefix+suffixTokenFilePath, defaultTokenFilePath,
8982
"The path to a file containing the bearer token which will be included when executing queries against the Prometheus API.")
90-
flagSet.Bool(nsConfig.namespace+suffixOverrideFromContext, true,
83+
flagSet.Bool(prefix+suffixOverrideFromContext, true,
9184
"Whether the bearer token should be overridden from context (incoming request)")
92-
flagSet.String(nsConfig.namespace+suffixMetricNamespace, defaultMetricNamespace,
85+
flagSet.String(prefix+suffixMetricNamespace, defaultMetricNamespace,
9386
`The metric namespace that is prefixed to the metric name. A '.' separator will be added between `+
9487
`the namespace and the metric name.`)
95-
flagSet.String(nsConfig.namespace+suffixLatencyUnit, defaultLatencyUnit,
88+
flagSet.String(prefix+suffixLatencyUnit, defaultLatencyUnit,
9689
`The units used for the "latency" histogram. It can be either "ms" or "s" and should be consistent with the `+
9790
`histogram unit value set in the spanmetrics connector (see: `+
9891
`https://github.com/open-telemetry/opentelemetry-collector-contrib/tree/main/connector/spanmetricsconnector#configurations). `+
9992
`This also helps jaeger-query determine the metric name when querying for "latency" metrics.`)
100-
flagSet.Bool(nsConfig.namespace+suffixNormalizeCalls, defaultNormalizeCalls,
93+
flagSet.Bool(prefix+suffixNormalizeCalls, defaultNormalizeCalls,
10194
`Whether to normalize the "calls" metric name according to `+
10295
`https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/main/pkg/translator/prometheus/README.md. `+
10396
`For example: `+
10497
`"calls" (not normalized) -> "calls_total" (normalized), `)
105-
flagSet.Bool(nsConfig.namespace+suffixNormalizeDuration, defaultNormalizeDuration,
98+
flagSet.Bool(prefix+suffixNormalizeDuration, defaultNormalizeDuration,
10699
`Whether to normalize the "duration" metric name according to `+
107100
`https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/main/pkg/translator/prometheus/README.md. `+
108101
`For example: `+
109102
`"duration_bucket" (not normalized) -> "duration_milliseconds_bucket (normalized)"`)
110103

111-
nsConfig.getTLSFlagsConfig().AddFlags(flagSet)
104+
opt.getTLSFlagsConfig().AddFlags(flagSet)
112105
}
113106

114107
// InitFromViper initializes the options struct with values from Viper.
115108
func (opt *Options) InitFromViper(v *viper.Viper) error {
116-
cfg := &opt.Primary
117-
cfg.ServerURL = stripWhiteSpace(v.GetString(cfg.namespace + suffixServerURL))
118-
cfg.ConnectTimeout = v.GetDuration(cfg.namespace + suffixConnectTimeout)
119-
cfg.TokenFilePath = v.GetString(cfg.namespace + suffixTokenFilePath)
109+
opt.ServerURL = stripWhiteSpace(v.GetString(prefix + suffixServerURL))
110+
opt.ConnectTimeout = v.GetDuration(prefix + suffixConnectTimeout)
111+
opt.TokenFilePath = v.GetString(prefix + suffixTokenFilePath)
120112

121-
cfg.MetricNamespace = v.GetString(cfg.namespace + suffixMetricNamespace)
122-
cfg.LatencyUnit = v.GetString(cfg.namespace + suffixLatencyUnit)
123-
cfg.NormalizeCalls = v.GetBool(cfg.namespace + suffixNormalizeCalls)
124-
cfg.NormalizeDuration = v.GetBool(cfg.namespace + suffixNormalizeDuration)
125-
cfg.TokenOverrideFromContext = v.GetBool(cfg.namespace + suffixOverrideFromContext)
113+
opt.MetricNamespace = v.GetString(prefix + suffixMetricNamespace)
114+
opt.LatencyUnit = v.GetString(prefix + suffixLatencyUnit)
115+
opt.NormalizeCalls = v.GetBool(prefix + suffixNormalizeCalls)
116+
opt.NormalizeDuration = v.GetBool(prefix + suffixNormalizeDuration)
117+
opt.TokenOverrideFromContext = v.GetBool(prefix + suffixOverrideFromContext)
126118

127119
isValidUnit := map[string]bool{"ms": true, "s": true}
128-
if _, ok := isValidUnit[cfg.LatencyUnit]; !ok {
129-
return fmt.Errorf(`duration-unit must be one of "ms" or "s", not %q`, cfg.LatencyUnit)
120+
if _, ok := isValidUnit[opt.LatencyUnit]; !ok {
121+
return fmt.Errorf(`duration-unit must be one of "ms" or "s", not %q`, opt.LatencyUnit)
130122
}
131123

132124
var err error
133-
cfg.TLS, err = cfg.getTLSFlagsConfig().InitFromViper(v)
125+
opt.TLS, err = opt.getTLSFlagsConfig().InitFromViper(v)
134126
if err != nil {
135127
return fmt.Errorf("failed to process Prometheus TLS options: %w", err)
136128
}
137129
return nil
138130
}
139131

140-
func (config *namespaceConfig) getTLSFlagsConfig() tlscfg.ClientFlagsConfig {
132+
func (*Options) getTLSFlagsConfig() tlscfg.ClientFlagsConfig {
141133
return tlscfg.ClientFlagsConfig{
142-
Prefix: config.namespace,
134+
Prefix: prefix,
143135
}
144136
}
145137

0 commit comments

Comments
 (0)