[jaeger-v2] Standard way to validate configuration

[yurishkuro]

Related to this thread: #5152 (comment)

For example, a field may have an annotation like this:

• mapstructure:"server_urls" validate:"required,min=1,dive,url"

How do we know that the syntax of those tags is valid?

[mahadzaryab1]

@yurishkuro It looks like our govalidator should this pick up. See the following tests:

No Error

func TestValidate(t *testing.T) {
	type TestConfiguration struct {
		Servers []string `mapstructure:"server_urls" valid:"required,url"`
	}
	config := TestConfiguration{
		Servers: []string{"localhost:8000/dummyserver"},
	}
	_, err := govalidator.ValidateStruct(config)
	require.NoError(t, err)
}

Misspelling Error

func TestValidate(t *testing.T) {
	type TestConfiguration struct {
		Servers []string `mapstructure:"server_urls" valid:"notatag,url"`
	}
	config := TestConfiguration{
		Servers: []string{"localhost:8000/dummyserver"},
	}
	require.NoError(t, err)
}

This test fails with

        	Error:      	Received unexpected error:
        	            	Servers: The following validator is invalid or can't be applied to the field: "notatag"

[mahadzaryab1]

Are we instead looking to validate tags like mapstructure and valid are defined correctly?

[yurishkuro]

Yes, the issue is about ensuring that the annotations are correct. It's possible that this comes out of the box when we invoke the corresponding utility - it would parse tags and hopefully complain about invalid syntax. But what if it doesn't?

[mahadzaryab1]

@yurishkuro Based on the unit tests above, it looks like anything defined under the valid tag will be validated for correct syntax. Is there something else that we're looking to accomplish here?

[yurishkuro]

in the PR where the comment was from the tag was validate, not valid, why?

I think at minimum we should have a unit test that checks that an invalid config values are properly reported by the validation function. Not the various permutations of fields, just the fact that the validation is seeing the right tags

[mahadzaryab1]

Ah yes - in that case nothing was reported by the govalidator because it only validates when the valid tag is present. One way that we could get around this is by using

govalidator.SetFieldsRequiredByDefault(true)

This will cause the validator to fail if the valid field is not present on any of the struct fields of the struct that its validating. What do you think?

[yurishkuro]

sounds good, although please double check that it's not actually making the struct fields themselves required in the configuration

[mahadzaryab1]

@yurishkuro Tried this out and unfortunately this won't work because we have some OTEL components embedded which do not have the valid tag so the validation will always fail. The govalidator should catch syntax errors for anything defined under valid. For example, the following would fail because thisisnotatag isn't defined.

TraceStorage string                     `mapstructure:"trace_storage" valid:"thisisnotatag"`

However, if our goal is to catch when we misspell the tag itself (validate instead of valid), we may need to implement a custom validator to see if all jaeger-defined configurations have that tag present.

[yurishkuro]

We can probably we can probably close this and wait for schema-first configs to be available, at which point the tags would be automatically generated.