Skip to content

Commit

Permalink
Merge branch 'main' into fix-sampling-config
Browse files Browse the repository at this point in the history
Signed-off-by: Yuri Shkuro <[email protected]>
  • Loading branch information
yurishkuro authored Dec 30, 2024
2 parents 0b94890 + 752e8d2 commit bb1e6b0
Show file tree
Hide file tree
Showing 5 changed files with 18 additions and 2 deletions.
1 change: 1 addition & 0 deletions cmd/jaeger/internal/all-in-one.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ extensions:
# We can either use file or adaptive sampling strategy in remote_sampling
file:
path:
reload_interval: 1s
# adaptive:
# sampling_store: some_store
# initial_sampling_probability: 0.1
Expand Down
8 changes: 8 additions & 0 deletions cmd/jaeger/internal/extension/remotesampling/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ package remotesampling

import (
"errors"
"time"

"github.com/asaskevich/govalidator"
"go.opentelemetry.io/collector/component"
Expand All @@ -18,6 +19,7 @@ import (
var (
errNoProvider = errors.New("no sampling strategy provider specified, expecting 'adaptive' or 'file'")
errMultipleProviders = errors.New("only one sampling strategy provider can be specified, 'adaptive' or 'file'")
errNegativeInterval = errors.New("reload interval must be a positive value, or zero to disable automatic reloading")
)

var (
Expand All @@ -36,6 +38,8 @@ type Config struct {
type FileConfig struct {
// File specifies a local file as the source of sampling strategies.
Path string `mapstructure:"path"`
// ReloadInterval is the time interval to check and reload sampling strategies file
ReloadInterval time.Duration `mapstructure:"reload_interval"`
}

type AdaptiveConfig struct {
Expand Down Expand Up @@ -86,6 +90,10 @@ func (cfg *Config) Validate() error {
return errMultipleProviders
}

if cfg.File != nil && cfg.File.ReloadInterval < 0 {
return errNegativeInterval
}

_, err := govalidator.ValidateStruct(cfg)
return err
}
9 changes: 8 additions & 1 deletion cmd/jaeger/internal/extension/remotesampling/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,13 @@ func Test_Validate(t *testing.T) {
},
expectedErr: "",
},
{
name: "File provider has negative reload interval",
config: &Config{
File: &FileConfig{Path: "", ReloadInterval: -1},
},
expectedErr: "must be a positive value",
},
{
name: "Invalid Adaptive provider",
config: &Config{
Expand All @@ -66,7 +73,7 @@ func Test_Validate(t *testing.T) {
if tt.expectedErr == "" {
require.NoError(t, err)
} else {
assert.Equal(t, tt.expectedErr, err.Error())
assert.ErrorContains(t, err, tt.expectedErr)
}
})
}
Expand Down
1 change: 1 addition & 0 deletions cmd/jaeger/internal/extension/remotesampling/extension.go
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,7 @@ func (ext *rsExtension) Shutdown(ctx context.Context) error {
func (ext *rsExtension) startFileBasedStrategyProvider(_ context.Context) error {
opts := static.Options{
StrategiesFile: ext.cfg.File.Path,
ReloadInterval: ext.cfg.File.ReloadInterval,
}

// contextcheck linter complains about next line that context is not passed.
Expand Down
1 change: 0 additions & 1 deletion plugin/sampling/strategyprovider/static/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,6 @@ func (h *samplingProvider) samplingStrategyLoader(strategiesFile string) strateg
}

return func() ([]byte, error) {
h.logger.Info("Loading sampling strategies", zap.String("filename", strategiesFile))
currBytes, err := os.ReadFile(filepath.Clean(strategiesFile))
if err != nil {
return nil, fmt.Errorf("failed to read strategies file %s: %w", strategiesFile, err)
Expand Down

0 comments on commit bb1e6b0

Please sign in to comment.