METRON-838 Incorrect set of ts in FireEye parser #528
Conversation
|
Hi bjigmp! |
|
Thanks for the contribution! Also, could you please close and reopen the PR? You hit an intermittent test failure and travis should try again. |
|
I created a gist of the template that you can copy and paste. https://gist.github.com/ottobackwards/9b7ea0689a79b9510c52ad49045aea7e |
|
Thanks @ottobackwards, Updated this PR. Are all checkboxes must be checked? |
|
No just the appropriate ones you can see. I don't see the steps to repo or test, although you have them checked as well |
|
Hi, What is the status of this PR? |
|
Hi Otto, sorry for delay. I did not have sample data so I was not able to provide reproducible steps or write unit/integration tests. But recently I found some sampledata for FireEye in metron-platform/metron-parsers/src/test/resources/logData/FireEyeParserTest.txt |
|
Thank you |
|
Wrote test for FireEye and found that it uses ParserUtils.convertToEpoch that returns incorrect value. |
|
@bjigmp When you have a chance, could you deconflict this? |
| JSONObject parsed = parser.parse(fireeyeMessage.getBytes()).get(0); | ||
| JSONParser parser = new JSONParser(); | ||
| Map json = (Map) parser.parse(parsed.toJSONString()); | ||
| long expectedTimestamp = ZonedDateTime.of(Year.now(UTC).getValue(), 3, 19, 5, 24, 39, 0, UTC).toInstant().toEpochMilli(); |
There was a problem hiding this comment.
It's incredibly minor (and optional), but we could just swap out the ZoneId.of("UTC") for ZoneOffset.UTC
At that point, this changes slightly, but still seems reasonable
long expectedTimestamp = ZonedDateTime.of(
Year.now(ZoneOffset.UTC).getValue(),
3,
19,
5,
24,
39,
0,
ZoneOffset.UTC
).toInstant().toEpochMilli();
|
+1 to this. There's a review comment, but quite frankly I consider it optional. If @bjigmp doesn't want to make that change, I'm still good. |
|
Thanks @justinleet. Changed to ZoneOffset.UTC |
Contributor Comments
Although log line is parsed (method getTimeStamp of BasicFireEyeParser class, line 120) and day/month/year are extracted ts is not set to correct value. It set in line 128 but with default null month, day and time
Pull Request Checklist
Thank you for submitting a contribution to Apache Metron (Incubating).
Please refer to our Development Guidelines for the complete guide to follow for contributions.
Please refer also to our Build Verification Guidelines for complete smoke testing guides.
In order to streamline the review of the contribution we ask you follow these guidelines and ask you to double check the following:
For all changes:
For code changes:
Have you included steps to reproduce the behavior or problem that is being changed or addressed?
Have you included steps or a guide to how the change may be verified and tested manually?
Have you ensured that the full suite of tests and checks have been executed in the root incubating-metron folder via:
Have you written or updated unit tests and or integration tests to verify your changes?
If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under ASF 2.0?
Have you verified the basic functionality of the build by building and running locally with Vagrant full-dev environment or the equivalent?
For documentation related changes:
Have you ensured that format looks appropriate for the output in which it is rendered by building and verifying the site-book? If not then run the following commands and the verify changes via
site-book/target/site/index.html:Note:
Please ensure that once the PR is submitted, you check travis-ci for build issues and submit an update to your PR as soon as possible.
It is also recommened that travis-ci is set up for your personal repository such that your branches are built there before submitting a pull request.