metron-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Nick Allen <n...@nickallen.org>
Subject Re: METRON-648 GrokWebSphereParserTest and BasicAsaParserTest are not 2017-safe
Date Wed, 04 Jan 2017 15:26:28 GMT
+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