Skip to content

Commit

Permalink
removed the changes made to Get method and fixed the sampling strateg…
Browse files Browse the repository at this point in the history
…y code
  • Loading branch information
Shubhanshu Surana committed Nov 2, 2019
1 parent 2e30f18 commit 5804c71
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 36 deletions.
35 changes: 16 additions & 19 deletions pkg/config/sampling/sampling.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,11 @@ func NewConfig(jaeger *v1.Jaeger) *Config {
}

// Get returns a configmap specification for the current instance
func (u *Config) Get(options *[]string) *corev1.ConfigMap {
func (u *Config) Get() *corev1.ConfigMap {
var jsonObject []byte
var err error

if CheckForSamplingConfigFile(u.jaeger, options) {
if CheckForSamplingConfigFile(u.jaeger) {
return nil
}
// Check for empty map
Expand All @@ -51,8 +51,6 @@ func (u *Config) Get(options *[]string) *corev1.ConfigMap {
"sampling": string(jsonObject),
}

fmt.Println(data)

return &corev1.ConfigMap{
TypeMeta: metav1.TypeMeta{
APIVersion: "v1",
Expand Down Expand Up @@ -85,29 +83,28 @@ func (u *Config) Get(options *[]string) *corev1.ConfigMap {

// CheckForSamplingConfigFile will check if there is a config file present
// if there is one it returns true
func CheckForSamplingConfigFile(jaeger *v1.Jaeger, options *[]string) bool {
for _, option := range *options {
if strings.Contains(option, "sampling.strategies-file") {
jaeger.Logger().Warn("Sampling strategy file is already passed as an option to collector. Will not be using default sampling strategy")
return true
}
func CheckForSamplingConfigFile(jaeger *v1.Jaeger) bool {
jsonObject, err := jaeger.Spec.Sampling.Options.MarshalJSON()

if err != nil {
return false
}
data := map[string]string{
"sampling": string(jsonObject),
}
if strings.Contains(data["sampling"], "sampling.strategies-file") {
jaeger.Logger().Warn("Sampling strategy file is already passed as an option to collector. Will not be using default sampling strategy")
return true
}

return false
}

// Update will modify the supplied common spec and options to include
// support for the Sampling configmap.
func Update(jaeger *v1.Jaeger, commonSpec *v1.JaegerCommonSpec, options *[]string) {

// // Check to validate if sampling strategy file is already passed as an option
// for _, option := range *options {
// if strings.Contains(option, "sampling.strategies-file") {
// jaeger.Logger().Warn("Sampling strategy file is already passed as an option to collector. Will not be using default sampling strategy")
// return
// }
// }

if CheckForSamplingConfigFile(jaeger, options) {
if CheckForSamplingConfigFile(jaeger) {
return
}

Expand Down
33 changes: 16 additions & 17 deletions pkg/config/sampling/sampling_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,7 @@ func TestNoSamplingConfig(t *testing.T) {
jaeger := v1.NewJaeger(types.NamespacedName{Name: "TestNoSamplingConfig"})

config := NewConfig(jaeger)
options := []string{}
cm := config.Get(&options)
cm := config.Get()
assert.NotNil(t, cm)
assert.Equal(t, defaultSamplingStrategy, cm.Data["sampling"])
}
Expand All @@ -25,8 +24,8 @@ func TestWithEmptySamplingConfig(t *testing.T) {
jaeger.Spec.UI.Options = uiconfig

config := NewConfig(jaeger)
options := []string{}
cm := config.Get(&options)

cm := config.Get()
assert.NotNil(t, cm)
assert.Equal(t, defaultSamplingStrategy, cm.Data["sampling"])
}
Expand All @@ -42,8 +41,7 @@ func TestWithSamplingConfig(t *testing.T) {
jaeger := v1.NewJaeger(types.NamespacedName{Name: "TestWithSamplingConfig"})
jaeger.Spec.Sampling.Options = samplingconfig
config := NewConfig(jaeger)
options := []string{}
cm := config.Get(&options)
cm := config.Get()
assert.Equal(t, json, cm.Data["sampling"])
}

Expand Down Expand Up @@ -85,23 +83,24 @@ func TestUpdateWithSamplingConfig(t *testing.T) {

func TestUpdateWithSamplingConfigFileOption(t *testing.T) {
jaeger := v1.NewJaeger(types.NamespacedName{Name: "TestUpdateWithSamplingConfigFileOption"})
options := []string{
0: "--sampling.strategies-file=/etc/jaeger/sampling.json",
}
jaeger.Spec.Sampling.Options = v1.NewFreeForm(map[string]interface{}{
"sampling.strategies-file": "/etc/jaeger/sampling.json",
})
options := []string{}
commonSpec := v1.JaegerCommonSpec{}
Update(jaeger, &commonSpec, &options)
assert.Len(t, options, 1)
assert.Equal(t, "--sampling.strategies-file=/etc/jaeger/sampling.json", options[0])
assert.Len(t, commonSpec.Volumes, 0)
assert.Len(t, commonSpec.VolumeMounts, 0)
assert.Len(t, options, 0)
}

func TestGetWithSamplingConfigFileOption(t *testing.T) {
jaeger := v1.NewJaeger(types.NamespacedName{Name: "TestGetWithSamplingConfigFileOption"})
options := []string{
0: "--sampling.strategies-file=/etc/jaeger/sampling.json",
}
jaeger.Spec.Sampling.Options = v1.NewFreeForm(map[string]interface{}{
"sampling.strategies-file": "/etc/jaeger/sampling.json",
})

config := NewConfig(jaeger)
cm := config.Get(&options)
assert.Len(t, options, 1)
assert.Equal(t, "--sampling.strategies-file=/etc/jaeger/sampling.json", options[0])
cm := config.Get()
assert.Nil(t, cm)
}

0 comments on commit 5804c71

Please sign in to comment.