[GitHub] metron-bro-plugin-kafka issue #6: Configurable JSON timestamps and default a...

[nickwallen]

Github user nickwallen commented on the issue:

https://github.com/apache/metron-bro-plugin-kafka/pull/6
  
@dcode 

1. The JIRA created for this is 
https://issues.apache.org/jira/browse/METRON-1469.

1. Please change the PR title to "METRON-1469: Kafka Plugin for Bro - 
Configurable JSON Timestamps".

1. Please update your PR description to remove references to "send all 
logs".  I just want to avoid any future confusion.


---

[GitHub] metron-bro-plugin-kafka issue #6: Configurable JSON timestamps and default a...

[dcode]

Github user dcode commented on the issue:

https://github.com/apache/metron-bro-plugin-kafka/pull/6
  
That'd be great if you wouldn't mind to create a ticket for this.


---

[GitHub] metron-bro-plugin-kafka issue #6: Configurable JSON timestamps and default a...

[JonZeolla]

Github user JonZeolla commented on the issue:

https://github.com/apache/metron-bro-plugin-kafka/pull/6
  
It's a part of the `apache/metron` project (of which this is considered a 
component) and uses the open apache JIRA that I linked above.  In order to 
accept PRs we need to have a JIRA.  You should be able to register and submit 
something rather simply, but I also wouldn't mind handling this if you'd 
prefer, just let me know.


---

[GitHub] metron-bro-plugin-kafka issue #6: Configurable JSON timestamps and default a...

[dcode]

Github user dcode commented on the issue:

https://github.com/apache/metron-bro-plugin-kafka/pull/6
  
I haven't created a JIRA ticket. Not sure if that's something internal.


---

[GitHub] metron-bro-plugin-kafka issue #6: Configurable JSON timestamps and default a...

[JonZeolla]

Github user JonZeolla commented on the issue:

https://github.com/apache/metron-bro-plugin-kafka/pull/6
  
This is really coming together.  Is there a 
[JIRA](https://issues.apache.org/jira/browse/METRON-1325?filter=-5=project%20%3D%20METRON%20AND%20resolution%20%3D%20Unresolved%20order%20by%20priority%20DESC%2Cupdated%20DESC)
 for this?  I poked around for a bit and couldn't find one.  


---

[GitHub] metron-bro-plugin-kafka issue #6: Configurable JSON timestamps and default a...

[JonZeolla]

Github user JonZeolla commented on the issue:

https://github.com/apache/metron-bro-plugin-kafka/pull/6
  
Okay great, thanks.  I will add to my todo list this week to get this and 
#5 tested and in.  Then we can look at merging #2 and maybe the bool option to 
send all logs.


---

[GitHub] metron-bro-plugin-kafka issue #6: Configurable JSON timestamps and default a...

[dcode]

Github user dcode commented on the issue:

https://github.com/apache/metron-bro-plugin-kafka/pull/6
  
Yes I can fix that

On Feb 24, 2018 13:51, "JonZeolla"  wrote:

> Would you mind removing the send all logs by default portion of this? I
> would love to get this reviewed and in but I feel like that approach could
> be problematic and is better addressed via #2
> . That said,
> please feel free to disagree.
>
> Another approach I plan to take (and have a branch somewhere for, just no
> PR yet) is adding a bool to turn "all" logs (except for reporter) on by
> default.
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> 
,
> or mute the thread
> 

> .
>



---

[GitHub] metron-bro-plugin-kafka issue #6: Configurable JSON timestamps and default a...

[dcode]

Github user dcode commented on the issue:

https://github.com/apache/metron-bro-plugin-kafka/pull/6
  
Oh I wasn't trying to throw accusations. I actually contributed that code
to the bro plugins repo and a couple other forks. I can remove the default
on behavior. I this patch in RockNSM and haven't experienced an Error loop.

On Feb 24, 2018 13:14, "JonZeolla"  wrote:

Thanks @dcode  I'll need to review this more
specifically but I don't recall using any of your contributions for my
other work. I modeled it after other plugins in the old bro-plugins repo.

We have also discussed the default on vs not before and I think we settled
on default off. However, even if you wanted to do something default on you
should exclude reporter.log otherwise you can get in an infinite loop with
errors.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub

,
or mute the thread


.



---

[GitHub] metron-bro-plugin-kafka issue #6: Configurable JSON timestamps and default a...

[JonZeolla]

Github user JonZeolla commented on the issue:

https://github.com/apache/metron-bro-plugin-kafka/pull/6
  
Would you mind removing the send all logs by default portion of this?  I 
would love to get this reviewed and in but I feel like that approach could be 
problematic and is better addressed via #2.  That said, please feel free to 
disagree.

Another approach I plan to take (and have a branch somewhere for, just no 
PR yet) is adding a bool to turn "all" logs (except for reporter) on by default.


---

[GitHub] metron-bro-plugin-kafka issue #6: Configurable JSON timestamps and default a...

[JonZeolla]

Github user JonZeolla commented on the issue:

https://github.com/apache/metron-bro-plugin-kafka/pull/6
  
Thanks @dcode I'll need to review this more specifically but I don't recall 
using any of your contributions for my other work.  I modeled it after other 
plugins in the old bro-plugins repo.

We have also discussed the default on vs not before and I think we settled 
on default off.  However, even if you wanted to do something default on you 
should exclude reporter.log otherwise you can get in an infinite loop with 
errors.


---

[GitHub] metron-bro-plugin-kafka issue #6: Configurable JSON timestamps and default a...

[dcode]

Github user dcode commented on the issue:

https://github.com/apache/metron-bro-plugin-kafka/pull/6
  
I know #2 has some of my original code of two separate sets (I previously 
PR'd this against @JonZeolla's branch), one to include and one to exclude. I 
think that's fine too, but I've found in practice this is easier for most 
use-cases, and the predicate approach is more appropriate to filter out 
specific logs.


---