config: Handle null attributes as invalid

[mikeblum]

Fixes #6629

prior art: https://github.com/open-telemetry/opentelemetry-go-contrib/pull/6425/files

[mikeblum]

As written the omitempty is allowing the YAML to parse without an error. Added a validate_config test to factor out the yaml:omitempty issue. Should we remove the omitempty annotations to catch these sorts of issues and check for null / empty after yaml.parse or is there another way?

I could see the matrix of property checks growing quite a bit as new properties get added over time - we'd need a check for each attribute or go down the reflect route which has it's own set of issues.

https://github.com/open-telemetry/opentelemetry-go-contrib/blob/main/config/v0.3.0/generated_config.go#L427

[pellared]

@mikeblum, I think that you need to implement func (j *AttributeNameValue) UnmarshalYAML(unmarshal func(interface{}) error) error and func (j *AttributeNameValue) MarshalJSON() ([]byte, error) methods. Look at config_json.go and config_yaml.go for examples.

[mikeblum]

attributes:
  -

is never reaching (j *AttributeNameValue) UnmarshalYAML but this diff trips the missing name value.

[mikeblum]

thinking out loud I would expect something like this to get hit to mimic UnmarshalJSON

func (j *AttributeNameValue) UnmarshalYAML(b []byte) error {
	var raw map[string]interface{}
	if err := yaml.Unmarshal(b, &raw); err != nil {
		return err
	}
	var name string
	var ok bool
	if _, ok = raw["name"]; raw != nil && !ok {
		return errors.New("field name in AttributeNameValue: required")
	}
	if name, ok = raw["name"].(string); !ok {
		return errors.New("yaml: cannot unmarshal field name in AttributeNameValue must be string")
	}
	if strings.TrimSpace(name) == "" {
		return errors.New("field name in AttributeNameValue: required")
	}
	if _, ok := raw["value"]; raw != nil && !ok {
		return errors.New("field value in AttributeNameValue: required")
	}
	type Plain AttributeNameValue
	var plain Plain
	if err := json.Unmarshal(b, &plain); err != nil {
		return err
	}
	if plain.Type != nil && plain.Type.Value == "int" {
		val, ok := plain.Value.(float64)
		if ok {
			plain.Value = int(val)
		}
	}
	if plain.Type != nil && plain.Type.Value == "int_array" {
		m, ok := plain.Value.([]interface{})
		if ok {
			var vals []interface{}
			for _, v := range m {
				val, ok := v.(float64)
				if ok {
					vals = append(vals, int(val))
				} else {
					vals = append(vals, val)
				}
			}
			plain.Value = vals
		}
	}

	*j = AttributeNameValue(plain)
	return nil
}

[mikeblum]

this is breaking the existing tests:

          	            	     Name: (string) (len=16) "double_array_key",
            	            	-    Type: (*config.AttributeNameValueType)({
            	            	-     Value: (string) (len=12) "double_array"
            	            	-    }),
            	            	+    Type: (*config.AttributeNameValueType)(<nil>),
            	            	     Value: ([]interface {}) (len=2) {
            	Test:       	TestParseYAML/valid_v0.3_config

how should the type get resolved here?

[mikeblum]

this is just to demo surfacing an error on an empty resource attributes list. This in effect makes resource attributes required so I don't think this will work.