metron-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Justin Leet <justinjl...@gmail.com>
Subject Re: METRON-648 GrokWebSphereParserTest and BasicAsaParserTest are not 2017-safe
Date Wed, 04 Jan 2017 17:45:34 GMT
This should be taken care of now that https://github.com/
apache/incubator-metron/pull/412 is merged into Apache master.

The follow-on Jira for handling dates without years as seen in the
GrokWebSphereParser is at https://issues.apache.org/jira/browse/METRON-649.

Justin

On Wed, Jan 4, 2017 at 11:31 AM, Kyle Richardson <kylerichardson2@gmail.com>
wrote:

> +1 Why didn't I think of that? :) Thanks, Justin.
>
> -Kyle
>
> On Wed, Jan 4, 2017 at 10:26 AM, Nick Allen <nick@nickallen.org> 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 <
> > michael.miklavcic@gmail.com> wrote:
> >
> > > +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
> > > > > >     > >>
> > > > > >     > >>
> > > > > >     > >>
> > > > > >     > >>
> > > > > >     > >>
> > > > > >     > >>
> > > > > >     > >>
> > > > > >     >
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
> >
> >
> > --
> > Nick Allen <nick@nickallen.org>
> >
>

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