You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@metron.apache.org by cestella <gi...@git.apache.org> on 2017/06/06 23:50:26 UTC
[GitHub] metron pull request #609: METRON-987: Allow stellar enrichments to be specif...
GitHub user cestella opened a pull request:
https://github.com/apache/metron/pull/609
METRON-987: Allow stellar enrichments to be specified by a list as well as a map
## Contributor Comments
Currently, stellar enrichments are specified by a map associating fields with the stellar expressions associated with the fields. There is a significant downside here in that you cannot update previously assigned fields. For instance, the following cannot be represented currently:
```
"fieldMap": {
...
"stellar" : {
"config" : {
"hostname" : "if ENDS_WITH(hostname, '.') then CHOP(hostname) else hostname",
"hostname" : "TO_LOWER(hostname)"
}
}
}
```
This would now be allowed thusly:
```
"fieldMap": {
...
"stellar" : {
"config" : [
"hostname := if ENDS_WITH(hostname, '.') then CHOP(hostname) else hostname",
"hostname := TO_LOWER(hostname)"
]
}
}
```
A consequent of this deficiency is that we also cannot use temporary variables and unset them after their use inside an enrichment group.
The proposed change is to allow users to use lists of strings representing stellar expression assignments with the same syntax as the Stellar REPL. This would be as an alternative to maps, but the map syntax would also be supported for legacy.
Test plan to follow in comments.
## Pull Request Checklist
Thank you for submitting a contribution to Apache Metron.
Please refer to our [Development Guidelines](https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=61332235) for the complete guide to follow for contributions.
Please refer also to our [Build Verification Guidelines](https://cwiki.apache.org/confluence/display/METRON/Verifying+Builds?show-miniview) 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:
- [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?
- [ ] 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 in the root incubating-metron folder via:
```
mvn -q clean integration-test install && build_utils/verify_licenses.sh
```
- [x] Have you written or updated unit tests and or integration tests to verify your changes?
- [x] 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 Vagrant full-dev environment or the equivalent?
### For documentation related changes:
- [x] 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`:
```
cd site-book
mvn site
```
#### 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 recommended that [travis-ci](https://travis-ci.org) is set up for your personal repository such that your branches are built there before submitting a pull request.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/cestella/incubator-metron stellar_list_support
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/metron/pull/609.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #609
----
commit e9ea04bc6e9f42e1029d1d7fa6e98fa00e0226fe
Author: cstella <ce...@gmail.com>
Date: 2017-06-06T22:21:49Z
Adding the ability to specify Stellar enrichments as lists as well as Maps.
commit cba07c78d91451eb1ec318418c418269e7127e38
Author: cstella <ce...@gmail.com>
Date: 2017-06-06T23:17:11Z
Documentation update
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] metron pull request #609: METRON-987: Allow stellar enrichments to be specif...
Posted by cestella <gi...@git.apache.org>.
Github user cestella commented on a diff in the pull request:
https://github.com/apache/metron/pull/609#discussion_r120932266
--- Diff: metron-platform/metron-enrichment/README.md ---
@@ -71,40 +73,94 @@ The `fieldMap`contents are of interest because they contain the routing and conf
]
}
```
-Based on this sample config, both ip_src_addr and ip_dst_addr will go to the `geo`, `host`, and `hbaseEnrichment` adapter bolts. For the `geo`, `host` and `hbaseEnrichment`, this is sufficient. However, more complex enrichments may contain their own configuration. Currently, the `stellar` enrichment requires a more complex configuration, such as:
+Based on this sample config, both `ip_src_addr` and `ip_dst_addr` will go to the `geo`, `host`, and
+`hbaseEnrichment` adapter bolts.
+
+#### Stellar Enrichment Configuration
+For the `geo`, `host` and `hbaseEnrichment`, this is sufficient. However, more complex enrichments
+may contain their own configuration. Currently, the `stellar` enrichment is more adaptable and thus
+requires a more nuanced configuration.
+
+At its most basic, we want to take a message and apply a couple of enrichments, such as converting the
+`hostname` field to lowercase. We do this by specifying the transformation inside of the
+`config` for the `stellar` fieldMap. There are two syntaxes that are supported, specifying the transformations
+as a map with the key as the field and the value the stellar expression:
```
"fieldMap": {
...
"stellar" : {
"config" : {
- "numeric" : {
- "foo": "1 + 1"
- }
- ,"ALL_CAPS" : "TO_UPPER(source.type)"
+ "hostname" : "TO_LOWER(hostname)"
}
}
}
```
-Whereas the simpler enrichments just need a set of fields explicitly stated so they can be separated from the message and sent to the enrichment adapter bolt for enrichment and ultimately joined back in the join bolt, the stellar enrichment has its set of required fields implicitly stated through usage. For instance, if your stellar statement references a field, it should be included and if not, then it should not be included. We did not want to require users to make explicit the implicit.
+Another approach is to make the transformations as a list with the same `var := expr` syntax as is used
+in the Stellar REPL, such as:
+```
+ "fieldMap": {
+ ...
+ "stellar" : {
+ "config" : [
+ "hostname := TO_LOWER(hostname)"
+ ]
+ }
+ }
+```
+
+Sometimes arbitrary stellar enrichments may take enough time that you would prefer to split some of them
+into groups and execute the groups of stellar enrichments in parallel. Take, for instance, if you wanted
+to do an HBase enrichment and a profiler call which were independent of one another. This usecase is
+supported by splitting the enrichments up as groups.
-The other way in which the stellar enrichment is somewhat more complex is in how the statements are executed. In the general purpose case for a list of fields, those fields are used to create a message to send to the enrichment adapter bolt and that bolt's worker will handle the fields one by one in serial for a given message. For stellar enrichment, we wanted to have a more complex design so that users could specify the groups of stellar statements sent to the same worker in the same message (and thus executed sequentially). Consider the following configuration:
+Consider the following example:
```
"fieldMap": {
+ ...
"stellar" : {
"config" : {
- "numeric" : {
- "foo": "1 + 1"
- "bar" : TO_LOWER(source.type)"
- }
- ,"text" : {
- "ALL_CAPS" : "TO_UPPER(source.type)"
- }
+ "malicious_domain_enrichment" : {
+ "is_bad_domain" : "ENRICHMENT_EXISTS('malicious_domains', ip_dst_addr, 'enrichments', 'cf')"
+ },
+ "login_profile" : [
+ "profile_window := PROFILE_WINDOW('from 6 months ago')",
+ "global_login_profile := PROFILE_GET('distinct_login_attempts', 'global', profile_window)",
+ "stats := STATS_MERGE(global_login_profile)",
+ "auth_attempts_median := STATS_PERCENTILE(stats, 0.5)",
+ "auth_attempts_sd := STATS_SD(stats)",
+ "profile_window := null",
+ "global_login_profile := null",
+ "stats := null"
+ ]
}
}
}
```
-We have a group called `numeric` whose stellar statements will be executed sequentially. In parallel to that, we have the group of stellar statements under the group `text` executing. The intent here is to allow you to not force higher latency operations to be done sequentially. You can use any name for your groupings you like. Be aware that the configuration is a map and duplicate configuration keys' values are not combined, so the duplicate configuration value will be overwritten.
+
+Here we want to perform two enrichments that hit HBase and we would rather not run in sequence. These
+enrichments are entirely independent of one another (i.e. neither relies on the output of the other). In
+this case, we've created a group called `malicious_domain_enrichment` to inquire about whether the destination
+address exists in the HBase enrichment table in the `malicious_domains` enrichment type. This is a simple
+enrichment, so we can express the enrichment group as a map with the new field `is_bad_domain` being a key
+and the stellar expression associated with that operation being the associated value.
+
+In contrast, the stellar enrichment group `login_profile` is interacting with the profiler, has multiple temporary
+expressions (i.e. `profile_window`, `global_login_profile`, and `stats`) that are useful only within the context
+of this group of stellar expressions. In this case, we would need to ensure that we use the list construct
+when specifying the group and remember to set the temporary variables to `null` so they are not passed along.
+
+In general, things to note from this section are as follows:
+* The stellar enrichments for the `stellar` enrichment adapter are specified in the `config` for the `stellar` enrichment
+adapter in the `fieldMap`
+* Groups of independent (i.e. no expression in any group depend on the output of an expression from an other group) may be executed in parallel
+* If you have the need to use temporary variables, you may use the list construct. Ensure that you assign the variables to `null` before the end of the group.
+* **Ensure that you do not assign a field to a stellar expression which returns an object which JSON cannot represent.**
--- End diff --
I agree, this was just filling in documentation that didn't exist. There should be a follow-on ticket here for a solution.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] metron pull request #609: METRON-987: Allow stellar enrichments to be specif...
Posted by cestella <gi...@git.apache.org>.
GitHub user cestella reopened a pull request:
https://github.com/apache/metron/pull/609
METRON-987: Allow stellar enrichments to be specified by a list as well as a map
## Contributor Comments
Currently, stellar enrichments are specified by a map associating fields with the stellar expressions associated with the fields. There is a significant downside here in that you cannot update previously assigned fields. For instance, the following cannot be represented currently:
```
"fieldMap": {
...
"stellar" : {
"config" : {
"hostname" : "if ENDS_WITH(hostname, '.') then CHOP(hostname) else hostname",
"hostname" : "TO_LOWER(hostname)"
}
}
}
```
This would now be allowed thusly:
```
"fieldMap": {
...
"stellar" : {
"config" : [
"hostname := if ENDS_WITH(hostname, '.') then CHOP(hostname) else hostname",
"hostname := TO_LOWER(hostname)"
]
}
}
```
A consequent of this deficiency is that we also cannot use temporary variables and unset them after their use inside an enrichment group.
The proposed change is to allow users to use lists of strings representing stellar expression assignments with the same syntax as the Stellar REPL. This would be as an alternative to maps, but the map syntax would also be supported for legacy.
## Test plan
* Follow the instructions located [here](https://github.com/apache/metron/tree/master/metron-platform/metron-enrichment#example-enrichment-via-stellar) to ensure no regressions
* Adjust the configuration for `$METRON_HOME/config/zookeeper/enrichments/squid.json` to the following and run data through the topologies again:
```
{
"enrichment" : {
"fieldMap": {
"stellar" : {
"config" : {
"numeric" : [
"foo := 1 + 1",
"grok := foo + 3"
]
,"ALL_CAPS" : "TO_UPPER(source.type)"
}
}
}
},
"threatIntel" : {
"fieldMap":{
"stellar" : {
"config" : [
"bar := TO_UPPER(source.type)"
]
}
},
"triageConfig" : {
}
}
}
```
* Ensure that each message has `ALL_CAPS`, `foo`, `grok` and `bar` fields.
## Pull Request Checklist
Thank you for submitting a contribution to Apache Metron.
Please refer to our [Development Guidelines](https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=61332235) for the complete guide to follow for contributions.
Please refer also to our [Build Verification Guidelines](https://cwiki.apache.org/confluence/display/METRON/Verifying+Builds?show-miniview) 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:
- [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?
- [ ] 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 in the root incubating-metron folder via:
```
mvn -q clean integration-test install && build_utils/verify_licenses.sh
```
- [x] Have you written or updated unit tests and or integration tests to verify your changes?
- [x] 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 Vagrant full-dev environment or the equivalent?
### For documentation related changes:
- [x] 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`:
```
cd site-book
mvn site
```
#### 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 recommended that [travis-ci](https://travis-ci.org) is set up for your personal repository such that your branches are built there before submitting a pull request.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/cestella/incubator-metron stellar_list_support
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/metron/pull/609.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #609
----
commit e9ea04bc6e9f42e1029d1d7fa6e98fa00e0226fe
Author: cstella <ce...@gmail.com>
Date: 2017-06-06T22:21:49Z
Adding the ability to specify Stellar enrichments as lists as well as Maps.
commit cba07c78d91451eb1ec318418c418269e7127e38
Author: cstella <ce...@gmail.com>
Date: 2017-06-06T23:17:11Z
Documentation update
commit 0960c6591edaedd612dafd04990af15209d3c65f
Author: cstella <ce...@gmail.com>
Date: 2017-06-06T23:50:43Z
Cleaning up a few things.
commit 5186857be94905f0837c393c854813847c6c7b85
Author: cstella <ce...@gmail.com>
Date: 2017-06-07T12:37:18Z
Fixed NPE.
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] metron issue #609: METRON-987: Allow stellar enrichments to be specified by ...
Posted by cestella <gi...@git.apache.org>.
Github user cestella commented on the issue:
https://github.com/apache/metron/pull/609
@justinleet yes, agreed. I wanted to get this functionality in and have it be backwards compatible for the short term, but ultimately, I think that it's a better approach and we should work toward phasing out the map-based approach. The issue is that there are a few other places that also need to be normalized:
* the field transformations
* the importer config
* possibly others that I am not thinking about
Anyway, to answer your question, yes I'll probably start a discuss thread on that tomorrow and construct some JIRAs to see if
* other people think deprecation is a good thing to work towards
* how in the heck we should accomplish it
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] metron pull request #609: METRON-987: Allow stellar enrichments to be specif...
Posted by justinleet <gi...@git.apache.org>.
Github user justinleet commented on a diff in the pull request:
https://github.com/apache/metron/pull/609#discussion_r120932742
--- Diff: metron-platform/metron-common/src/main/java/org/apache/metron/common/message/JSONFromPosition.java ---
@@ -40,10 +40,12 @@ public JSONFromPosition(int position) {
@Override
public JSONObject get(Tuple tuple) {
+ String s = null;
try {
- return (JSONObject) parser.get().parse(new String(tuple.getBinary(position), "UTF8"));
+ s = new String(tuple.getBinary(position), "UTF8");
--- End diff --
Can we just use StandardCharsets.UTF_8 then, and not even worry about the string being right? I'm not sure where that alias is, so I'm not convinced it's guaranteed (although a source proving me wrong would also be acceptable)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] metron pull request #609: METRON-987: Allow stellar enrichments to be specif...
Posted by justinleet <gi...@git.apache.org>.
Github user justinleet commented on a diff in the pull request:
https://github.com/apache/metron/pull/609#discussion_r120917140
--- Diff: metron-platform/metron-enrichment/README.md ---
@@ -71,40 +73,94 @@ The `fieldMap`contents are of interest because they contain the routing and conf
]
}
```
-Based on this sample config, both ip_src_addr and ip_dst_addr will go to the `geo`, `host`, and `hbaseEnrichment` adapter bolts. For the `geo`, `host` and `hbaseEnrichment`, this is sufficient. However, more complex enrichments may contain their own configuration. Currently, the `stellar` enrichment requires a more complex configuration, such as:
+Based on this sample config, both `ip_src_addr` and `ip_dst_addr` will go to the `geo`, `host`, and
+`hbaseEnrichment` adapter bolts.
+
+#### Stellar Enrichment Configuration
+For the `geo`, `host` and `hbaseEnrichment`, this is sufficient. However, more complex enrichments
+may contain their own configuration. Currently, the `stellar` enrichment is more adaptable and thus
+requires a more nuanced configuration.
+
+At its most basic, we want to take a message and apply a couple of enrichments, such as converting the
+`hostname` field to lowercase. We do this by specifying the transformation inside of the
+`config` for the `stellar` fieldMap. There are two syntaxes that are supported, specifying the transformations
+as a map with the key as the field and the value the stellar expression:
```
"fieldMap": {
...
"stellar" : {
"config" : {
- "numeric" : {
- "foo": "1 + 1"
- }
- ,"ALL_CAPS" : "TO_UPPER(source.type)"
+ "hostname" : "TO_LOWER(hostname)"
}
}
}
```
-Whereas the simpler enrichments just need a set of fields explicitly stated so they can be separated from the message and sent to the enrichment adapter bolt for enrichment and ultimately joined back in the join bolt, the stellar enrichment has its set of required fields implicitly stated through usage. For instance, if your stellar statement references a field, it should be included and if not, then it should not be included. We did not want to require users to make explicit the implicit.
+Another approach is to make the transformations as a list with the same `var := expr` syntax as is used
+in the Stellar REPL, such as:
+```
+ "fieldMap": {
+ ...
+ "stellar" : {
+ "config" : [
+ "hostname := TO_LOWER(hostname)"
+ ]
+ }
+ }
+```
+
+Sometimes arbitrary stellar enrichments may take enough time that you would prefer to split some of them
+into groups and execute the groups of stellar enrichments in parallel. Take, for instance, if you wanted
+to do an HBase enrichment and a profiler call which were independent of one another. This usecase is
+supported by splitting the enrichments up as groups.
-The other way in which the stellar enrichment is somewhat more complex is in how the statements are executed. In the general purpose case for a list of fields, those fields are used to create a message to send to the enrichment adapter bolt and that bolt's worker will handle the fields one by one in serial for a given message. For stellar enrichment, we wanted to have a more complex design so that users could specify the groups of stellar statements sent to the same worker in the same message (and thus executed sequentially). Consider the following configuration:
+Consider the following example:
```
"fieldMap": {
+ ...
"stellar" : {
"config" : {
- "numeric" : {
- "foo": "1 + 1"
- "bar" : TO_LOWER(source.type)"
- }
- ,"text" : {
- "ALL_CAPS" : "TO_UPPER(source.type)"
- }
+ "malicious_domain_enrichment" : {
+ "is_bad_domain" : "ENRICHMENT_EXISTS('malicious_domains', ip_dst_addr, 'enrichments', 'cf')"
+ },
+ "login_profile" : [
+ "profile_window := PROFILE_WINDOW('from 6 months ago')",
+ "global_login_profile := PROFILE_GET('distinct_login_attempts', 'global', profile_window)",
+ "stats := STATS_MERGE(global_login_profile)",
+ "auth_attempts_median := STATS_PERCENTILE(stats, 0.5)",
+ "auth_attempts_sd := STATS_SD(stats)",
+ "profile_window := null",
+ "global_login_profile := null",
+ "stats := null"
+ ]
}
}
}
```
-We have a group called `numeric` whose stellar statements will be executed sequentially. In parallel to that, we have the group of stellar statements under the group `text` executing. The intent here is to allow you to not force higher latency operations to be done sequentially. You can use any name for your groupings you like. Be aware that the configuration is a map and duplicate configuration keys' values are not combined, so the duplicate configuration value will be overwritten.
+
+Here we want to perform two enrichments that hit HBase and we would rather not run in sequence. These
+enrichments are entirely independent of one another (i.e. neither relies on the output of the other). In
+this case, we've created a group called `malicious_domain_enrichment` to inquire about whether the destination
+address exists in the HBase enrichment table in the `malicious_domains` enrichment type. This is a simple
+enrichment, so we can express the enrichment group as a map with the new field `is_bad_domain` being a key
+and the stellar expression associated with that operation being the associated value.
+
+In contrast, the stellar enrichment group `login_profile` is interacting with the profiler, has multiple temporary
+expressions (i.e. `profile_window`, `global_login_profile`, and `stats`) that are useful only within the context
+of this group of stellar expressions. In this case, we would need to ensure that we use the list construct
+when specifying the group and remember to set the temporary variables to `null` so they are not passed along.
+
+In general, things to note from this section are as follows:
+* The stellar enrichments for the `stellar` enrichment adapter are specified in the `config` for the `stellar` enrichment
+adapter in the `fieldMap`
+* Groups of independent (i.e. no expression in any group depend on the output of an expression from an other group) may be executed in parallel
+* If you have the need to use temporary variables, you may use the list construct. Ensure that you assign the variables to `null` before the end of the group.
+* **Ensure that you do not assign a field to a stellar expression which returns an object which JSON cannot represent.**
--- End diff --
I don't think it's in scope for this, but it feels like this should be enforceable at an early time. It would be nice to be able to do some high level checks like this before allowing things like this.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] metron pull request #609: METRON-987: Allow stellar enrichments to be specif...
Posted by cestella <gi...@git.apache.org>.
Github user cestella closed the pull request at:
https://github.com/apache/metron/pull/609
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] metron pull request #609: METRON-987: Allow stellar enrichments to be specif...
Posted by justinleet <gi...@git.apache.org>.
Github user justinleet commented on a diff in the pull request:
https://github.com/apache/metron/pull/609#discussion_r120921756
--- Diff: metron-platform/metron-enrichment/src/test/java/org/apache/metron/enrichment/adapters/stellar/StellarAdapterTest.java ---
@@ -0,0 +1,134 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.metron.enrichment.adapters.stellar;
+
+import com.google.common.collect.ImmutableList;
+import org.apache.metron.common.configuration.StellarEnrichmentTest;
+import org.apache.metron.common.configuration.enrichment.EnrichmentConfig;
+import org.apache.metron.common.configuration.enrichment.handler.ConfigHandler;
+import org.apache.metron.common.dsl.Context;
+import org.apache.metron.common.dsl.MapVariableResolver;
+import org.apache.metron.common.dsl.VariableResolver;
+import org.apache.metron.common.stellar.StellarProcessor;
+import org.apache.metron.common.utils.JSONUtils;
+import org.json.simple.JSONObject;
+import org.junit.Assert;
+import org.junit.Test;
+
+public class StellarAdapterTest extends StellarEnrichmentTest {
+ StellarProcessor processor = new StellarProcessor();
+
+ private JSONObject enrich(JSONObject message, String field, ConfigHandler handler) {
+ VariableResolver resolver = new MapVariableResolver(message);
+ return StellarAdapter.process( message
+ , handler
+ , field
+ , 1000l
--- End diff --
Out of my personality preference, can you make this 1000L, so I don't look at it again and think it's 10001?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] metron issue #609: METRON-987: Allow stellar enrichments to be specified by ...
Posted by justinleet <gi...@git.apache.org>.
Github user justinleet commented on the issue:
https://github.com/apache/metron/pull/609
A lot of this brings up the discussions around deprecating / dropping functionality (both for Stellar and in general). It seems like ideally, we'd be deprecating the map functionality in favor of the list functionality, probably letting it ride a few releases, and eventually dropping it after a migration period.
Do we need to start getting together a set of discussions and plans for handling this sort of deprecation as we build out enhanced versions of older features?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] metron issue #609: METRON-987: Allow stellar enrichments to be specified by ...
Posted by justinleet <gi...@git.apache.org>.
Github user justinleet commented on the issue:
https://github.com/apache/metron/pull/609
@cestella Perfect. Definitely didn't want to imply that belongs in this ticket at all. I'm in full support of it being backwards compatible for now, and keeping the discussion of how we prune old implementations separate.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] metron issue #609: METRON-987: Allow stellar enrichments to be specified by ...
Posted by justinleet <gi...@git.apache.org>.
Github user justinleet commented on the issue:
https://github.com/apache/metron/pull/609
+1, I was able to run this up, both the smoke test and the adjusted variant
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] metron pull request #609: METRON-987: Allow stellar enrichments to be specif...
Posted by cestella <gi...@git.apache.org>.
Github user cestella commented on a diff in the pull request:
https://github.com/apache/metron/pull/609#discussion_r120924836
--- Diff: metron-platform/metron-enrichment/src/test/java/org/apache/metron/enrichment/adapters/stellar/StellarAdapterTest.java ---
@@ -0,0 +1,134 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.metron.enrichment.adapters.stellar;
+
+import com.google.common.collect.ImmutableList;
+import org.apache.metron.common.configuration.StellarEnrichmentTest;
+import org.apache.metron.common.configuration.enrichment.EnrichmentConfig;
+import org.apache.metron.common.configuration.enrichment.handler.ConfigHandler;
+import org.apache.metron.common.dsl.Context;
+import org.apache.metron.common.dsl.MapVariableResolver;
+import org.apache.metron.common.dsl.VariableResolver;
+import org.apache.metron.common.stellar.StellarProcessor;
+import org.apache.metron.common.utils.JSONUtils;
+import org.json.simple.JSONObject;
+import org.junit.Assert;
+import org.junit.Test;
+
+public class StellarAdapterTest extends StellarEnrichmentTest {
+ StellarProcessor processor = new StellarProcessor();
+
+ private JSONObject enrich(JSONObject message, String field, ConfigHandler handler) {
+ VariableResolver resolver = new MapVariableResolver(message);
+ return StellarAdapter.process( message
+ , handler
+ , field
+ , 1000l
--- End diff --
But the confusion is part of the charm!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] metron issue #609: METRON-987: Allow stellar enrichments to be specified by ...
Posted by ottobackwards <gi...@git.apache.org>.
Github user ottobackwards commented on the issue:
https://github.com/apache/metron/pull/609
Do we want to add a version number to these configuration now? Then we can say no version is 0, this is 1 etc.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] metron pull request #609: METRON-987: Allow stellar enrichments to be specif...
Posted by cestella <gi...@git.apache.org>.
Github user cestella commented on a diff in the pull request:
https://github.com/apache/metron/pull/609#discussion_r120924708
--- Diff: metron-platform/metron-common/src/main/java/org/apache/metron/common/message/JSONFromPosition.java ---
@@ -40,10 +40,12 @@ public JSONFromPosition(int position) {
@Override
public JSONObject get(Tuple tuple) {
+ String s = null;
try {
- return (JSONObject) parser.get().parse(new String(tuple.getBinary(position), "UTF8"));
+ s = new String(tuple.getBinary(position), "UTF8");
--- End diff --
`UTF-8` and `UTF8` are aliases in the `StandardCharsets` from looking through the java API code.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] metron issue #609: METRON-987: Allow stellar enrichments to be specified by ...
Posted by cestella <gi...@git.apache.org>.
Github user cestella commented on the issue:
https://github.com/apache/metron/pull/609
@ottobackwards Yeah, that's why I went with backwards compatibility here, so we could defer that decision until we had a discussion. I think the enrichment topology should really change quite a bit and the config really should reflect that. It may be broader changes than just a version can encapsulate, in my mind.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] metron pull request #609: METRON-987: Allow stellar enrichments to be specif...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/metron/pull/609
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] metron pull request #609: METRON-987: Allow stellar enrichments to be specif...
Posted by justinleet <gi...@git.apache.org>.
Github user justinleet commented on a diff in the pull request:
https://github.com/apache/metron/pull/609#discussion_r120919459
--- Diff: metron-platform/metron-common/src/main/java/org/apache/metron/common/message/JSONFromPosition.java ---
@@ -40,10 +40,12 @@ public JSONFromPosition(int position) {
@Override
public JSONObject get(Tuple tuple) {
+ String s = null;
try {
- return (JSONObject) parser.get().parse(new String(tuple.getBinary(position), "UTF8"));
+ s = new String(tuple.getBinary(position), "UTF8");
--- End diff --
Isn't the identifying string "UTF-8", not "UTF8"?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---