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 15:04:10 GMT
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