+1 Why didn't I think of that? :) Thanks, Justin. -Kyle
On Wed, Jan 4, 2017 at 10:26 AM, Nick Allen <[email protected]> wrote: > +1 We can't merge anything else until we address this. Thanks for > volunteering, Justin. > > On Wed, Jan 4, 2017 at 10:24 AM, Michael Miklavcic < > [email protected]> wrote: > > > +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 > > > > > > >> > > > > > > >> > > > > > > >> > > > > > > >> > > > > > > >> > > > > > > >> > > > > > > >> > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > -- > Nick Allen <[email protected]> >
