[GitHub] metron pull request #871: METRON-1365: Allow PROFILE_GET to return a default...
Github user asfgit closed the pull request at: https://github.com/apache/metron/pull/871 ---
[GitHub] metron pull request #871: METRON-1365: Allow PROFILE_GET to return a default...
Github user cestella commented on a diff in the pull request:
https://github.com/apache/metron/pull/871#discussion_r158040472
--- Diff:
metron-analytics/metron-profiler-client/src/main/java/org/apache/metron/profiler/client/stellar/ProfilerClientConfig.java
---
@@ -56,8 +56,11 @@
/**
* A global property that defines the salt divisor used to store profile
data.
*/
- PROFILER_SALT_DIVISOR("profiler.client.salt.divisor", 1000L, Long.class);
-
+ PROFILER_SALT_DIVISOR("profiler.client.salt.divisor", 1000L, Long.class)
+ /**
+ * The default value to be returned if a profile is not written for a
given period for a profile and entity.
+ */
+ ,PROFILER_DEFAULT_VALUE("profiler.default.value", null, Object.class);
--- End diff --
Good catch, corrected.
---
[GitHub] metron pull request #871: METRON-1365: Allow PROFILE_GET to return a default...
Github user cestella commented on a diff in the pull request: https://github.com/apache/metron/pull/871#discussion_r158040446 --- Diff: metron-analytics/metron-profiler-client/src/test/java/org/apache/metron/profiler/client/stellar/GetProfileTest.java --- @@ -39,11 +39,7 @@ import org.junit.Before; import org.junit.Test; -import java.util.Arrays; -import java.util.Collections; -import java.util.HashMap; -import java.util.List; -import java.util.Map; +import java.util.*; --- End diff -- Good catch, corrected. ---
[GitHub] metron pull request #871: METRON-1365: Allow PROFILE_GET to return a default...
Github user cestella commented on a diff in the pull request:
https://github.com/apache/metron/pull/871#discussion_r158040425
--- Diff:
metron-analytics/metron-profiler-client/src/test/java/org/apache/metron/profiler/client/stellar/GetProfileTest.java
---
@@ -310,6 +306,36 @@ public void testOutsideTimeHorizon() {
Assert.assertEquals(0, result.size());
}
+ /**
+ * Default value should be able to be specified
+ */
+ @Test
+ public void testWithDefaultValue() {
+String expr = "PROFILE_GET('profile1', 'entity1', PROFILE_FIXED(4,
'HOURS'))";
+@SuppressWarnings("unchecked")
+List result = run(expr, List.class);
+
+// validate - expect to fail to read any values because we didn't
write any.
+Assert.assertEquals(0, result.size());
+
+// execute - read the profile values - with config_override.
+// first two override values are strings, third is deliberately a
number.
+testOverride("{'profiler.default.value' : 0}", 0);
+testOverride("{'profiler.default.value' : 'metron'}", "metron");
+testOverride("{'profiler.default.value' : []}", new ArrayList<>());
+ }
+
+ private void testOverride(String overrides, Object defaultVal) {
+ String expr = "PROFILE_GET('profile1', 'entity1', PROFILE_FIXED(4,
'HOURS'), [], " + overrides + ")"
+ ;
--- End diff --
Good catch, corrected.
---
[GitHub] metron pull request #871: METRON-1365: Allow PROFILE_GET to return a default...
Github user nickwallen commented on a diff in the pull request:
https://github.com/apache/metron/pull/871#discussion_r157390830
--- Diff:
metron-analytics/metron-profiler-client/src/test/java/org/apache/metron/profiler/client/stellar/GetProfileTest.java
---
@@ -310,6 +306,36 @@ public void testOutsideTimeHorizon() {
Assert.assertEquals(0, result.size());
}
+ /**
+ * Default value should be able to be specified
+ */
+ @Test
+ public void testWithDefaultValue() {
+String expr = "PROFILE_GET('profile1', 'entity1', PROFILE_FIXED(4,
'HOURS'))";
+@SuppressWarnings("unchecked")
+List result = run(expr, List.class);
+
+// validate - expect to fail to read any values because we didn't
write any.
+Assert.assertEquals(0, result.size());
+
+// execute - read the profile values - with config_override.
+// first two override values are strings, third is deliberately a
number.
+testOverride("{'profiler.default.value' : 0}", 0);
+testOverride("{'profiler.default.value' : 'metron'}", "metron");
+testOverride("{'profiler.default.value' : []}", new ArrayList<>());
+ }
+
+ private void testOverride(String overrides, Object defaultVal) {
+ String expr = "PROFILE_GET('profile1', 'entity1', PROFILE_FIXED(4,
'HOURS'), [], " + overrides + ")"
+ ;
--- End diff --
Small nit... semi on a new line?
---
[GitHub] metron pull request #871: METRON-1365: Allow PROFILE_GET to return a default...
Github user nickwallen commented on a diff in the pull request:
https://github.com/apache/metron/pull/871#discussion_r157390718
--- Diff:
metron-analytics/metron-profiler-client/src/main/java/org/apache/metron/profiler/client/stellar/ProfilerClientConfig.java
---
@@ -56,8 +56,11 @@
/**
* A global property that defines the salt divisor used to store profile
data.
*/
- PROFILER_SALT_DIVISOR("profiler.client.salt.divisor", 1000L, Long.class);
-
+ PROFILER_SALT_DIVISOR("profiler.client.salt.divisor", 1000L, Long.class)
+ /**
+ * The default value to be returned if a profile is not written for a
given period for a profile and entity.
+ */
+ ,PROFILER_DEFAULT_VALUE("profiler.default.value", null, Object.class);
--- End diff --
Small nits... Add a space above this field and move the comma so that it
matches the rest of the file
---
[GitHub] metron pull request #871: METRON-1365: Allow PROFILE_GET to return a default...
GitHub user cestella opened a pull request:
https://github.com/apache/metron/pull/871
METRON-1365: Allow PROFILE_GET to return a default value for a profile and
entity that does not have a value written.
## Contributor Comments
Right now the profiler is a sparse system, namely if data is not written
during a profile period for a given profile and entity, no values are written
to HBase. Some algorithms may need non-sparse data (e.g. time series outlier
detection algorithms). We should keep the default behavior as it stands, but
allow the user at read time to specify a default value. This is most cleanly
done by using the existing profiler config overrides (the 4rd argument to
`PROFILE_GET`) and adding a `profiler.default.value`.
You should be able to validate this in the REPL in full-dev by running:
```
PROFILE_GET('profile1', 'entity1', PROFILE_FIXED(4, 'HOURS'), {
'profiler.default.value' : 'metron'} )
```
You should see a list of strings with 16 or 17 copies of `metron` in it.
If you run it without the overrides, you should see an empty list returned.
## 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- where 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 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
profiler_default_values
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/metron/pull/871.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 #871
commit f763a728bce19f10b49c2d431814e25006a5b5d3
Author: cstella
Date: 2017-12-15T22:37:22Z
Adding the ability to return default values from the profiler.
commit 44267b0850c7fc0912408f36badf64556a30d927
Author: cstella
Date: 2017-12-15T22:39:14Z
Updating readme to support default values
---
