+1 On Wed, Jan 4, 2017 at 8:04 AM, Justin Leet <[email protected]> wrote:
> In the short term, we could just generate the timestamp appropriately with > the current year in the test for the test and spin off another JIRA for > actually addressing the question of what we do with this data (Keep in mind > we can eventually have replay use cases, so assuming the past year might > not be totally sufficient either.) > > At that point it'll at least be year agnostic, but probably not the actual > output we want. Normally, I'd rather it be handled correctly, but given > that our builds fail, I'd rather have something less broken until we get a > more correct solution. > > I can take care of doing that today. Any objections to that solution? > > Justin > > On Wed, Jan 4, 2017 at 9:34 AM, Kyle Richardson <[email protected] > > > wrote: > > > Unfortunately, it's not going to be quite as simple as just adding the > year > > into the test strings, at least for GrokWebSphereParserTest. (For > > BasicAsaParserTest, updating the test string worked just fine.) > > > > It turns out that that grok pattern being used only expects the month and > > day in the timestamp of the syslog messages. I'm happy to a take a stab > and > > making it year safe by reusing some of the code from the BasicAsaParser; > > however, I have limited time today and it will likely take me until > Friday > > to get a PR submitted given the new scope of changes required. > > > > -Kyle > > > > On Wed, Jan 4, 2017 at 12:50 AM, Matt Foley <[email protected]> wrote: > > > > > Yes, this is an endemic problem with log processing. And I agree > adding > > > the year to the testString is the best fix for our short-term problem. > > > > > > For future consideration, we should consider if there should be an > > > assumption/preference in the parser that the logs are in the “past”. > > > Granted, if the timezone is also unspecified, there is still a 24 hr > > period > > > of uncertainty, but it does seem that on Jan 3 2017 the preferred > > > interpretation of “Apr 15” would be Apr 15 2016, not 2017. > > > > > > Cheers, > > > --Matt > > > > > > On 1/3/17, 5:14 PM, "Michael Miklavcic" <[email protected]> > > > wrote: > > > > > > I also introduced a Clock object and testing mechanism back in > > > METRON-235 - > > > https://github.com/apache/incubator-metron/pull/156 > > > Sample test utilizing the Clock object here - > > > https://github.com/apache/incubator-metron/blob/master/ > > > metron-platform/metron-pcap-backend/src/test/java/org/ > > > apache/metron/pcap/query/PcapCliTest.java > > > > > > That being said, it's probably better to use the new java.time > fixed > > > clock > > > implementation in all places, as referenced by Matt. I'm agreed > with > > > everyone on a quick fix for the build and a follow-on PR to > introduce > > > appropriate dep injection for testing. > > > > > > AFA string dates with no year, we had something similar show up in > > the > > > Snort parser. There ended up being a configuration option in Snort > to > > > enable a year to be printed, but we may want to offer alternatives > > for > > > other parsers. Regardless of how we approach this it gets messy > when > > > you > > > start thinking about potentially different src/dest timezones > across > > a > > > new > > > year boundary in addition to data replay. I would urge our main > goal > > > here > > > to be idempotency. > > > > > > Best, > > > Mike > > > > > > On Tue, Jan 3, 2017 at 5:05 PM, Kyle Richardson < > > > [email protected]> > > > wrote: > > > > > > > Agreed. I prefer the quick win to get us back to successful > builds. > > > > > > > > I do think it's worth a general discussion around how we want to > > > handle > > > > the parsing of string dates with no year. In the long run, Matt's > > > > suggestion of incorporating the Clock object is probably the > route > > > to go; > > > > albeit as a separate enhancement PR. > > > > > > > > I'll start a new discuss thread for that and submit a PR for the > > > quick fix. > > > > > > > > -Kyle > > > > > > > > > On Jan 3, 2017, at 5:20 PM, David Lyle <[email protected]> > > > wrote: > > > > > > > > > > I'm not sure I'm an owner, but I have an opinion. :) > > > > > > > > > > I'd just add "2016". Easy and targeted. > > > > > > > > > > -D... > > > > > > > > > > > > > > >> On Tue, Jan 3, 2017 at 5:08 PM, Matt Foley <[email protected]> > > > wrote: > > > > >> > > > > >> I’ll subordinate this to METRON-647 since it was evidently > filed > > > while I > > > > >> was writing METRON-648 (I did check before!) > > > > >> > > > > >> The question below remains valid, however… > > > > >> > > > > >> > > > > >> On 1/3/17, 1:59 PM, "Matt Foley" <[email protected]> wrote: > > > > >> > > > > >> Hi all, > > > > >> As described in https://issues.apache.org/ > > > jira/browse/METRON-648 , > > > > >> these two test modules are not year-safe, and are suddenly (as > > of > > > 2017) > > > > >> giving false Travis errors. > > > > >> > > > > >> I can fix it quickly, but a question for the “owners” of > > > GrokParser: > > > > >> Do you have an opinion as to whether the fix should be done by > > > adding > > > > >> "2016" to the testString values in the GrokWebSphereParserTest > > > test > > > > module > > > > >> (easy, and only affects the test module), vs making GrokParser > > > use a > > > > Clock > > > > >> object set to 2016 (more involved, and affecting core code, > but > > > allowing > > > > >> for more interesting testing)? > > > > >> > > > > >> For those interested, BasicAsaParserTest:: > > testShortTimestamp() > > > > >> illustrates the use of Clock object in the Asa Parser and its > > test > > > > module. > > > > >> > > > > >> Thanks, > > > > >> --Matt > > > > >> > > > > >> > > > > >> > > > > >> > > > > >> > > > > >> > > > > >> > > > > > > > > > > > > > > > > > > >
