Skip to content

Commit

Permalink
[prometheus] Use OTEL helper instead of tlscfg package (#6266)
Browse files Browse the repository at this point in the history
## Which problem is this PR solving?
- Part of #4316 

## Description of the changes
- 

## How was this change tested?
- 

## Checklist
- [ ] I have read
https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md
- [ ] I have signed all commits
- [ ] I have added unit tests for the new functionality
- [ ] I have run lint and test steps successfully
  - for `jaeger`: `make lint test`
  - for `jaeger-ui`: `yarn lint` and `yarn test`

---------

Signed-off-by: chahatsagarmain <[email protected]>
Signed-off-by: chahat sagar <[email protected]>
Co-authored-by: Yuri Shkuro <[email protected]>
  • Loading branch information
chahatsagarmain and yurishkuro authored Nov 28, 2024
1 parent 58e08c5 commit 4a14e87
Show file tree
Hide file tree
Showing 4 changed files with 19 additions and 28 deletions.
5 changes: 2 additions & 3 deletions pkg/prometheus/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,14 @@ import (
"time"

"github.com/asaskevich/govalidator"

"github.com/jaegertracing/jaeger/pkg/config/tlscfg"
"go.opentelemetry.io/collector/config/configtls"
)

// Configuration describes the options to customize the storage behavior.
type Configuration struct {
ServerURL string `valid:"required" mapstructure:"endpoint"`
ConnectTimeout time.Duration `mapstructure:"connect_timeout"`
TLS tlscfg.Options
TLS configtls.ClientConfig

TokenFilePath string `mapstructure:"token_file_path"`
TokenOverrideFromContext bool `mapstructure:"token_override_from_context"`
Expand Down
11 changes: 4 additions & 7 deletions plugin/metrics/prometheus/metricsstore/reader.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ package metricsstore

import (
"context"
"crypto/tls"
"fmt"
"net"
"net/http"
Expand Down Expand Up @@ -315,12 +314,10 @@ func logErrorToSpan(span trace.Span, err error) {
span.SetStatus(codes.Error, err.Error())
}

func getHTTPRoundTripper(c *config.Configuration, logger *zap.Logger) (rt http.RoundTripper, err error) {
var ctlsConfig *tls.Config
if c.TLS.Enabled {
if ctlsConfig, err = c.TLS.Config(logger); err != nil {
return nil, err
}
func getHTTPRoundTripper(c *config.Configuration, _ *zap.Logger) (rt http.RoundTripper, err error) {
ctlsConfig, err := c.TLS.LoadTLSConfig(context.Background())
if err != nil {
return nil, err
}
// KeepAlive and TLSHandshake timeouts are kept to existing Prometheus client's
// DefaultRoundTripper to simplify user configuration and may be made configurable when required.
Expand Down
16 changes: 7 additions & 9 deletions plugin/metrics/prometheus/metricsstore/reader_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,14 @@ import (

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"go.opentelemetry.io/collector/config/configtls"
"go.opentelemetry.io/otel/codes"
sdktrace "go.opentelemetry.io/otel/sdk/trace"
"go.opentelemetry.io/otel/sdk/trace/tracetest"
"go.opentelemetry.io/otel/trace"
"go.uber.org/zap"

"github.com/jaegertracing/jaeger/pkg/bearertoken"
"github.com/jaegertracing/jaeger/pkg/config/tlscfg"
"github.com/jaegertracing/jaeger/pkg/prometheus/config"
"github.com/jaegertracing/jaeger/pkg/testutils"
"github.com/jaegertracing/jaeger/proto-gen/api_v2/metrics"
Expand Down Expand Up @@ -739,13 +739,10 @@ func TestGetRoundTripperTLSConfig(t *testing.T) {
t.Run(tc.name, func(t *testing.T) {
logger := zap.NewNop()
config := &config.Configuration{
ConnectTimeout: 9 * time.Millisecond,
TLS: tlscfg.Options{
Enabled: tc.tlsEnabled,
},
ConnectTimeout: 9 * time.Millisecond,
TLS: configtls.ClientConfig{},
TokenOverrideFromContext: true,
}
defer config.TLS.Close()
rt, err := getHTTPRoundTripper(config, logger)
require.NoError(t, err)

Expand Down Expand Up @@ -856,9 +853,10 @@ func TestInvalidCertFile(t *testing.T) {
reader, err := NewMetricsReader(config.Configuration{
ServerURL: "https://localhost:1234",
ConnectTimeout: defaultTimeout,
TLS: tlscfg.Options{
Enabled: true,
CAPath: "foo",
TLS: configtls.ClientConfig{
Config: configtls.Config{
CAFile: "foo",
},
},
}, logger, tracer)
require.Error(t, err)
Expand Down
15 changes: 6 additions & 9 deletions plugin/metrics/prometheus/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@ type Options struct {
config.Configuration `mapstructure:",squash"`
}

var tlsFlagsCfg = tlscfg.ClientFlagsConfig{Prefix: prefix}

func DefaultConfig() config.Configuration {
return config.Configuration{
ServerURL: defaultServerURL,
Expand All @@ -64,7 +66,7 @@ func NewOptions() *Options {
}

// AddFlags from this storage to the CLI.
func (opt *Options) AddFlags(flagSet *flag.FlagSet) {
func (*Options) AddFlags(flagSet *flag.FlagSet) {
flagSet.String(prefix+suffixServerURL, defaultServerURL,
"The Prometheus server's URL, must include the protocol scheme e.g. http://localhost:9090")
flagSet.Duration(prefix+suffixConnectTimeout, defaultConnectTimeout,
Expand Down Expand Up @@ -92,7 +94,7 @@ func (opt *Options) AddFlags(flagSet *flag.FlagSet) {
`For example: `+
`"duration_bucket" (not normalized) -> "duration_milliseconds_bucket (normalized)"`)

opt.getTLSFlagsConfig().AddFlags(flagSet)
tlsFlagsCfg.AddFlags(flagSet)
}

// InitFromViper initializes the options struct with values from Viper.
Expand All @@ -113,19 +115,14 @@ func (opt *Options) InitFromViper(v *viper.Viper) error {
}

var err error
opt.TLS, err = opt.getTLSFlagsConfig().InitFromViper(v)
tlsOpts, err := tlsFlagsCfg.InitFromViper(v)
if err != nil {
return fmt.Errorf("failed to process Prometheus TLS options: %w", err)
}
opt.TLS = tlsOpts.ToOtelClientConfig()
return nil
}

func (*Options) getTLSFlagsConfig() tlscfg.ClientFlagsConfig {
return tlscfg.ClientFlagsConfig{
Prefix: prefix,
}
}

// stripWhiteSpace removes all whitespace characters from a string.
func stripWhiteSpace(str string) string {
return strings.ReplaceAll(str, " ", "")
Expand Down

0 comments on commit 4a14e87

Please sign in to comment.