add modification_time field to filestat input plugin

[puckpuck]

Required for all PRs:

• Signed CLA.
• Associated README.md updated.
• Has appropriate unit tests.

Adds a modification_time field to filestat. Fixes #2116

I'm not particularly fond of the way unit tests has to work for this. Because the modification time will change based on when you fetch from git, there is no easy way to unit test this. My solution was to create a function which is leveraged by each unit test to set the expected modification time to that of the actual file. If this approach isn't acceptable, I'm happy to change it to something else.

[danielnelson]

Sounds like we need to add an interface and mock globpath.

I also wonder if we should use a different format for the time, other options I can think of are unixnano 1507152973123456789i or as a decimal float 1507152973.123456789. It is just a file so precision is likely not that important, so maybe the current unix time is fine. @desa What do you think?

[phemmer]

What about providing precision greater than 1s?

[danielnelson]

@phemmer Do you think we should use UnixNano format here? Floats are easier to read IMO but not as accurate?

[phemmer]

That would make sense to me (using UnixNano()).

[danielnelson]

Let's make that change too then @puckpuck

[puckpuck]

updated to use UnixNano() ... let me know if you need anything else

[danielnelson]

Just use acc.HasInt64Field( to assert that the field is set, then you can remove the getModificationTime function. This will be good enough for now in my opinion.

[puckpuck]

I was hoping you wouldn't ask for something like that 😉

means I need to modify each test case to assert all fields individually, because acc.AssertContainsTaggedFields(...) will fail now. Hence why I did the getModificationTime function... required fewer changes to implement.

[puckpuck]

Updated to test each field individually and modification_time is an int64 if the file exists. I think this looks kinda ugly, happy to revert it back, change something else, or leave it as is.

[danielnelson]

It is too bad from a naming perspective that AssertContainsTaggedFields requires the fields to be exactly equal, I might rename this function sometime soon.

In the meantime I added HasPoint and converted filestat_test.go, sorry about all the conflicts but I think this will allow the tests to be reasonable.

[puckpuck]

This is helping, but still have the issue that I don't have a way to test an accumulator that it contains an int64 field, for a specific set of tags.

Given this test set:

		dir + "log1.log",
		dir + "log2.log",
		"/non/existant/file",
	}

	acc := testutil.Accumulator{}
	acc.GatherError(fs.Gather)

The first 2 files should have a modification_time but the 3rd should not, however acc.HasInt64Field("filestat", "modification_time") will always return true because at least 1 of the 3 metrics have it. I almost need a HasInt64FieldForTags function

Not wanting to make this become such a monumental task for testing, I decided to break it out into 2 more unit test. Each will test for a single file only, first one should have the field, the second should not.