metron-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Michael Miklavcic <michael.miklav...@gmail.com>
Subject Re: METRON-648 GrokWebSphereParserTest and BasicAsaParserTest are not 2017-safe
Date Wed, 04 Jan 2017 15:24:47 GMT
+1

On Wed, Jan 4, 2017 at 8:04 AM, Justin Leet <justinjleet@gmail.com> 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 <kylerichardson2@gmail.com
> >
> 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 <mattf@apache.org> 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" <michael.miklavcic@gmail.com>
> > > 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 <
> > > kylerichardson2@gmail.com>
> > >     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 <dlyle65535@gmail.com>
> > > 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 <mattf@apache.org>
> > > 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" <mattf@apache.org>
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
> > >     > >>
> > >     > >>
> > >     > >>
> > >     > >>
> > >     > >>
> > >     > >>
> > >     > >>
> > >     >
> > >
> > >
> > >
> > >
> >
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message