You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@storm.apache.org by unsleepy22 <gi...@git.apache.org> on 2016/03/16 07:18:52 UTC

[GitHub] storm pull request: [STORM-1268] port builtin-metrics to java

GitHub user unsleepy22 opened a pull request:

    https://github.com/apache/storm/pull/1218

    [STORM-1268] port builtin-metrics to java

    

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

    $ git pull https://github.com/unsleepy22/storm STORM-1268

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

    https://github.com/apache/storm/pull/1218.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 #1218
    
----
commit 5ddab72ae9cc13bc7f680daa44c27ee9fb7b87ef
Author: 卫乐 <we...@taobao.com>
Date:   2016-03-16T06:09:59Z

    port builtin-metrics to java

----


---
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] storm pull request: [STORM-1268] port builtin-metrics to java

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

    https://github.com/apache/storm/pull/1218#discussion_r56305899
  
    --- Diff: storm-core/src/jvm/org/apache/storm/daemon/metrics/BuiltinMetricsUtil.java ---
    @@ -0,0 +1,77 @@
    +/**
    + * 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.storm.daemon.metrics;
    +
    +import java.util.HashMap;
    +import java.util.Map;
    +import org.apache.storm.Config;
    +import org.apache.storm.metric.api.IMetric;
    +import org.apache.storm.metric.api.IStatefulObject;
    +import org.apache.storm.metric.api.StateMetric;
    +import org.apache.storm.stats.BoltExecutorStats;
    +import org.apache.storm.stats.SpoutExecutorStats;
    +import org.apache.storm.stats.StatsUtil;
    +import org.apache.storm.task.TopologyContext;
    +
    +public class BuiltinMetricsUtil {
    +    public static BuiltinMetrics mkData(String type, Object stats) {
    +        if (StatsUtil.SPOUT.equals(type)) {
    +            return new BuiltinSpoutMetrics((SpoutExecutorStats) stats);
    +        }
    +        return new BuiltinBoltMetrics((BoltExecutorStats) stats);
    +    }
    +
    +    public static void registerIconnectionServerMetric(Object server, Map stormConf, TopologyContext context) {
    +        if (server instanceof IStatefulObject) {
    +            registerMetric("__recv-iconnection", new StateMetric((IStatefulObject) server), stormConf, context);
    +        }
    +    }
    +
    +    public static void registerIconnectionClientMetrics(final Map nodePort2socket, Map stormConf, TopologyContext context) {
    +        IMetric metric = new IMetric() {
    +            @Override
    +            public Object getValueAndReset() {
    +                Map<Object, Object> ret = new HashMap<>();
    +                for (Object o : nodePort2socket.entrySet()) {
    --- End diff --
    
    No, it has to be cast to Map.Entry inside the `for` loop code because it's actually a clojure map which has no generic definition. Java compiler will complain if I replace `Object o` to `Map.Entry o`


---
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] storm pull request: [STORM-1268] port builtin-metrics to java

Posted by unsleepy22 <gi...@git.apache.org>.
Github user unsleepy22 commented on the pull request:

    https://github.com/apache/storm/pull/1218#issuecomment-198410768
  
    @revans2 could you take a look? Since porting task.clj depends on this, I'd like this to be merged as soon as possible and since stats.clj is done, builtin-metrics is much simpler.


---
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] storm pull request: [STORM-1268] port builtin-metrics to java

Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on the pull request:

    https://github.com/apache/storm/pull/1218#issuecomment-200542649
  
    It looks good to me I am +1, but I would like to see the one minor naming comment addressed.
    
    > nodePort2Socket can be renamed to nodePortToSocket. 
    



---
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] storm pull request: [STORM-1268] port builtin-metrics to java

Posted by unsleepy22 <gi...@git.apache.org>.
Github user unsleepy22 commented on the pull request:

    https://github.com/apache/storm/pull/1218#issuecomment-200407083
  
    ping @revans2 , could you take time to have a look? this PR blocks task.clj.


---
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] storm pull request: [STORM-1268] port builtin-metrics to java

Posted by abhishekagarwal87 <gi...@git.apache.org>.
Github user abhishekagarwal87 commented on the pull request:

    https://github.com/apache/storm/pull/1218#issuecomment-200660046
  
    @unsleepy22 can you squash the commits? 


---
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] storm pull request: [STORM-1268] port builtin-metrics to java

Posted by abhishekagarwal87 <gi...@git.apache.org>.
Github user abhishekagarwal87 commented on the pull request:

    https://github.com/apache/storm/pull/1218#issuecomment-197201014
  
    Minor comments. Looks good otherwise


---
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] storm pull request: [STORM-1268] port builtin-metrics to java

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

    https://github.com/apache/storm/pull/1218#discussion_r56317156
  
    --- Diff: storm-core/src/jvm/org/apache/storm/daemon/metrics/BuiltinMetricsUtil.java ---
    @@ -0,0 +1,77 @@
    +/**
    + * 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.storm.daemon.metrics;
    +
    +import java.util.HashMap;
    +import java.util.Map;
    +import org.apache.storm.Config;
    +import org.apache.storm.metric.api.IMetric;
    +import org.apache.storm.metric.api.IStatefulObject;
    +import org.apache.storm.metric.api.StateMetric;
    +import org.apache.storm.stats.BoltExecutorStats;
    +import org.apache.storm.stats.SpoutExecutorStats;
    +import org.apache.storm.stats.StatsUtil;
    +import org.apache.storm.task.TopologyContext;
    +
    +public class BuiltinMetricsUtil {
    +    public static BuiltinMetrics mkData(String type, Object stats) {
    +        if (StatsUtil.SPOUT.equals(type)) {
    +            return new BuiltinSpoutMetrics((SpoutExecutorStats) stats);
    +        }
    +        return new BuiltinBoltMetrics((BoltExecutorStats) stats);
    +    }
    +
    +    public static void registerIconnectionServerMetric(Object server, Map stormConf, TopologyContext context) {
    +        if (server instanceof IStatefulObject) {
    +            registerMetric("__recv-iconnection", new StateMetric((IStatefulObject) server), stormConf, context);
    +        }
    +    }
    +
    +    public static void registerIconnectionClientMetrics(final Map nodePort2socket, Map stormConf, TopologyContext context) {
    +        IMetric metric = new IMetric() {
    +            @Override
    +            public Object getValueAndReset() {
    +                Map<Object, Object> ret = new HashMap<>();
    +                for (Object o : nodePort2socket.entrySet()) {
    --- End diff --
    
    Yeah. Interesting nuance of java type erasure. This will work 
    ```
    Map.Entry o : (Set<Map.Entry>) nodePort2socket.entrySet()
    ```
    You need not change your code though. More details on the above behavior 
    http://stackoverflow.com/questions/14933648/iterating-over-a-map-entryset


---
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] storm pull request: [STORM-1268] port builtin-metrics to java

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

    https://github.com/apache/storm/pull/1218


---
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] storm pull request: [STORM-1268] port builtin-metrics to java

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

    https://github.com/apache/storm/pull/1218#discussion_r56292195
  
    --- Diff: storm-core/src/jvm/org/apache/storm/daemon/metrics/BuiltinMetricsUtil.java ---
    @@ -0,0 +1,77 @@
    +/**
    + * 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.storm.daemon.metrics;
    +
    +import java.util.HashMap;
    +import java.util.Map;
    +import org.apache.storm.Config;
    +import org.apache.storm.metric.api.IMetric;
    +import org.apache.storm.metric.api.IStatefulObject;
    +import org.apache.storm.metric.api.StateMetric;
    +import org.apache.storm.stats.BoltExecutorStats;
    +import org.apache.storm.stats.SpoutExecutorStats;
    +import org.apache.storm.stats.StatsUtil;
    +import org.apache.storm.task.TopologyContext;
    +
    +public class BuiltinMetricsUtil {
    +    public static BuiltinMetrics mkData(String type, Object stats) {
    +        if (StatsUtil.SPOUT.equals(type)) {
    +            return new BuiltinSpoutMetrics((SpoutExecutorStats) stats);
    +        }
    +        return new BuiltinBoltMetrics((BoltExecutorStats) stats);
    +    }
    +
    +    public static void registerIconnectionServerMetric(Object server, Map stormConf, TopologyContext context) {
    +        if (server instanceof IStatefulObject) {
    +            registerMetric("__recv-iconnection", new StateMetric((IStatefulObject) server), stormConf, context);
    +        }
    +    }
    +
    +    public static void registerIconnectionClientMetrics(final Map nodePort2socket, Map stormConf, TopologyContext context) {
    +        IMetric metric = new IMetric() {
    +            @Override
    +            public Object getValueAndReset() {
    +                Map<Object, Object> ret = new HashMap<>();
    +                for (Object o : nodePort2socket.entrySet()) {
    +                    Map.Entry entry = (Map.Entry) o;
    +                    Object nodePort = entry.getKey();
    +                    Object connection = entry.getValue();
    +                    if (connection instanceof IStatefulObject) {
    +                        ret.put(nodePort, ((IStatefulObject) connection).getState());
    +                    }
    +                }
    +                return ret;
    +            }
    +        };
    +        registerMetric("__send-iconnection", metric, stormConf, context);
    +    }
    +
    +    public static void registerQueueMetrics(Map queues, Map stormConf, TopologyContext context) {
    +        for (Object o : queues.entrySet()) {
    --- End diff --
    
    same comment here. 


---
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] storm pull request: [STORM-1268] port builtin-metrics to java

Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on the pull request:

    https://github.com/apache/storm/pull/1218#issuecomment-200982734
  
    Still +1


---
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] storm pull request: [STORM-1268] port builtin-metrics to java

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

    https://github.com/apache/storm/pull/1218#discussion_r56292137
  
    --- Diff: storm-core/src/jvm/org/apache/storm/daemon/metrics/BuiltinMetricsUtil.java ---
    @@ -0,0 +1,77 @@
    +/**
    + * 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.storm.daemon.metrics;
    +
    +import java.util.HashMap;
    +import java.util.Map;
    +import org.apache.storm.Config;
    +import org.apache.storm.metric.api.IMetric;
    +import org.apache.storm.metric.api.IStatefulObject;
    +import org.apache.storm.metric.api.StateMetric;
    +import org.apache.storm.stats.BoltExecutorStats;
    +import org.apache.storm.stats.SpoutExecutorStats;
    +import org.apache.storm.stats.StatsUtil;
    +import org.apache.storm.task.TopologyContext;
    +
    +public class BuiltinMetricsUtil {
    +    public static BuiltinMetrics mkData(String type, Object stats) {
    +        if (StatsUtil.SPOUT.equals(type)) {
    +            return new BuiltinSpoutMetrics((SpoutExecutorStats) stats);
    +        }
    +        return new BuiltinBoltMetrics((BoltExecutorStats) stats);
    +    }
    +
    +    public static void registerIconnectionServerMetric(Object server, Map stormConf, TopologyContext context) {
    +        if (server instanceof IStatefulObject) {
    +            registerMetric("__recv-iconnection", new StateMetric((IStatefulObject) server), stormConf, context);
    +        }
    +    }
    +
    +    public static void registerIconnectionClientMetrics(final Map nodePort2socket, Map stormConf, TopologyContext context) {
    +        IMetric metric = new IMetric() {
    +            @Override
    +            public Object getValueAndReset() {
    +                Map<Object, Object> ret = new HashMap<>();
    +                for (Object o : nodePort2socket.entrySet()) {
    --- End diff --
    
    this can be Map.Entry<Object, Object> entry : 


---
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] storm pull request: [STORM-1268] port builtin-metrics to java

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

    https://github.com/apache/storm/pull/1218#discussion_r56292072
  
    --- Diff: storm-core/src/jvm/org/apache/storm/daemon/metrics/BuiltinMetricsUtil.java ---
    @@ -0,0 +1,77 @@
    +/**
    + * 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.storm.daemon.metrics;
    +
    +import java.util.HashMap;
    +import java.util.Map;
    +import org.apache.storm.Config;
    +import org.apache.storm.metric.api.IMetric;
    +import org.apache.storm.metric.api.IStatefulObject;
    +import org.apache.storm.metric.api.StateMetric;
    +import org.apache.storm.stats.BoltExecutorStats;
    +import org.apache.storm.stats.SpoutExecutorStats;
    +import org.apache.storm.stats.StatsUtil;
    +import org.apache.storm.task.TopologyContext;
    +
    +public class BuiltinMetricsUtil {
    +    public static BuiltinMetrics mkData(String type, Object stats) {
    +        if (StatsUtil.SPOUT.equals(type)) {
    +            return new BuiltinSpoutMetrics((SpoutExecutorStats) stats);
    +        }
    +        return new BuiltinBoltMetrics((BoltExecutorStats) stats);
    +    }
    +
    +    public static void registerIconnectionServerMetric(Object server, Map stormConf, TopologyContext context) {
    +        if (server instanceof IStatefulObject) {
    +            registerMetric("__recv-iconnection", new StateMetric((IStatefulObject) server), stormConf, context);
    +        }
    +    }
    +
    +    public static void registerIconnectionClientMetrics(final Map nodePort2socket, Map stormConf, TopologyContext context) {
    --- End diff --
    
    nodePort2Socket can be renamed to nodePortToSocket. 


---
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] storm pull request: [STORM-1268] port builtin-metrics to java

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

    https://github.com/apache/storm/pull/1218#discussion_r57266090
  
    --- Diff: storm-core/src/jvm/org/apache/storm/daemon/metrics/BuiltinMetricsUtil.java ---
    @@ -0,0 +1,77 @@
    +/**
    + * 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.storm.daemon.metrics;
    +
    +import java.util.HashMap;
    +import java.util.Map;
    +import org.apache.storm.Config;
    +import org.apache.storm.metric.api.IMetric;
    +import org.apache.storm.metric.api.IStatefulObject;
    +import org.apache.storm.metric.api.StateMetric;
    +import org.apache.storm.stats.BoltExecutorStats;
    +import org.apache.storm.stats.SpoutExecutorStats;
    +import org.apache.storm.stats.StatsUtil;
    +import org.apache.storm.task.TopologyContext;
    +
    +public class BuiltinMetricsUtil {
    +    public static BuiltinMetrics mkData(String type, Object stats) {
    +        if (StatsUtil.SPOUT.equals(type)) {
    +            return new BuiltinSpoutMetrics((SpoutExecutorStats) stats);
    +        }
    +        return new BuiltinBoltMetrics((BoltExecutorStats) stats);
    +    }
    +
    +    public static void registerIconnectionServerMetric(Object server, Map stormConf, TopologyContext context) {
    +        if (server instanceof IStatefulObject) {
    +            registerMetric("__recv-iconnection", new StateMetric((IStatefulObject) server), stormConf, context);
    +        }
    +    }
    +
    +    public static void registerIconnectionClientMetrics(final Map nodePort2socket, Map stormConf, TopologyContext context) {
    +        IMetric metric = new IMetric() {
    +            @Override
    +            public Object getValueAndReset() {
    +                Map<Object, Object> ret = new HashMap<>();
    +                for (Object o : nodePort2socket.entrySet()) {
    +                    Map.Entry entry = (Map.Entry) o;
    +                    Object nodePort = entry.getKey();
    +                    Object connection = entry.getValue();
    +                    if (connection instanceof IStatefulObject) {
    +                        ret.put(nodePort, ((IStatefulObject) connection).getState());
    +                    }
    +                }
    +                return ret;
    +            }
    +        };
    +        registerMetric("__send-iconnection", metric, stormConf, context);
    +    }
    +
    +    public static void registerQueueMetrics(Map queues, Map stormConf, TopologyContext context) {
    +        for (Object o : queues.entrySet()) {
    --- End diff --
    
    addressed


---
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] storm pull request: [STORM-1268] port builtin-metrics to java

Posted by unsleepy22 <gi...@git.apache.org>.
Github user unsleepy22 commented on the pull request:

    https://github.com/apache/storm/pull/1218#issuecomment-200613922
  
    thanks, addressed


---
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] storm pull request: [STORM-1268] port builtin-metrics to java

Posted by unsleepy22 <gi...@git.apache.org>.
Github user unsleepy22 commented on the pull request:

    https://github.com/apache/storm/pull/1218#issuecomment-200667388
  
    @abhishekagarwal87 done~


---
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] storm pull request: [STORM-1268] port builtin-metrics to java

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

    https://github.com/apache/storm/pull/1218#discussion_r56292345
  
    --- Diff: storm-core/src/jvm/org/apache/storm/daemon/metrics/BuiltinMetricsUtil.java ---
    @@ -0,0 +1,77 @@
    +/**
    + * 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.storm.daemon.metrics;
    +
    +import java.util.HashMap;
    +import java.util.Map;
    +import org.apache.storm.Config;
    +import org.apache.storm.metric.api.IMetric;
    +import org.apache.storm.metric.api.IStatefulObject;
    +import org.apache.storm.metric.api.StateMetric;
    +import org.apache.storm.stats.BoltExecutorStats;
    +import org.apache.storm.stats.SpoutExecutorStats;
    +import org.apache.storm.stats.StatsUtil;
    +import org.apache.storm.task.TopologyContext;
    +
    +public class BuiltinMetricsUtil {
    +    public static BuiltinMetrics mkData(String type, Object stats) {
    +        if (StatsUtil.SPOUT.equals(type)) {
    +            return new BuiltinSpoutMetrics((SpoutExecutorStats) stats);
    +        }
    +        return new BuiltinBoltMetrics((BoltExecutorStats) stats);
    --- End diff --
    
    just to be safer, can you also check that type is equal to Bolt in the else block


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