Fix tags type to []interface{} in event (#5389)

[liketic]

Closes #5389 @urso Could you please review this? 😄

[elasticmachine]

Can one of the admins verify this patch?

[liketic]

Maybe we should support both []string and []interface{} like:

       if tagsIfc, ok := ms[TagsKey]; ok {
		switch tagsIfc.(type) {
		case []interface{}:
			existingTags := tagsIfc.([]interface{})
			for _, tag := range tags {
				existingTags = append(existingTags, tag)
			}
			ms[TagsKey] = existingTags
		case []string:
			existingTags := tagsIfc.([]string)
			existingTags = append(existingTags, tags...)
			ms[TagsKey] = existingTags
		default:
			return errors.Errorf("expected string array by type is %T", tagsIfc)
		}
	} else {
		ms[TagsKey] = tags
	}

[ruflin]

@liketic Thanks for the fix. This could also use a CHANGELOG entry. The code review I let to @urso .

[urso]

just changing []string to []interface{} unfortunately won't be enough. We need to support both, as the internal tags from config files are correctly parsed as []string.

[urso]

The type-switch use-case from your comment is definitely better. With some cleanup it becomes:

  T, exists := ms[TagsKey]
  if !exists {
    ms[TagsKey] = tags
    return nil
  }

  switch arr := T.(type) {
    case []string:
      ms[TagsKey] = append(arr, tags...)
    case []interface:
      for _, tag := range tags {
        arr = append(arr, tag)
      }
      ms[TagsKey] = arr
    default:
      return errors....
  }

  return nil

In addition one can change this event normalization/parsing code to also handle the tags keyword. But once AddTags supports []string and []interface{} it should be fine.

Please add a Changlog entry when you are done.

As this fixes an issue with filebeat, having a filebeat system test would be nice to have. Check for tests/system/test_json.py.

[liketic]

@urso I updated the pr, please review again. Thanks!

[liketic]

Sorry I didn't notice your comments, will update again.

[liketic]

Hi @urso , I didn't find a way to run the system test system/test_json.py, could you please provide some help? 😃

[urso]

system tests are run via nosetest. You can setup the python environment from within the filebeat directory via

$ make python-env
$ source build/python-env/bin/active

head over to tests/system and run the tests via nosetest test_json.py.

[urso]

Not sure, but the linter might complain about a newline at the end of the file.

[urso]

👍

[urso]

PR looks good to me. Thanks for taking the time!!! Also the filebeat system test are much appreciated!

Unfortunately there is a conflict in the changelog file (github sometimes has problem with the changelog). Please rebase and check git not introducing any more errors (e.g. wrong changelog entries). This should also restart the tests, which have not been executed at all :(

[liketic]

@urso I have to say I didn't get it passed on my mac... Because I always got a error file not found seems like the root cause is the file filebeat.test not generated. It's too hard to do the test without a guide. And also, seems like the permission of a lot of files is root user access only. Is that expected behaviour? Is the testing failure caused by my changes? Thanks very much!

[urso]

I see. you can build filebeat.test via make filebeat.test.

No idea why files are owned by root user. Did you run via docker? Try sudo make clean.

[urso]

I checked CI and your tests did pass. It's some flaky kafka tests in metricbeat that have failed.

Will merge PR. Thanks for taking to time fix the issue.