You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@metron.apache.org by GitBox <gi...@apache.org> on 2020/05/04 16:55:10 UTC
[GitHub] [metron-bro-plugin-kafka] JonZeolla opened a new pull request #44: METRON-2270 Update to reflect bro project rename
JonZeolla opened a new pull request #44:
URL: https://github.com/apache/metron-bro-plugin-kafka/pull/44
## Contributor Comments
This is a breaking change in order to update the plugin to work with more recent versions of zeek (previously bro). There were a few cascading issues that required upgrades, most notably due to [this](https://github.com/zeek/zeek/blob/fda94982769e4216c6b2270f4f276149b1b6d7d1/NEWS#L261-L265) and [this](https://github.com/edenhill/librdkafka/issues/2681).
As of this PR, `zkg.meta` points to a librdkafka 1.4.2-RC2. We may want to wait to merge until 1.4.2 or 1.5.0 are released and update `zkg.meta`.
### Testing
Run `./run_end_to_end.sh`
## Pull Request Checklist
Thank you for submitting a contribution to Apache Metron's Bro kafka writer plugin.
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:
- [X] Is there a JIRA ticket associated with this PR? If not one needs to be created at [Metron Jira](https://issues.apache.org/jira/browse/METRON/?selectedTab=com.atlassian.jira.jira-projects-plugin:summary-panel).
- [X] Does your PR title start with METRON-XXXX where XXXX is the JIRA number you are trying to resolve? Pay particular attention to the hyphen "-" character.
- [X] Has your PR been rebased against the latest commit within the target branch (typically master)?
### For code changes:
- [X] Have you included steps to reproduce the behavior or problem that is being changed or addressed?
- [X] Have you included steps or a guide to how the change may be verified and tested manually?
- [X] Have you ensured that the full suite of tests and checks have been executed via:
```
bro-pkg test $GITHUB_USERNAME/metron-bro-plugin-kafka --version $BRANCH
```
- [ ] 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](http://www.apache.org/legal/resolved.html#category-a)?
- [ ] Have you verified the basic functionality of the build by building and running locally with Apache Metron's [Vagrant full-dev environment](https://github.com/apache/metron/tree/master/metron-deployment/development/centos6) or the equivalent?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [metron-bro-plugin-kafka] ottobackwards commented on pull request #44: METRON-2270 Update to reflect bro project rename
Posted by GitBox <gi...@apache.org>.
ottobackwards commented on pull request #44:
URL: https://github.com/apache/metron-bro-plugin-kafka/pull/44#issuecomment-623586512
We should mention specifically in the PR description that we are updating to zeek version XXXXX. If this is the PR that does it
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [metron-bro-plugin-kafka] JonZeolla commented on a change in pull request #44: METRON-2270 Update to reflect bro project rename
Posted by GitBox <gi...@apache.org>.
JonZeolla commented on a change in pull request #44:
URL: https://github.com/apache/metron-bro-plugin-kafka/pull/44#discussion_r420178988
##########
File path: README.md
##########
@@ -16,7 +16,7 @@ This software is a part of the [Apache Metron](http://metron.apache.org/) projec
`zkg` is the preferred mechanism for installing this plugin, as it will dynamically retrieve, build, test, and load the plugin. Note, that you will still need to [activate](#activation) and configure the plugin after your installation.
-1. Install [librdkafka](https://github.com/edenhill/librdkafka), a native client library for Kafka. This plugin has been tested against the latest release of librdkafka, which at the time of this writing is v1.4.2.
+1. Install [librdkafka](https://github.com/edenhill/librdkafka), a native client library for Kafka. This plugin has been tested against librdkafka v1.4.2.
Review comment:
This is up for debate, I'm good with waiting for the release or running with the RC.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [metron-bro-plugin-kafka] JonZeolla commented on pull request #44: METRON-2270 Update to reflect bro project rename
Posted by GitBox <gi...@apache.org>.
JonZeolla commented on pull request #44:
URL: https://github.com/apache/metron-bro-plugin-kafka/pull/44#issuecomment-624657441
That's just a side effect of our merging process, which doesn't use the github built-ins. This was squash merged as 587e9dac9beeee915f7d0c1116a4432447b5d92e meaning it's in master. At some point probably not too far out I expect to cut a 1.0.0 release
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [metron-bro-plugin-kafka] JonZeolla commented on pull request #44: METRON-2270 Update to reflect bro project rename
Posted by GitBox <gi...@apache.org>.
JonZeolla commented on pull request #44:
URL: https://github.com/apache/metron-bro-plugin-kafka/pull/44#issuecomment-623645487
My concern with writing it in a README is that it would be prone to being incorrect. In `zkg.meta` do we say it requires `zeek >=3.0.0`. I tested against 3.0.5 and 3.1.2. I would rather us pursue a CI process that has a testing matrix for multiple zeek versions, and embed that into the README. What do you think?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [metron-bro-plugin-kafka] ottobackwards commented on a change in pull request #44: METRON-2270 Update to reflect bro project rename
Posted by GitBox <gi...@apache.org>.
ottobackwards commented on a change in pull request #44:
URL: https://github.com/apache/metron-bro-plugin-kafka/pull/44#discussion_r420274006
##########
File path: README.md
##########
@@ -16,7 +16,7 @@ This software is a part of the [Apache Metron](http://metron.apache.org/) projec
`zkg` is the preferred mechanism for installing this plugin, as it will dynamically retrieve, build, test, and load the plugin. Note, that you will still need to [activate](#activation) and configure the plugin after your installation.
-1. Install [librdkafka](https://github.com/edenhill/librdkafka), a native client library for Kafka. This plugin has been tested against the latest release of librdkafka, which at the time of this writing is v1.4.2.
+1. Install [librdkafka](https://github.com/edenhill/librdkafka), a native client library for Kafka. This plugin has been tested against librdkafka v1.4.2.
Review comment:
Ok, I wasn't thinking of this as a question of the release or release process, I'm just thinking that with 'stable master', this needs to be correct (as of this commit )
The question of cutting a release off of master or not is separate.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [metron-bro-plugin-kafka] ottobackwards commented on a change in pull request #44: METRON-2270 Update to reflect bro project rename
Posted by GitBox <gi...@apache.org>.
ottobackwards commented on a change in pull request #44:
URL: https://github.com/apache/metron-bro-plugin-kafka/pull/44#discussion_r420173561
##########
File path: README.md
##########
@@ -16,7 +16,7 @@ This software is a part of the [Apache Metron](http://metron.apache.org/) projec
`zkg` is the preferred mechanism for installing this plugin, as it will dynamically retrieve, build, test, and load the plugin. Note, that you will still need to [activate](#activation) and configure the plugin after your installation.
-1. Install [librdkafka](https://github.com/edenhill/librdkafka), a native client library for Kafka. This plugin has been tested against the latest release of librdkafka, which at the time of this writing is v1.4.2.
+1. Install [librdkafka](https://github.com/edenhill/librdkafka), a native client library for Kafka. This plugin has been tested against librdkafka v1.4.2.
Review comment:
If we are going to commit with the -RC version, this should say the RC version.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [metron-bro-plugin-kafka] ottobackwards commented on a change in pull request #44: METRON-2270 Update to reflect bro project rename
Posted by GitBox <gi...@apache.org>.
ottobackwards commented on a change in pull request #44:
URL: https://github.com/apache/metron-bro-plugin-kafka/pull/44#discussion_r420173786
##########
File path: README.md
##########
@@ -63,11 +63,11 @@ This software is a part of the [Apache Metron](http://metron.apache.org/) projec
### Manual Installation
-Manually installing the plugin should only occur in situations where installing and configuring `zkg` is not reasonable, such as in a docker container. If you are running zeek in an environment where you do not have Internet connectivity, investigate [bundles](https://docs.zeek.org/projects/package-manager/en/stable/zkg.html#bundle) or creating an internal [package source](https://docs.zeek.org/projects/package-manager/en/stable/source.html).
+Manually installing the plugin should *only* occur in situations where installing and configuring `zkg` is not reasonable. If you are running zeek in an environment where you do not have Internet connectivity, investigate [bundles](https://docs.zeek.org/projects/package-manager/en/stable/zkg.html#bundle) or creating an internal [package source](https://docs.zeek.org/projects/package-manager/en/stable/source.html).
These instructions could also be helpful if you were interested in distributing this as a package (such as a deb or rpm).
-1. Install [librdkafka](https://github.com/edenhill/librdkafka), a native client library for Kafka. This plugin has been tested against the latest release of librdkafka, which at the time of this writing is v1.4.2.
+1. Install [librdkafka](https://github.com/edenhill/librdkafka), a native client library for Kafka. This plugin has been tested against librdkafka v1.4.2.
Review comment:
same as above, put in the RC version
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [metron-bro-plugin-kafka] JonZeolla commented on a change in pull request #44: METRON-2270 Update to reflect bro project rename
Posted by GitBox <gi...@apache.org>.
JonZeolla commented on a change in pull request #44:
URL: https://github.com/apache/metron-bro-plugin-kafka/pull/44#discussion_r420359510
##########
File path: README.md
##########
@@ -63,11 +63,11 @@ This software is a part of the [Apache Metron](http://metron.apache.org/) projec
### Manual Installation
-Manually installing the plugin should only occur in situations where installing and configuring `zkg` is not reasonable, such as in a docker container. If you are running zeek in an environment where you do not have Internet connectivity, investigate [bundles](https://docs.zeek.org/projects/package-manager/en/stable/zkg.html#bundle) or creating an internal [package source](https://docs.zeek.org/projects/package-manager/en/stable/source.html).
+Manually installing the plugin should *only* occur in situations where installing and configuring `zkg` is not reasonable. If you are running zeek in an environment where you do not have Internet connectivity, investigate [bundles](https://docs.zeek.org/projects/package-manager/en/stable/zkg.html#bundle) or creating an internal [package source](https://docs.zeek.org/projects/package-manager/en/stable/source.html).
These instructions could also be helpful if you were interested in distributing this as a package (such as a deb or rpm).
-1. Install [librdkafka](https://github.com/edenhill/librdkafka), a native client library for Kafka. This plugin has been tested against the latest release of librdkafka, which at the time of this writing is v1.4.2.
+1. Install [librdkafka](https://github.com/edenhill/librdkafka), a native client library for Kafka. This plugin has been tested against librdkafka v1.4.2.
Review comment:
Update made throughout
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [metron-bro-plugin-kafka] JonZeolla commented on a change in pull request #44: METRON-2270 Update to reflect bro project rename
Posted by GitBox <gi...@apache.org>.
JonZeolla commented on a change in pull request #44:
URL: https://github.com/apache/metron-bro-plugin-kafka/pull/44#discussion_r420092815
##########
File path: README.md
##########
@@ -12,68 +12,68 @@ This software is a part of the [Apache Metron](http://metron.apache.org/) projec
## Installation
Review comment:
README should be accurate as of 761462a, ready for review.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [metron-bro-plugin-kafka] ottobackwards commented on a change in pull request #44: METRON-2270 Update to reflect bro project rename
Posted by GitBox <gi...@apache.org>.
ottobackwards commented on a change in pull request #44:
URL: https://github.com/apache/metron-bro-plugin-kafka/pull/44#discussion_r420304901
##########
File path: README.md
##########
@@ -16,7 +16,7 @@ This software is a part of the [Apache Metron](http://metron.apache.org/) projec
`zkg` is the preferred mechanism for installing this plugin, as it will dynamically retrieve, build, test, and load the plugin. Note, that you will still need to [activate](#activation) and configure the plugin after your installation.
-1. Install [librdkafka](https://github.com/edenhill/librdkafka), a native client library for Kafka. This plugin has been tested against the latest release of librdkafka, which at the time of this writing is v1.4.2.
+1. Install [librdkafka](https://github.com/edenhill/librdkafka), a native client library for Kafka. This plugin has been tested against librdkafka v1.4.2.
Review comment:
Yes, if we don't have a timeframe. This is an in between release build in our terms, and it passes our tests. I don't see why we wait. But maybe I don't understand the arguments?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [metron-bro-plugin-kafka] dcode commented on pull request #44: METRON-2270 Update to reflect bro project rename
Posted by GitBox <gi...@apache.org>.
dcode commented on pull request #44:
URL: https://github.com/apache/metron-bro-plugin-kafka/pull/44#issuecomment-624639843
@JonZeolla thanks for the heads up. @rocknsm does still use this plugin. Looks like this PR was closed without merging though? Is this still happening?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [metron-bro-plugin-kafka] JonZeolla commented on pull request #44: METRON-2270 Update to reflect bro project rename
Posted by GitBox <gi...@apache.org>.
JonZeolla commented on pull request #44:
URL: https://github.com/apache/metron-bro-plugin-kafka/pull/44#issuecomment-623701958
End to end tests are succeeding, but the unit tests appear to be failing so that needs addressed. Converting this PR to a draft since it's still WIP it seems.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [metron-bro-plugin-kafka] JonZeolla commented on pull request #44: METRON-2270 Update to reflect bro project rename
Posted by GitBox <gi...@apache.org>.
JonZeolla commented on pull request #44:
URL: https://github.com/apache/metron-bro-plugin-kafka/pull/44#issuecomment-623587568
Done
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [metron-bro-plugin-kafka] JonZeolla commented on a change in pull request #44: METRON-2270 Update to reflect bro project rename
Posted by GitBox <gi...@apache.org>.
JonZeolla commented on a change in pull request #44:
URL: https://github.com/apache/metron-bro-plugin-kafka/pull/44#discussion_r420364150
##########
File path: README.md
##########
@@ -16,7 +16,7 @@ This software is a part of the [Apache Metron](http://metron.apache.org/) projec
`zkg` is the preferred mechanism for installing this plugin, as it will dynamically retrieve, build, test, and load the plugin. Note, that you will still need to [activate](#activation) and configure the plugin after your installation.
-1. Install [librdkafka](https://github.com/edenhill/librdkafka), a native client library for Kafka. This plugin has been tested against the latest release of librdkafka, which at the time of this writing is v1.4.2.
+1. Install [librdkafka](https://github.com/edenhill/librdkafka), a native client library for Kafka. This plugin has been tested against librdkafka v1.4.2.
Review comment:
Updated documentation to point to RC3 regarding what was tested and `zkg.meta` points to `~1.4.2-RC1` as the required external_depends
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [metron-bro-plugin-kafka] JonZeolla commented on pull request #44: METRON-2270 Update to reflect bro project rename
Posted by GitBox <gi...@apache.org>.
JonZeolla commented on pull request #44:
URL: https://github.com/apache/metron-bro-plugin-kafka/pull/44#issuecomment-624043661
@ottobackwards Unit tests and e2e are both passing now.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [metron-bro-plugin-kafka] JonZeolla commented on a change in pull request #44: METRON-2270 Update to reflect bro project rename
Posted by GitBox <gi...@apache.org>.
JonZeolla commented on a change in pull request #44:
URL: https://github.com/apache/metron-bro-plugin-kafka/pull/44#discussion_r419702207
##########
File path: README.md
##########
@@ -12,68 +12,68 @@ This software is a part of the [Apache Metron](http://metron.apache.org/) projec
## Installation
Review comment:
I think that the installation instructions need an update.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [metron-bro-plugin-kafka] ottobackwards commented on pull request #44: METRON-2270 Update to reflect bro project rename
Posted by GitBox <gi...@apache.org>.
ottobackwards commented on pull request #44:
URL: https://github.com/apache/metron-bro-plugin-kafka/pull/44#issuecomment-624366596
for the love of puppies, merge this already
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [metron-bro-plugin-kafka] JonZeolla commented on a change in pull request #44: METRON-2270 Update to reflect bro project rename
Posted by GitBox <gi...@apache.org>.
JonZeolla commented on a change in pull request #44:
URL: https://github.com/apache/metron-bro-plugin-kafka/pull/44#discussion_r420299628
##########
File path: README.md
##########
@@ -16,7 +16,7 @@ This software is a part of the [Apache Metron](http://metron.apache.org/) projec
`zkg` is the preferred mechanism for installing this plugin, as it will dynamically retrieve, build, test, and load the plugin. Note, that you will still need to [activate](#activation) and configure the plugin after your installation.
-1. Install [librdkafka](https://github.com/edenhill/librdkafka), a native client library for Kafka. This plugin has been tested against the latest release of librdkafka, which at the time of this writing is v1.4.2.
+1. Install [librdkafka](https://github.com/edenhill/librdkafka), a native client library for Kafka. This plugin has been tested against librdkafka v1.4.2.
Review comment:
Right, we are on the same page. I'm saying I'm open to waiting to merge this until 1.4.2 is released and any remnants for RC3 are removed. If we don't do that, I agree the docs should be accurate as of the commit and refer to RC3.
I assume your preference is update the README to reflect the RC and get this merged sooner rather than later, given we don't know what the 1.4.2 timeframe is?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org