Github user justinleet commented on the issue:
https://github.com/apache/metron/pull/579
@ctramnitz Just noticed this, but could you change the name of the PR to
match the Jira?
i.e.
> METRON-941 native PaloAlto parser corrupts message when having a comma in
the payload
Github user ottobackwards commented on the issue:
https://github.com/apache/metron/pull/579
after that I'll merge
---
Github user ottobackwards commented on the issue:
https://github.com/apache/metron/pull/579
@ctramnitz one thing, since this is a regression, technically, we need to
update the release notes / upgrade guide.
Can you add a note to the Upgrading.md about the removal of the Syslog
Github user justinleet commented on the issue:
https://github.com/apache/metron/pull/579
I'm +1, this is definitely a major improvement and I agree, getting it
merged would be great. @simonellistonball Any comment as @ottobackwards asked,
or are we good to pull this in?
---
Github user simonellistonball commented on the issue:
https://github.com/apache/metron/pull/579
I don't believe I have any further things to add. I'm +1 and keen to see
this get in.
---
Github user ctramnitz commented on the issue:
https://github.com/apache/metron/pull/579
Any further comments? The old parser was plain broken. So even if there is
room for improvement here, I would really like to see this merged.
Thanks!
---
Github user ottobackwards commented on the issue:
https://github.com/apache/metron/pull/579
Im +1 on this. I would like to get comment from @simonellistonball et al
on the change for syslog
---
Github user ctramnitz commented on the issue:
https://github.com/apache/metron/pull/579
8.0 log format is also working now
In the latest two commits I included the changed tests from @justinleet but
changed the expected input from full syslog messages including syslog header
Github user ctramnitz commented on the issue:
https://github.com/apache/metron/pull/579
For the syslog header vs message parser this future_use is not really
important. This has other implications than just carrying arbitrary data in a
field that is labeled to do something else.
Github user justinleet commented on the issue:
https://github.com/apache/metron/pull/579
from @ctramnitz on the PR I made against his branch.
> However, I'm not sure the result for is really as expected.
> It shouldn't be "<11>Jan 5 05:38:59 PAN1.exampleCustomer.com 1",
Github user ctramnitz commented on the issue:
https://github.com/apache/metron/pull/579
I think
https://github.com/apache/metron/pull/579/commits/ccd99dda3c8a72408ae13eeaca078af1e345a36c#diff-e0385f97ebea64bab3a83bceef70bb4aR67
Github user ottobackwards commented on the issue:
https://github.com/apache/metron/pull/579
@ctramnitz thank you! Let us know where you are at and if we can help
---
Github user ctramnitz commented on the issue:
https://github.com/apache/metron/pull/579
Rebasing to master to see where we are. If this comes back clean please
don't merge yet, I want to add 8.0 log format first.
---
Github user ottobackwards commented on the issue:
https://github.com/apache/metron/pull/579
@ctramnitz is there any update on this PR? Is there something we can do to
help?
---
Github user james-sirota commented on the issue:
https://github.com/apache/metron/pull/579
Hi we would like to get this into the next release. @ctramnitz we'll be
happy to help you fix it
---
Github user justinleet commented on the issue:
https://github.com/apache/metron/pull/579
Just the start. I got the previously existing test cases in a better
shape, and @ctramnitz should be able to add on pretty easily.
I do want to echo what @mattf-horton said about the
Github user mattf-horton commented on the issue:
https://github.com/apache/metron/pull/579
First, a note to @ctramnitz : Christian, please be patient with the long
cycle on this. It is not a reflection on your work, but rather a desire to
make sure that work gets appropriate
Github user justinleet commented on the issue:
https://github.com/apache/metron/pull/579
https://github.com/ctramnitz/metron/pull/1 opened, if anyone cares. As
noted,
https://github.com/ctramnitz/metron/commit/bf8ce62cb5ed3846f288e4a7a606410ebbaf9d30
is the relevant commit.
---
Github user justinleet commented on the issue:
https://github.com/apache/metron/pull/579
I lied, it's a bit circular. It depends on your PR's fixes, so let's go
back to the original strategy of making a PR against your branch. I'm more
than happy to give up credit, since this is
Github user justinleet commented on the issue:
https://github.com/apache/metron/pull/579
Actually, I'll just open a separate ticket/PR. You'll still want to merge
it in.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well.
Github user justinleet commented on the issue:
https://github.com/apache/metron/pull/579
I just kicked up https://github.com/apache/metron/pull/612, which
theoretically fixes the unit tests (and simplifies them a lot). Straight
replacing the logs would work, but we might want
Github user kylerichardson commented on the issue:
https://github.com/apache/metron/pull/579
Given these unit tests have been broken for quite some time and @ctramnitz
has thoroughly tested in his environment and that this is a relatively minor
change, I'm okay to move it forward
Github user ctramnitz commented on the issue:
https://github.com/apache/metron/pull/579
I'm running this myself and actively parsing PaloAlto logs with this change
for about 3 weeks. You can also see that the changes are fairly trivial and
that structure and naming follows the
Github user mattf-horton commented on the issue:
https://github.com/apache/metron/pull/579
Also, @ctramnitz , could you please describe in more detail the manual
testing you did, which might make us feel better about a commit without unit
test? Thanks.
---
If your project is set
Github user kylerichardson commented on the issue:
https://github.com/apache/metron/pull/579
Thanks for the contribution! Would you mind having a look at the unit tests
for this parser and updating them?
From a quick glance, the unit test appears to be defunct. It's using
Github user ctramnitz commented on the issue:
https://github.com/apache/metron/pull/579
I updated the mapping to use the Metron name right away and added some more
handling for different versions of the log format. Any other feedback?
---
If your project is set up for it, you can
26 matches
Mail list logo
