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/02/02 17:20:20 UTC

[GitHub] incubator-metron pull request #435: METRON-684: Decouple Timestamp calculati...

GitHub user cestella opened a pull request:

    https://github.com/apache/incubator-metron/pull/435

    METRON-684: Decouple Timestamp calculation from PROFILE_GET

    Currently `PROFILE_GET` only supports a static lookback of a fixed duration. As we have more complicated, potentially sparse, lookbacks (e.g. the same time slice every tuesday for a month), it would be nice to decouple the construction of timestamps from `PROFILE_GET` into its own set of functions.
    
    The decoupling plus the default timestamp function is provided here, which is a fixed window starting from now.  This is implemented as `PROFILE_FIXED`.

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/cestella/incubator-metron METRON-684

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/incubator-metron/pull/435.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 #435
    
----
commit 6d6b0385a23fb2b8512e6cb85425425e73159be1
Author: cstella <ce...@gmail.com>
Date:   2017-02-02T17:17:57Z

    METRON-684: Decouple Timestamp calculation from PROFILE_GET

----


---
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] incubator-metron pull request #435: METRON-684: Decouple Timestamp calculati...

Posted by nickwallen <gi...@git.apache.org>.
Github user nickwallen commented on a diff in the pull request:

    https://github.com/apache/incubator-metron/pull/435#discussion_r99375831
  
    --- Diff: metron-analytics/metron-profiler-client/README.md ---
    @@ -29,8 +29,7 @@ The Stellar client consists of the `PROFILE_GET` command, which takes the follow
     REQUIRED:
         profile - The name of the profile
         entity - The name of the entity
    -    durationAgo - How long ago should values be retrieved from?
    -    units - The units of 'durationAgo'
    +    periods - The list of profile periods to grab
    --- End diff --
    
    Oops.  And here is an example already of the potential confusion between profile period and timestamp.  My own fault here, but it is a distinction that we need to try and make really clear.


---
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] incubator-metron pull request #435: METRON-684: Decouple Timestamp calculati...

Posted by cestella <gi...@git.apache.org>.
Github user cestella closed the pull request at:

    https://github.com/apache/incubator-metron/pull/435


---
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] incubator-metron pull request #435: METRON-684: Decouple Timestamp calculati...

Posted by cestella <gi...@git.apache.org>.
Github user cestella commented on a diff in the pull request:

    https://github.com/apache/incubator-metron/pull/435#discussion_r99391590
  
    --- Diff: metron-analytics/metron-profiler-client/src/main/java/org/apache/metron/profiler/client/stellar/Util.java ---
    @@ -0,0 +1,117 @@
    +/*
    + *
    + *  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.profiler.client.stellar;
    +
    +import org.apache.commons.lang.StringUtils;
    +import org.apache.metron.common.dsl.Context;
    +import org.apache.metron.common.dsl.ParseException;
    +import org.apache.metron.common.utils.ConversionUtils;
    +import org.slf4j.Logger;
    +import org.slf4j.LoggerFactory;
    +
    +import java.util.Collections;
    +import java.util.HashMap;
    +import java.util.List;
    +import java.util.Map;
    +import java.util.stream.Collectors;
    +import java.util.stream.Stream;
    +
    +import static java.lang.String.format;
    +import static org.apache.metron.common.dsl.Context.Capabilities.GLOBAL_CONFIG;
    +
    +public class Util {
    +  private static final Logger LOG = LoggerFactory.getLogger(Util.class);
    +
    +  /**
    +   * Ensure that the required capabilities are defined.
    +   * @param context The context to validate.
    +   * @param required The required capabilities.
    +   * @throws IllegalStateException if all of the required capabilities are not present in the Context.
    +   */
    +  public static void validateCapabilities(Context context, Context.Capabilities[] required) throws IllegalStateException {
    +
    +    // collect the name of each missing capability
    +    String missing = Stream
    +            .of(required)
    +            .filter(c -> !context.getCapability(c).isPresent())
    +            .map(c -> c.toString())
    +            .collect(Collectors.joining(", "));
    +
    +    if(StringUtils.isNotBlank(missing) || context == null) {
    +      throw new IllegalStateException("missing required context: " + missing);
    +    }
    +  }
    +  /**
    --- End diff --
    
    agreed


---
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] incubator-metron pull request #435: METRON-684: Decouple Timestamp calculati...

Posted by cestella <gi...@git.apache.org>.
Github user cestella commented on a diff in the pull request:

    https://github.com/apache/incubator-metron/pull/435#discussion_r99376030
  
    --- Diff: metron-analytics/metron-profiler-client/README.md ---
    @@ -29,8 +29,7 @@ The Stellar client consists of the `PROFILE_GET` command, which takes the follow
     REQUIRED:
         profile - The name of the profile
         entity - The name of the entity
    -    durationAgo - How long ago should values be retrieved from?
    -    units - The units of 'durationAgo'
    +    periods - The list of profile periods to grab
    --- End diff --
    
    Agreed and I think it should be the ProfilePeriod


---
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] incubator-metron issue #435: METRON-684: Decouple Timestamp calculation from...

Posted by cestella <gi...@git.apache.org>.
Github user cestella commented on the issue:

    https://github.com/apache/incubator-metron/pull/435
  
    By the way, I reran the acceptance tests after the last few commits to make sure it works on vagrant.


---
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] incubator-metron issue #435: METRON-684: Decouple Timestamp calculation from...

Posted by cestella <gi...@git.apache.org>.
Github user cestella commented on the issue:

    https://github.com/apache/incubator-metron/pull/435
  
    Testing Instructions beyond the normal smoke test (i.e. letting data
    flow through to the indices and checking them).
    
    ## Preliminaries
    * Set an environment variable to indicate `METRON_HOME`:
    `export METRON_HOME=/usr/metron/0.3.0` 
    
    * Create the profiler hbase table
    `echo "create 'profiler', 'P'" | hbase shell`
    
    * Open `~/rand_gen.py` and paste the following:
    ```
    #!/usr/bin/python
    import random
    import sys
    import time
    def main():
      mu = float(sys.argv[1])
      sigma = float(sys.argv[2])
      freq_s = int(sys.argv[3])
      while True:
        out = '{ "value" : ' + str(random.gauss(mu, sigma)) + ' }'
        print out
        sys.stdout.flush()
        time.sleep(freq_s)
    
    if __name__ == '__main__':
      main()
    ```
    This will generate random JSON maps with a numeric field called `value`
    
    * Set the profiler to use 1 minute tick durations:
      * Edit `$METRON_HOME/config/profiler.properties` to adjust the capture duration by changing `profiler.period.duration=15` to `profiler.period.duration=1`
      * Edit `$METRON_HOME/config/zookeeper/global.json` and add the following properties:
    ```
    "profiler.client.period.duration" : "1",
    "profiler.client.period.duration.units" : "MINUTES"
    ```
    
    ## Free Up Space on the virtual machine
    
    First, let's free up some headroom on the virtual machine.  If you are running this on a
    multinode cluster, you would not have to do this.
    * Kill monit via `service monit stop`
    * Kill tcpreplay via `for i in $(ps -ef | grep tcpreplay | awk '{print $2}');do kill -9 $i;done`
    * Kill existing parser topologies via 
       * `storm kill snort`
       * `storm kill bro`
    * We won't need the enrichment or indexing topologies for this test, so you can kill them via:
       * `storm kill enrichment`
       * `storm kill indexing`
    * Kill yaf via `for i in $(ps -ef | grep yaf | awk '{print $2}');do kill -9 $i;done`
    * Kill bro via `for i in $(ps -ef | grep bro | awk '{print $2}');do kill -9 $i;done`
    
    ## Start the profiler
    * `$METRON_HOME/bin/start_profiler_topology.sh`
    
    ## Test Case
    
    * Set up a profile to accept some synthetic data with a numeric `value` field and persist a stats summary of the data
      * Edit `$METRON_HOME/config/zookeeper/profiler.json` and paste in the following:
    ```
    {
      "profiles": [
        {
          "profile": "stat",
          "foreach": "'global'",
          "onlyif": "true",
          "init" : {
                   },
          "update": {
            "s": "STATS_ADD(s, value)"
                    },
          "result": "s"
        }
      ]
    }
    ```
    
    * Send some synthetic data directly to the profiler:
    `python ~/rand_gen.py 0 1 1 | /usr/hdp/current/kafka-broker/bin/kafka-console-producer.sh --broker-list node1:6667 --topic indexing`
    * Wait for at least 10 minutes and execute the following via the Stellar REPL:
    ```
    # Grab the last 10 minutes worth of timestamps
    PROFILE_FIXED( 10, 'MINUTES')
    # Looks like 10 were returned, great.  Now, validate that I get 10 profile measurements back
    PROFILE_GET('stat', 'global', PROFILE_FIXED( 10, 'MINUTES' ) )
    # Ok, now look at the mean across the distribution
    # STATS_MEAN( STATS_MERGE(PROFILE_GET('stat', 'global', PROFILE_FIXED( 10, 'MINUTES' ) )))
    ```
    For me, the following was the result:
    ```
    Stellar, Go!
    Please note that functions are loading lazily in the background and will be unavailable until loaded fully.
    {es.clustername=metron, es.ip=node1, es.port=9300, es.date.format=yyyy.MM.dd.HH, profiler.client.period.duration=1, profiler.client.period.duration.units=MINUTES}
    [Stellar]>>> # Grab the last 10 minutes worth of timestamps
    [Stellar]>>> PROFILE_FIXED( 10, 'MINUTES')
    Functions loaded, you may refer to functions now...
    [24767772, 24767773, 24767774, 24767775, 24767776, 24767777, 24767778, 24767779, 24767780, 24767781, 24767782]
    [Stellar]>>> # Looks like 10 were returned, great.  Now, validate that I get 10 profile measurements back
    [Stellar]>>> PROFILE_GET('stat', 'global', PROFILE_FIXED( 10, 'MINUTES' ) )
    [org.apache.metron.statistics.OnlineStatisticsProvider@44749031, org.apache.metron.statistics.OnlineStatisticsProvider@d2a7fbb9, org.apache.metron.statistics.OnlineStatisticsProvider@a217cfd7, org.apache.metron.statistics.OnlineStatisticsProvider@c5e42aed, org.apache.metron.statistics.OnlineStatisticsProvider@c4f4753d, org.apache.metron.statistics.OnlineStatisticsProvider@87a1606a, org.apache.metron.statistics.OnlineStatisticsProvider@e1b4c8dc, org.apache.metron.statistics.OnlineStatisticsProvider@fdb7b8d8]
    [Stellar]>>> # Ok, now look at the mean across the distribution
    [Stellar]>>> STATS_MEAN( STATS_MERGE(PROFILE_GET('stat', 'global', PROFILE_FIXED( 10, 'MINUTES' ) )))
    -0.0077433441069769265
    [Stellar]>>>
    ```



---
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] incubator-metron issue #435: METRON-684: Decouple Timestamp calculation from...

Posted by cestella <gi...@git.apache.org>.
Github user cestella commented on the issue:

    https://github.com/apache/incubator-metron/pull/435
  
    Ok, I think I captured your comments, @nickwallen 


---
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] incubator-metron pull request #435: METRON-684: Decouple Timestamp calculati...

Posted by cestella <gi...@git.apache.org>.
Github user cestella commented on a diff in the pull request:

    https://github.com/apache/incubator-metron/pull/435#discussion_r99375854
  
    --- Diff: metron-analytics/metron-profiler-common/src/main/java/org/apache/metron/profiler/ProfilePeriod.java ---
    @@ -51,7 +61,7 @@ public ProfilePeriod(long epochMillis, long duration, TimeUnit units) {
           throw new IllegalArgumentException(format(
                   "period duration must be greater than 0; got '%d %s'", duration, units));
         }
    -
    +    this.timestamp = epochMillis;
    --- End diff --
    
    That's good feedback, agreed.


---
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] incubator-metron pull request #435: METRON-684: Decouple Timestamp calculati...

Posted by cestella <gi...@git.apache.org>.
Github user cestella commented on a diff in the pull request:

    https://github.com/apache/incubator-metron/pull/435#discussion_r99391617
  
    --- Diff: metron-analytics/metron-profiler-client/src/main/java/org/apache/metron/profiler/client/stellar/ProfilerConfig.java ---
    @@ -0,0 +1,104 @@
    +/*
    + *
    + *  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.profiler.client.stellar;
    +
    +import org.apache.metron.common.utils.ConversionUtils;
    +import org.apache.metron.hbase.HTableProvider;
    +
    +import java.util.Map;
    +
    +public enum ProfilerConfig {
    --- End diff --
    
    thanks! :)


---
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] incubator-metron pull request #435: METRON-684: Decouple Timestamp calculati...

Posted by nickwallen <gi...@git.apache.org>.
Github user nickwallen commented on a diff in the pull request:

    https://github.com/apache/incubator-metron/pull/435#discussion_r99374496
  
    --- Diff: metron-analytics/metron-profiler-common/src/main/java/org/apache/metron/profiler/ProfilePeriod.java ---
    @@ -51,7 +61,7 @@ public ProfilePeriod(long epochMillis, long duration, TimeUnit units) {
           throw new IllegalArgumentException(format(
                   "period duration must be greater than 0; got '%d %s'", duration, units));
         }
    -
    +    this.timestamp = epochMillis;
    --- End diff --
    
    I can't see where you are using this value, but maybe I am missing it.
    
    I'd be cautious of using this value because it could be a timestamp anywhere throughout the period.  If you really need a timestamp for the period, then it might be better to use `getStartTimeMillis()`. That will give you the same timestamp for the period; no matter what initial timestamp was used to find that period.


---
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] incubator-metron pull request #435: METRON-684: Decouple Timestamp calculati...

Posted by cestella <gi...@git.apache.org>.
GitHub user cestella reopened a pull request:

    https://github.com/apache/incubator-metron/pull/435

    METRON-684: Decouple Timestamp calculation from PROFILE_GET

    Currently `PROFILE_GET` only supports a static lookback of a fixed duration. As we have more complicated, potentially sparse, lookbacks (e.g. the same time slice every tuesday for a month), it would be nice to decouple the construction of timestamps from `PROFILE_GET` into its own set of functions.
    
    The decoupling plus the default timestamp function is provided here, which is a fixed window starting from now.  This is implemented as `PROFILE_FIXED`.
    
    Test plan to follow.

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/cestella/incubator-metron METRON-684

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/incubator-metron/pull/435.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 #435
    
----
commit 6d6b0385a23fb2b8512e6cb85425425e73159be1
Author: cstella <ce...@gmail.com>
Date:   2017-02-02T17:17:57Z

    METRON-684: Decouple Timestamp calculation from PROFILE_GET

commit 365444a84c4d0142262d01a739aea6df525c2228
Author: cstella <ce...@gmail.com>
Date:   2017-02-02T17:22:00Z

    Licensing and a stupid search/replace mistake.

commit 5d6bac6f5e15ab8056cf86cbedcf3f82b82e3ca1
Author: cstella <ce...@gmail.com>
Date:   2017-02-02T18:00:17Z

    Fixed comments and licenses.

commit 1a80d32ce2bd59746b0b0b453ce867a9fea4f8f8
Author: cstella <ce...@gmail.com>
Date:   2017-02-02T20:34:19Z

    We want to transform the timestamp, not the period

commit e98f80cf472bbc62bce3b8cb8dae6068bee6eb9f
Author: cstella <ce...@gmail.com>
Date:   2017-02-02T20:53:22Z

    Whoops, mistook profile measurement periods for timestamps..they are not.

commit 6ed69fb17b66d97cb20e6da2eeefa9574b5145d8
Author: cstella <ce...@gmail.com>
Date:   2017-02-02T21:03:46Z

    Updating docs and variable naming to better reflect that we are interacting with profile periods, not timestamps.

commit c49eb2cee62c5044ca2e88f44375f5e1c151a7eb
Author: cstella <ce...@gmail.com>
Date:   2017-02-02T21:05:17Z

    renamed visitor

commit d2008fa8c78ff83c6532443ffdc57a46332ebfb1
Author: cstella <ce...@gmail.com>
Date:   2017-02-03T18:15:43Z

    reacting to nick's comments

commit 4869f563f5293a3c57d5de2671f3fbb1e0908f6a
Author: cstella <ce...@gmail.com>
Date:   2017-02-03T18:18:34Z

    Updated docs.

----


---
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] incubator-metron pull request #435: METRON-684: Decouple Timestamp calculati...

Posted by cestella <gi...@git.apache.org>.
Github user cestella closed the pull request at:

    https://github.com/apache/incubator-metron/pull/435


---
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] incubator-metron pull request #435: METRON-684: Decouple Timestamp calculati...

Posted by nickwallen <gi...@git.apache.org>.
Github user nickwallen commented on a diff in the pull request:

    https://github.com/apache/incubator-metron/pull/435#discussion_r99367666
  
    --- Diff: metron-analytics/metron-profiler-client/src/main/java/org/apache/metron/profiler/client/stellar/FixedLookback.java ---
    @@ -0,0 +1,74 @@
    +/*
    + *
    + *  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.profiler.client.stellar;
    +
    +import org.apache.metron.common.dsl.Context;
    +import org.apache.metron.common.dsl.ParseException;
    +import org.apache.metron.common.dsl.Stellar;
    +import org.apache.metron.common.dsl.StellarFunction;
    +import org.apache.metron.profiler.ProfilePeriod;
    +
    +import java.util.List;
    +import java.util.Map;
    +import java.util.Optional;
    +import java.util.concurrent.TimeUnit;
    +
    +@Stellar(
    +      namespace="PROFILE",
    +      name="FIXED",
    +      description="The profiler periods associated with a fixed lookback starting from now.",
    +      params={
    +        "durationAgo - How long ago should values be retrieved from?",
    +        "units - The units of 'durationAgo'.",
    +        "config_overrides - Optional - Map (in curly braces) of name:value pairs, each overriding the global config parameter " +
    +                "of the same name. Default is the empty Map, meaning no overrides."
    +      },
    +      returns="The selected profile measurement periods."
    --- End diff --
    
    Also, describing the type here would be good too.


---
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] incubator-metron pull request #435: METRON-684: Decouple Timestamp calculati...

Posted by nickwallen <gi...@git.apache.org>.
Github user nickwallen commented on a diff in the pull request:

    https://github.com/apache/incubator-metron/pull/435#discussion_r99372272
  
    --- Diff: metron-analytics/metron-profiler-client/src/main/java/org/apache/metron/profiler/client/stellar/ProfilerConfig.java ---
    @@ -0,0 +1,104 @@
    +/*
    + *
    + *  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.profiler.client.stellar;
    +
    +import org.apache.metron.common.utils.ConversionUtils;
    +import org.apache.metron.hbase.HTableProvider;
    +
    +import java.util.Map;
    +
    +public enum ProfilerConfig {
    --- End diff --
    
    Much better way to handle defaults.  I like it.  Clean.


---
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] incubator-metron pull request #435: METRON-684: Decouple Timestamp calculati...

Posted by cestella <gi...@git.apache.org>.
GitHub user cestella reopened a pull request:

    https://github.com/apache/incubator-metron/pull/435

    METRON-684: Decouple Timestamp calculation from PROFILE_GET

    Currently `PROFILE_GET` only supports a static lookback of a fixed duration. As we have more complicated, potentially sparse, lookbacks (e.g. the same time slice every tuesday for a month), it would be nice to decouple the construction of timestamps from `PROFILE_GET` into its own set of functions.
    
    The decoupling plus the default timestamp function is provided here, which is a fixed window starting from now.  This is implemented as `PROFILE_FIXED`.
    
    Test plan to follow.

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/cestella/incubator-metron METRON-684

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/incubator-metron/pull/435.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 #435
    
----
commit 6d6b0385a23fb2b8512e6cb85425425e73159be1
Author: cstella <ce...@gmail.com>
Date:   2017-02-02T17:17:57Z

    METRON-684: Decouple Timestamp calculation from PROFILE_GET

commit 365444a84c4d0142262d01a739aea6df525c2228
Author: cstella <ce...@gmail.com>
Date:   2017-02-02T17:22:00Z

    Licensing and a stupid search/replace mistake.

commit 5d6bac6f5e15ab8056cf86cbedcf3f82b82e3ca1
Author: cstella <ce...@gmail.com>
Date:   2017-02-02T18:00:17Z

    Fixed comments and licenses.

commit 1a80d32ce2bd59746b0b0b453ce867a9fea4f8f8
Author: cstella <ce...@gmail.com>
Date:   2017-02-02T20:34:19Z

    We want to transform the timestamp, not the period

commit e98f80cf472bbc62bce3b8cb8dae6068bee6eb9f
Author: cstella <ce...@gmail.com>
Date:   2017-02-02T20:53:22Z

    Whoops, mistook profile measurement periods for timestamps..they are not.

commit 6ed69fb17b66d97cb20e6da2eeefa9574b5145d8
Author: cstella <ce...@gmail.com>
Date:   2017-02-02T21:03:46Z

    Updating docs and variable naming to better reflect that we are interacting with profile periods, not timestamps.

commit c49eb2cee62c5044ca2e88f44375f5e1c151a7eb
Author: cstella <ce...@gmail.com>
Date:   2017-02-02T21:05:17Z

    renamed visitor

commit d2008fa8c78ff83c6532443ffdc57a46332ebfb1
Author: cstella <ce...@gmail.com>
Date:   2017-02-03T18:15:43Z

    reacting to nick's comments

commit 4869f563f5293a3c57d5de2671f3fbb1e0908f6a
Author: cstella <ce...@gmail.com>
Date:   2017-02-03T18:18:34Z

    Updated docs.

----


---
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] incubator-metron pull request #435: METRON-684: Decouple Timestamp calculati...

Posted by nickwallen <gi...@git.apache.org>.
Github user nickwallen commented on a diff in the pull request:

    https://github.com/apache/incubator-metron/pull/435#discussion_r99366827
  
    --- Diff: metron-analytics/metron-profiler-client/README.md ---
    @@ -29,8 +29,7 @@ The Stellar client consists of the `PROFILE_GET` command, which takes the follow
     REQUIRED:
         profile - The name of the profile
         entity - The name of the entity
    -    durationAgo - How long ago should values be retrieved from?
    -    units - The units of 'durationAgo'
    +    periods - The list of profile periods to grab
    --- End diff --
    
    Would be good to mention what type 'periods' is; aka epoch milliseconds.


---
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] incubator-metron pull request #435: METRON-684: Decouple Timestamp calculati...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/incubator-metron/pull/435


---
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] incubator-metron pull request #435: METRON-684: Decouple Timestamp calculati...

Posted by nickwallen <gi...@git.apache.org>.
Github user nickwallen commented on a diff in the pull request:

    https://github.com/apache/incubator-metron/pull/435#discussion_r99368863
  
    --- Diff: metron-analytics/metron-profiler-client/src/main/java/org/apache/metron/profiler/client/stellar/Util.java ---
    @@ -0,0 +1,117 @@
    +/*
    + *
    + *  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.profiler.client.stellar;
    +
    +import org.apache.commons.lang.StringUtils;
    +import org.apache.metron.common.dsl.Context;
    +import org.apache.metron.common.dsl.ParseException;
    +import org.apache.metron.common.utils.ConversionUtils;
    +import org.slf4j.Logger;
    +import org.slf4j.LoggerFactory;
    +
    +import java.util.Collections;
    +import java.util.HashMap;
    +import java.util.List;
    +import java.util.Map;
    +import java.util.stream.Collectors;
    +import java.util.stream.Stream;
    +
    +import static java.lang.String.format;
    +import static org.apache.metron.common.dsl.Context.Capabilities.GLOBAL_CONFIG;
    +
    +public class Util {
    +  private static final Logger LOG = LoggerFactory.getLogger(Util.class);
    +
    +  /**
    +   * Ensure that the required capabilities are defined.
    +   * @param context The context to validate.
    +   * @param required The required capabilities.
    +   * @throws IllegalStateException if all of the required capabilities are not present in the Context.
    +   */
    +  public static void validateCapabilities(Context context, Context.Capabilities[] required) throws IllegalStateException {
    +
    +    // collect the name of each missing capability
    +    String missing = Stream
    +            .of(required)
    +            .filter(c -> !context.getCapability(c).isPresent())
    +            .map(c -> c.toString())
    +            .collect(Collectors.joining(", "));
    +
    +    if(StringUtils.isNotBlank(missing) || context == null) {
    +      throw new IllegalStateException("missing required context: " + missing);
    +    }
    +  }
    +  /**
    --- End diff --
    
    Minor spacing issues (1 empty line between methods) here and later in the file.


---
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] incubator-metron pull request #435: METRON-684: Decouple Timestamp calculati...

Posted by cestella <gi...@git.apache.org>.
Github user cestella commented on a diff in the pull request:

    https://github.com/apache/incubator-metron/pull/435#discussion_r99376045
  
    --- Diff: metron-analytics/metron-profiler-client/src/main/java/org/apache/metron/profiler/client/stellar/FixedLookback.java ---
    @@ -0,0 +1,74 @@
    +/*
    + *
    + *  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.profiler.client.stellar;
    +
    +import org.apache.metron.common.dsl.Context;
    +import org.apache.metron.common.dsl.ParseException;
    +import org.apache.metron.common.dsl.Stellar;
    +import org.apache.metron.common.dsl.StellarFunction;
    +import org.apache.metron.profiler.ProfilePeriod;
    +
    +import java.util.List;
    +import java.util.Map;
    +import java.util.Optional;
    +import java.util.concurrent.TimeUnit;
    +
    +@Stellar(
    +      namespace="PROFILE",
    +      name="FIXED",
    +      description="The profiler periods associated with a fixed lookback starting from now.",
    +      params={
    +        "durationAgo - How long ago should values be retrieved from?",
    +        "units - The units of 'durationAgo'.",
    +        "config_overrides - Optional - Map (in curly braces) of name:value pairs, each overriding the global config parameter " +
    +                "of the same name. Default is the empty Map, meaning no overrides."
    +      },
    +      returns="The selected profile measurement periods."
    --- End diff --
    
    agreed


---
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] incubator-metron issue #435: METRON-684: Decouple Timestamp calculation from...

Posted by cestella <gi...@git.apache.org>.
Github user cestella commented on the issue:

    https://github.com/apache/incubator-metron/pull/435
  
    @nickwallen Yeah, I was under the mistaken impression that period was milliseconds since epoch and it's not.  I think I like your idea better and just pass ProfilePeriods around instead.  I'll make that adjustment.


---
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.
---