You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by wido <gi...@git.apache.org> on 2015/07/27 14:44:07 UTC

[GitHub] cloudstack pull request: CLOUDSTACK-8677: Call-home functionality ...

GitHub user wido opened a pull request:

    https://github.com/apache/cloudstack/pull/625

    CLOUDSTACK-8677: Call-home functionality for CloudStack

    With this commit the Management Server will be default generate a anonymous Usage
    report every 7 (seven) days and submit this information back to the Apache CloudStack project.
    
    These anonymous reports do NOT contain any information about Instance names, subnets, etc. It only
    contains numbers about how CloudStack is being used.
    
    This information is vital for the project to gain more insight in how CloudStack is being used.
    
    Users can turn the reporting off by setting usage.report.interval to 0 (zero)

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

    $ git pull https://github.com/wido/cloudstack CLOUDSTACK-8677

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

    https://github.com/apache/cloudstack/pull/625.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 #625
    
----
commit 1c61afb7b93313743e9df1d10242415f838e660b
Author: Wido den Hollander <wi...@widodh.nl>
Date:   2015-07-08T19:17:30Z

    CLOUDSTACK-8677: Call-home functionality for CloudStack
    
    With this commit the Management Server will be default generate a anonymous Usage
    report every 7 (seven) days and submit this information back to the Apache CloudStack project.
    
    These anonymous reports do NOT contain any information about Instance names, subnets, etc. It only
    contains numbers about how CloudStack is being used.
    
    This information is vital for the project to gain more insight in how CloudStack is being used.
    
    Users can turn the reporting off by setting usage.report.interval to 0 (zero)

----


---
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] cloudstack pull request: CLOUDSTACK-8677: Call-home functionality ...

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

    https://github.com/apache/cloudstack/pull/625#discussion_r35625883
  
    --- Diff: reporter/usage-report-collector.py ---
    @@ -0,0 +1,64 @@
    +#!/usr/bin/env python
    +# 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.
    +
    +from flask import abort, Flask, request, Response
    --- End diff --
    
    This service will not run in the mgmt server. It is where the data is being send. Users will not deploy this, it will be running on call-home.cloudstack.org


---
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] cloudstack pull request: CLOUDSTACK-8677: Call-home functionality ...

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

    https://github.com/apache/cloudstack/pull/625#issuecomment-135369995
  
    @bhaisaab Isn't it already a plug-in?
    
    Don't know if the PR is the right way to discuss it, but I also want to know how many Instances and cloud is running. So not only on upgrades. So it can send diagnostics every 7 days (default) and that way we can see how clouds grow and such.
    
    We know the install data and after that we see the growth.


---
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] cloudstack pull request: CLOUDSTACK-8677: Call-home functionality ...

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

    https://github.com/apache/cloudstack/pull/625#issuecomment-126695290
  
    @DaanHoogland I need 2.3.1 for this patch to work. It builds with 2.3.1, but not with 1.7.2


---
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] cloudstack pull request: CLOUDSTACK-8677: Call-home functionality ...

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

    https://github.com/apache/cloudstack/pull/625#issuecomment-127384025
  
    @wido I am pretty sure acs code has a problem with the new gson version (see https://builds.apache.org/job/cloudstack-pull-requests/872/org.apache.cloudstack$cloud-core/testReport/)


---
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] cloudstack pull request: CLOUDSTACK-8677: Call-home functionality ...

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

    https://github.com/apache/cloudstack/pull/625#discussion_r35948897
  
    --- Diff: setup/db/db/schema-452to460.sql ---
    @@ -398,3 +398,5 @@ CREATE TABLE `cloud`.`external_bigswitch_bcf_devices` (
       CONSTRAINT `fk_external_bigswitch_bcf_devices__host_id` FOREIGN KEY (`host_id`) REFERENCES `host`(`id`) ON DELETE CASCADE,
       CONSTRAINT `fk_external_bigswitch_bcf_devices__physical_network_id` FOREIGN KEY (`physical_network_id`) REFERENCES `physical_network`(`id`) ON DELETE CASCADE
     ) ENGINE=InnoDB DEFAULT CHARSET=utf8;
    +
    +INSERT IGNORE INTO `cloud`.`configuration` VALUES ("Advanced", 'DEFAULT', 'management-server', "usage.report.interval", 7, "Interval (days) between sending anonymous Usage Reports back to the CloudStack project", "", NULL, NULL, 0);
    --- End diff --
    
    @wido 
    @DaanHoogland is correct. DB query to insert new config items is not required now.


---
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] cloudstack pull request: CLOUDSTACK-8677: Call-home functionality ...

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

    https://github.com/apache/cloudstack/pull/625#issuecomment-212789349
  
    This will probably never merge. Closing for now.


---
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] cloudstack pull request: CLOUDSTACK-8677: Call-home functionality ...

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

    https://github.com/apache/cloudstack/pull/625#discussion_r35655350
  
    --- Diff: server/src/org/apache/cloudstack/report/UsageReporter.java ---
    @@ -0,0 +1,490 @@
    +// 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.cloudstack.report;
    +
    +import java.util.concurrent.Executors;
    +import java.util.concurrent.ScheduledExecutorService;
    +import java.util.concurrent.TimeUnit;
    +import java.util.ArrayList;
    +import java.util.List;
    +import java.util.Map;
    +import java.util.HashMap;
    +import java.text.DateFormat;
    +import java.text.SimpleDateFormat;
    +import java.sql.Connection;
    +import java.sql.PreparedStatement;
    +import java.sql.ResultSet;
    +import java.sql.SQLException;
    +import java.net.URL;
    +import java.net.SocketTimeoutException;
    +import java.net.MalformedURLException;
    +import java.net.ProtocolException;
    +import java.net.UnknownHostException;
    +import java.io.OutputStreamWriter;
    +import java.io.IOException;
    +
    +import javax.inject.Inject;
    +import javax.net.ssl.HttpsURLConnection;
    +
    +import org.apache.log4j.Logger;
    +import org.springframework.stereotype.Component;
    +
    +import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
    +import org.apache.cloudstack.managed.context.ManagedContextRunnable;
    +
    +import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao;
    +import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
    +
    +import org.apache.commons.codec.digest.DigestUtils;
    +
    +import com.cloud.host.HostVO;
    +import com.cloud.host.dao.HostDao;
    +import com.cloud.dc.ClusterVO;
    +import com.cloud.dc.dao.ClusterDao;
    +import com.cloud.dc.DataCenterVO;
    +import com.cloud.dc.dao.DataCenterDao;
    +import com.cloud.vm.UserVmVO;
    +import com.cloud.vm.dao.UserVmDao;
    +import com.cloud.vm.VMInstanceVO;
    +import com.cloud.vm.dao.VMInstanceDao;
    +import com.cloud.utils.db.SearchCriteria;
    +import com.cloud.utils.NumbersUtil;
    +import com.cloud.utils.component.ManagerBase;
    +import com.cloud.utils.component.ComponentMethodInterceptable;
    +import com.cloud.utils.concurrency.NamedThreadFactory;
    +import com.cloud.utils.db.DB;
    +import com.cloud.utils.db.TransactionLegacy;
    +import com.cloud.upgrade.dao.VersionDao;
    +import com.cloud.upgrade.dao.VersionVO;
    +import com.cloud.storage.dao.DiskOfferingDao;
    +import com.cloud.storage.DiskOfferingVO;
    +import com.google.gson.Gson;
    +import com.google.gson.GsonBuilder;
    +import com.google.common.util.concurrent.AtomicLongMap;
    +
    +@Component
    +public class UsageReporter extends ManagerBase implements ComponentMethodInterceptable {
    +    public static final Logger s_logger = Logger.getLogger(UsageReporter.class.getName());
    +
    +    /* !FIX ME! This should point to a Apache Infra host with SSL! */
    +    private String reportHost = "https://call-home.cloudstack.org/report";
    +
    +    private String uniqueID = null;
    +
    +    private static UsageReporter s_instance = null;
    +
    +    private ScheduledExecutorService _executor = null;
    +
    +    @Inject
    +    private ConfigurationDao _configDao;
    +    @Inject
    +    private HostDao _hostDao;
    +    @Inject
    +    private ClusterDao _clusterDao;
    +    @Inject
    +    private PrimaryDataStoreDao _storagePoolDao;
    +    @Inject
    +    private DataCenterDao _dataCenterDao;
    +    @Inject
    +    private UserVmDao _userVmDao;
    +    @Inject
    +    private VMInstanceDao _vmInstance;
    +    @Inject
    +    private VersionDao _versionDao;
    +    @Inject
    +    private DiskOfferingDao _diskOfferingDao;
    +
    +    int usageReportInterval = -1;
    +
    +    public static UsageReporter getInstance(Map<String, String> configs) {
    +        if (s_instance == null) {
    +            s_instance = new UsageReporter();
    +            s_instance.init(configs);
    --- End diff --
    
    Yes, add synchronized block or make that method synchronized for safety.


---
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] cloudstack pull request: CLOUDSTACK-8677: Call-home functionality ...

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

    https://github.com/apache/cloudstack/pull/625#issuecomment-127927660
  
    @DaanHoogland You are right. There has been a change between GSON 1.7 and 2.0 which causes a failure of the tests.
    
    Looking into this, but my Java Reflecting knowledge is low and that makes it hard to figure this out.


---
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] cloudstack pull request: CLOUDSTACK-8677: Call-home functionality ...

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

    https://github.com/apache/cloudstack/pull/625#issuecomment-126691096
  
    I confirmed the problem in the failing test is in the newer gson version. Hope this works with the older gson. testing ... 1-2


---
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] cloudstack pull request: CLOUDSTACK-8677: Call-home functionality ...

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

    https://github.com/apache/cloudstack/pull/625#discussion_r35605766
  
    --- Diff: reporter/usage-report-collector.py ---
    @@ -0,0 +1,64 @@
    +#!/usr/bin/env python
    +# 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.
    +
    +from flask import abort, Flask, request, Response
    +from elasticsearch import Elasticsearch
    +import json
    +import time
    +
    +def json_response(response):
    +    return json.dumps(response, indent=2) + "\n", 200, {'Content-Type': 'application/json; charset=utf-8'}
    +
    +def generate_app(config=None):
    +    app = Flask(__name__)
    +
    +    @app.route('/report/<unique_id>', methods=['POST'])
    +    def report(unique_id):
    +        # We expect JSON data, so if the Content-Type doesn't match JSON data we throw an error
    +        if 'Content-Type' in request.headers:
    +            if request.headers['Content-Type'] != 'application/json':
    +                abort(417, "No or incorrect Content-Type header was supplied")
    +
    +        index = "cloudstack-%s" % time.strftime("%Y.%m.%d", time.gmtime())
    +        timestamp = time.strftime("%Y-%m-%dT%H:%M:%SZ", time.gmtime())
    +
    +        es = Elasticsearch()
    +        es.indices.create(index=index, ignore=400)
    +
    +        report = json.loads(request.data)
    --- End diff --
    
    May be we wanted to add a small check to see report is None and proceed.


---
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] cloudstack pull request: CLOUDSTACK-8677: Call-home functionality ...

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

    https://github.com/apache/cloudstack/pull/625#issuecomment-131030515
  
    I have create a issue regarding the new Gson version: https://issues.apache.org/jira/browse/CLOUDSTACK-8708


---
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] cloudstack pull request: CLOUDSTACK-8677: Call-home functionality ...

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

    https://github.com/apache/cloudstack/pull/625#discussion_r35534267
  
    --- Diff: server/src/org/apache/cloudstack/report/UsageReporter.java ---
    @@ -0,0 +1,473 @@
    +// 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.cloudstack.report;
    +
    +import java.util.concurrent.Executors;
    +import java.util.concurrent.ScheduledExecutorService;
    +import java.util.concurrent.TimeUnit;
    +import java.util.ArrayList;
    +import java.util.List;
    +import java.util.Map;
    +import java.util.HashMap;
    +import java.text.DateFormat;
    +import java.text.SimpleDateFormat;
    +import java.sql.Connection;
    +import java.sql.PreparedStatement;
    +import java.sql.ResultSet;
    +import java.sql.SQLException;
    +import java.net.URL;
    +import java.net.SocketTimeoutException;
    +import java.net.MalformedURLException;
    +import java.net.ProtocolException;
    +import java.net.UnknownHostException;
    +import java.io.OutputStreamWriter;
    +import java.io.IOException;
    +
    +import javax.inject.Inject;
    +import javax.net.ssl.HttpsURLConnection;
    +
    +import org.apache.log4j.Logger;
    +import org.springframework.stereotype.Component;
    +
    +import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
    +import org.apache.cloudstack.managed.context.ManagedContextRunnable;
    +
    +import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao;
    +import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
    +
    +import org.apache.commons.codec.digest.DigestUtils;
    +
    +import com.cloud.host.HostVO;
    +import com.cloud.host.dao.HostDao;
    +import com.cloud.dc.ClusterVO;
    +import com.cloud.dc.dao.ClusterDao;
    +import com.cloud.dc.DataCenterVO;
    +import com.cloud.dc.dao.DataCenterDao;
    +import com.cloud.vm.UserVmVO;
    +import com.cloud.vm.dao.UserVmDao;
    +import com.cloud.vm.VMInstanceVO;
    +import com.cloud.vm.dao.VMInstanceDao;
    +import com.cloud.utils.db.SearchCriteria;
    +import com.cloud.utils.NumbersUtil;
    +import com.cloud.utils.component.ManagerBase;
    +import com.cloud.utils.component.ComponentMethodInterceptable;
    +import com.cloud.utils.concurrency.NamedThreadFactory;
    +import com.cloud.utils.db.DB;
    +import com.cloud.utils.db.TransactionLegacy;
    +import com.cloud.upgrade.dao.VersionDao;
    +import com.cloud.upgrade.dao.VersionVO;
    +import com.cloud.storage.dao.DiskOfferingDao;
    +import com.cloud.storage.DiskOfferingVO;
    +import com.google.gson.Gson;
    +import com.google.gson.GsonBuilder;
    +import com.google.common.util.concurrent.AtomicLongMap;
    +
    +@Component
    +public class UsageReporter extends ManagerBase implements ComponentMethodInterceptable {
    +    public static final Logger s_logger = Logger.getLogger(UsageReporter.class.getName());
    +
    +    /* !FIX ME! This should point to a Apache Infra host with SSL! */
    +    private String reportHost = "https://call-home.cloudstack.org/report";
    +
    +    private String uniqueID = null;
    +
    +    private static UsageReporter s_instance = null;
    +
    +    private ScheduledExecutorService _executor = null;
    +
    +    @Inject
    +    private ConfigurationDao _configDao;
    +    @Inject
    +    private HostDao _hostDao;
    +    @Inject
    +    private ClusterDao _clusterDao;
    +    @Inject
    +    private PrimaryDataStoreDao _storagePoolDao;
    +    @Inject
    +    private DataCenterDao _dataCenterDao;
    +    @Inject
    +    private UserVmDao _userVmDao;
    +    @Inject
    +    private VMInstanceDao _vmInstance;
    +    @Inject
    +    private VersionDao _versionDao;
    +    @Inject
    +    private DiskOfferingDao _diskOfferingDao;
    +
    +    int usageReportInterval = -1;
    +
    +    public static UsageReporter getInstance() {
    +        return s_instance;
    +    }
    +
    +    public static UsageReporter getInstance(Map<String, String> configs) {
    +        s_instance.init(configs);
    +        return s_instance;
    +    }
    +
    +    public UsageReporter() {
    +        s_instance = this;
    +    }
    +
    +    @Override
    +    public boolean start() {
    +        init(_configDao.getConfiguration());
    +        return true;
    +    }
    +
    +    private void init(Map<String, String> configs) {
    +        _executor = Executors.newScheduledThreadPool(1, new NamedThreadFactory("UsageReporter"));
    +
    +        usageReportInterval = NumbersUtil.parseInt(configs.get("usage.report.interval"), 7);
    +
    +        if (usageReportInterval > 0) {
    +            _executor.scheduleWithFixedDelay(new UsageCollector(), 7, usageReportInterval, TimeUnit.DAYS);
    +        }
    +
    +        uniqueID = getUniqueId();
    +    }
    +
    +    private void sendReport(String reportUri, String uniqueID, Map<String, Object> reportMap) {
    +
    +        GsonBuilder builder = new GsonBuilder();
    +
    +        AtomicGsonAdapter adapter = new AtomicGsonAdapter();
    +        builder.registerTypeAdapter(AtomicLongMap.class, adapter);
    +
    +        Gson gson = builder.create();
    +        String report = gson.toJson(reportMap);
    +
    +        int http_timeout = 15000;
    +
    +        try {
    +            s_logger.info("Usage Report will be send to: " + reportUri);
    +            s_logger.debug("REPORT: " + report);
    +
    +            URL url = new URL(reportUri + "/" + uniqueID);
    +
    +            HttpsURLConnection conn = (HttpsURLConnection) url.openConnection();
    --- End diff --
    
    Does it need to be closed as well? As well, we want to close all the resources in exception path case as well.


---
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] cloudstack pull request: CLOUDSTACK-8677: Call-home functionality ...

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

    https://github.com/apache/cloudstack/pull/625#discussion_r35625795
  
    --- Diff: reporter/usage-report-collector.py ---
    @@ -0,0 +1,64 @@
    +#!/usr/bin/env python
    +# 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.
    +
    +from flask import abort, Flask, request, Response
    +from elasticsearch import Elasticsearch
    +import json
    +import time
    +
    +def json_response(response):
    +    return json.dumps(response, indent=2) + "\n", 200, {'Content-Type': 'application/json; charset=utf-8'}
    +
    +def generate_app(config=None):
    +    app = Flask(__name__)
    +
    +    @app.route('/report/<unique_id>', methods=['POST'])
    +    def report(unique_id):
    +        # We expect JSON data, so if the Content-Type doesn't match JSON data we throw an error
    +        if 'Content-Type' in request.headers:
    +            if request.headers['Content-Type'] != 'application/json':
    +                abort(417, "No or incorrect Content-Type header was supplied")
    +
    +        index = "cloudstack-%s" % time.strftime("%Y.%m.%d", time.gmtime())
    +        timestamp = time.strftime("%Y-%m-%dT%H:%M:%SZ", time.gmtime())
    +
    +        es = Elasticsearch()
    +        es.indices.create(index=index, ignore=400)
    +
    +        report = json.loads(request.data)
    --- End diff --
    
    Good point, although Flask will catch exceptions and generate HTTP errors based on those


---
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] cloudstack pull request: CLOUDSTACK-8677: Call-home functionality ...

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

    https://github.com/apache/cloudstack/pull/625#discussion_r35954149
  
    --- Diff: setup/db/db/schema-452to460.sql ---
    @@ -398,3 +398,5 @@ CREATE TABLE `cloud`.`external_bigswitch_bcf_devices` (
       CONSTRAINT `fk_external_bigswitch_bcf_devices__host_id` FOREIGN KEY (`host_id`) REFERENCES `host`(`id`) ON DELETE CASCADE,
       CONSTRAINT `fk_external_bigswitch_bcf_devices__physical_network_id` FOREIGN KEY (`physical_network_id`) REFERENCES `physical_network`(`id`) ON DELETE CASCADE
     ) ENGINE=InnoDB DEFAULT CHARSET=utf8;
    +
    +INSERT IGNORE INTO `cloud`.`configuration` VALUES ("Advanced", 'DEFAULT', 'management-server', "usage.report.interval", 7, "Interval (days) between sending anonymous Usage Reports back to the CloudStack project", "", NULL, NULL, 0);
    --- End diff --
    
    But only if you use new style ConfigKey!


---
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] cloudstack pull request: CLOUDSTACK-8677: Call-home functionality ...

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

    https://github.com/apache/cloudstack/pull/625#discussion_r35532842
  
    --- Diff: setup/db/db/schema-452to460.sql ---
    @@ -398,3 +398,5 @@ CREATE TABLE `cloud`.`external_bigswitch_bcf_devices` (
       CONSTRAINT `fk_external_bigswitch_bcf_devices__host_id` FOREIGN KEY (`host_id`) REFERENCES `host`(`id`) ON DELETE CASCADE,
       CONSTRAINT `fk_external_bigswitch_bcf_devices__physical_network_id` FOREIGN KEY (`physical_network_id`) REFERENCES `physical_network`(`id`) ON DELETE CASCADE
     ) ENGINE=InnoDB DEFAULT CHARSET=utf8;
    +
    +INSERT IGNORE INTO `cloud`.`configuration` VALUES ("Advanced", 'DEFAULT', 'management-server', "usage.report.interval", 7, "Interval (days) between sending anonymous Usage Reports back to the CloudStack project", "", NULL, NULL, 0);
    --- End diff --
    
    with the new style config (ConfigKey<T>), this isn't needed is it? It should insert the default if it doesn't find the value. 


---
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] cloudstack pull request: CLOUDSTACK-8677: Call-home functionality ...

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

    https://github.com/apache/cloudstack/pull/625#discussion_r35625751
  
    --- Diff: server/src/org/apache/cloudstack/report/UsageReporter.java ---
    @@ -0,0 +1,487 @@
    +// 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.cloudstack.report;
    +
    +import java.util.concurrent.Executors;
    +import java.util.concurrent.ScheduledExecutorService;
    +import java.util.concurrent.TimeUnit;
    +import java.util.ArrayList;
    +import java.util.List;
    +import java.util.Map;
    +import java.util.HashMap;
    +import java.text.DateFormat;
    +import java.text.SimpleDateFormat;
    +import java.sql.Connection;
    +import java.sql.PreparedStatement;
    +import java.sql.ResultSet;
    +import java.sql.SQLException;
    +import java.net.URL;
    +import java.net.SocketTimeoutException;
    +import java.net.MalformedURLException;
    +import java.net.ProtocolException;
    +import java.net.UnknownHostException;
    +import java.io.OutputStreamWriter;
    +import java.io.IOException;
    +
    +import javax.inject.Inject;
    +import javax.net.ssl.HttpsURLConnection;
    +
    +import org.apache.log4j.Logger;
    +import org.springframework.stereotype.Component;
    +
    +import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
    +import org.apache.cloudstack.managed.context.ManagedContextRunnable;
    +
    +import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao;
    +import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
    +
    +import org.apache.commons.codec.digest.DigestUtils;
    +
    +import com.cloud.host.HostVO;
    +import com.cloud.host.dao.HostDao;
    +import com.cloud.dc.ClusterVO;
    +import com.cloud.dc.dao.ClusterDao;
    +import com.cloud.dc.DataCenterVO;
    +import com.cloud.dc.dao.DataCenterDao;
    +import com.cloud.vm.UserVmVO;
    +import com.cloud.vm.dao.UserVmDao;
    +import com.cloud.vm.VMInstanceVO;
    +import com.cloud.vm.dao.VMInstanceDao;
    +import com.cloud.utils.db.SearchCriteria;
    +import com.cloud.utils.NumbersUtil;
    +import com.cloud.utils.component.ManagerBase;
    +import com.cloud.utils.component.ComponentMethodInterceptable;
    +import com.cloud.utils.concurrency.NamedThreadFactory;
    +import com.cloud.utils.db.DB;
    +import com.cloud.utils.db.TransactionLegacy;
    +import com.cloud.upgrade.dao.VersionDao;
    +import com.cloud.upgrade.dao.VersionVO;
    +import com.cloud.storage.dao.DiskOfferingDao;
    +import com.cloud.storage.DiskOfferingVO;
    +import com.google.gson.Gson;
    +import com.google.gson.GsonBuilder;
    +import com.google.common.util.concurrent.AtomicLongMap;
    +
    +@Component
    +public class UsageReporter extends ManagerBase implements ComponentMethodInterceptable {
    +    public static final Logger s_logger = Logger.getLogger(UsageReporter.class.getName());
    +
    +    /* !FIX ME! This should point to a Apache Infra host with SSL! */
    +    private String reportHost = "https://call-home.cloudstack.org/report";
    +
    +    private String uniqueID = null;
    +
    +    private static UsageReporter s_instance = null;
    +
    +    private ScheduledExecutorService _executor = null;
    +
    +    @Inject
    +    private ConfigurationDao _configDao;
    +    @Inject
    +    private HostDao _hostDao;
    +    @Inject
    +    private ClusterDao _clusterDao;
    +    @Inject
    +    private PrimaryDataStoreDao _storagePoolDao;
    +    @Inject
    +    private DataCenterDao _dataCenterDao;
    +    @Inject
    +    private UserVmDao _userVmDao;
    +    @Inject
    +    private VMInstanceDao _vmInstance;
    +    @Inject
    +    private VersionDao _versionDao;
    +    @Inject
    +    private DiskOfferingDao _diskOfferingDao;
    +
    +    int usageReportInterval = -1;
    +
    +    public static UsageReporter getInstance() {
    --- End diff --
    
    Can you explain that a bit more? I think I'm missing some knowledge 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] cloudstack pull request: CLOUDSTACK-8677: Call-home functionality ...

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

    https://github.com/apache/cloudstack/pull/625#issuecomment-126685792
  
    let's find out. it might reveal some totally unrelated problem due to .... I never saw the serialization tests before AFAIR but will look at them now ;)


---
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] cloudstack pull request: CLOUDSTACK-8677: Call-home functionality ...

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

    https://github.com/apache/cloudstack/pull/625#discussion_r35533953
  
    --- Diff: setup/db/db/schema-452to460.sql ---
    @@ -398,3 +398,5 @@ CREATE TABLE `cloud`.`external_bigswitch_bcf_devices` (
       CONSTRAINT `fk_external_bigswitch_bcf_devices__host_id` FOREIGN KEY (`host_id`) REFERENCES `host`(`id`) ON DELETE CASCADE,
       CONSTRAINT `fk_external_bigswitch_bcf_devices__physical_network_id` FOREIGN KEY (`physical_network_id`) REFERENCES `physical_network`(`id`) ON DELETE CASCADE
     ) ENGINE=InnoDB DEFAULT CHARSET=utf8;
    +
    +INSERT IGNORE INTO `cloud`.`configuration` VALUES ("Advanced", 'DEFAULT', 'management-server', "usage.report.interval", 7, "Interval (days) between sending anonymous Usage Reports back to the CloudStack project", "", NULL, NULL, 0);
    --- End diff --
    
    Hmm, I don't know. I wrote this code in 2014 and was that in there by now? I'm just doing this because I assumed it was still required.
    
    Maybe @bhaisaab knows?


---
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] cloudstack pull request: CLOUDSTACK-8677: Call-home functionality ...

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

    https://github.com/apache/cloudstack/pull/625#discussion_r35534192
  
    --- Diff: server/src/org/apache/cloudstack/report/UsageReporter.java ---
    @@ -0,0 +1,473 @@
    +// 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.cloudstack.report;
    +
    +import java.util.concurrent.Executors;
    +import java.util.concurrent.ScheduledExecutorService;
    +import java.util.concurrent.TimeUnit;
    +import java.util.ArrayList;
    +import java.util.List;
    +import java.util.Map;
    +import java.util.HashMap;
    +import java.text.DateFormat;
    +import java.text.SimpleDateFormat;
    +import java.sql.Connection;
    +import java.sql.PreparedStatement;
    +import java.sql.ResultSet;
    +import java.sql.SQLException;
    +import java.net.URL;
    +import java.net.SocketTimeoutException;
    +import java.net.MalformedURLException;
    +import java.net.ProtocolException;
    +import java.net.UnknownHostException;
    +import java.io.OutputStreamWriter;
    +import java.io.IOException;
    +
    +import javax.inject.Inject;
    +import javax.net.ssl.HttpsURLConnection;
    +
    +import org.apache.log4j.Logger;
    +import org.springframework.stereotype.Component;
    +
    +import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
    +import org.apache.cloudstack.managed.context.ManagedContextRunnable;
    +
    +import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao;
    +import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
    +
    +import org.apache.commons.codec.digest.DigestUtils;
    +
    +import com.cloud.host.HostVO;
    +import com.cloud.host.dao.HostDao;
    +import com.cloud.dc.ClusterVO;
    +import com.cloud.dc.dao.ClusterDao;
    +import com.cloud.dc.DataCenterVO;
    +import com.cloud.dc.dao.DataCenterDao;
    +import com.cloud.vm.UserVmVO;
    +import com.cloud.vm.dao.UserVmDao;
    +import com.cloud.vm.VMInstanceVO;
    +import com.cloud.vm.dao.VMInstanceDao;
    +import com.cloud.utils.db.SearchCriteria;
    +import com.cloud.utils.NumbersUtil;
    +import com.cloud.utils.component.ManagerBase;
    +import com.cloud.utils.component.ComponentMethodInterceptable;
    +import com.cloud.utils.concurrency.NamedThreadFactory;
    +import com.cloud.utils.db.DB;
    +import com.cloud.utils.db.TransactionLegacy;
    +import com.cloud.upgrade.dao.VersionDao;
    +import com.cloud.upgrade.dao.VersionVO;
    +import com.cloud.storage.dao.DiskOfferingDao;
    +import com.cloud.storage.DiskOfferingVO;
    +import com.google.gson.Gson;
    +import com.google.gson.GsonBuilder;
    +import com.google.common.util.concurrent.AtomicLongMap;
    +
    +@Component
    +public class UsageReporter extends ManagerBase implements ComponentMethodInterceptable {
    +    public static final Logger s_logger = Logger.getLogger(UsageReporter.class.getName());
    +
    +    /* !FIX ME! This should point to a Apache Infra host with SSL! */
    +    private String reportHost = "https://call-home.cloudstack.org/report";
    +
    +    private String uniqueID = null;
    +
    +    private static UsageReporter s_instance = null;
    +
    +    private ScheduledExecutorService _executor = null;
    +
    +    @Inject
    +    private ConfigurationDao _configDao;
    +    @Inject
    +    private HostDao _hostDao;
    +    @Inject
    +    private ClusterDao _clusterDao;
    +    @Inject
    +    private PrimaryDataStoreDao _storagePoolDao;
    +    @Inject
    +    private DataCenterDao _dataCenterDao;
    +    @Inject
    +    private UserVmDao _userVmDao;
    +    @Inject
    +    private VMInstanceDao _vmInstance;
    +    @Inject
    +    private VersionDao _versionDao;
    +    @Inject
    +    private DiskOfferingDao _diskOfferingDao;
    +
    +    int usageReportInterval = -1;
    +
    +    public static UsageReporter getInstance() {
    +        return s_instance;
    +    }
    +
    +    public static UsageReporter getInstance(Map<String, String> configs) {
    +        s_instance.init(configs);
    +        return s_instance;
    +    }
    +
    +    public UsageReporter() {
    +        s_instance = this;
    +    }
    +
    +    @Override
    +    public boolean start() {
    +        init(_configDao.getConfiguration());
    +        return true;
    +    }
    +
    +    private void init(Map<String, String> configs) {
    +        _executor = Executors.newScheduledThreadPool(1, new NamedThreadFactory("UsageReporter"));
    +
    +        usageReportInterval = NumbersUtil.parseInt(configs.get("usage.report.interval"), 7);
    +
    +        if (usageReportInterval > 0) {
    +            _executor.scheduleWithFixedDelay(new UsageCollector(), 7, usageReportInterval, TimeUnit.DAYS);
    +        }
    +
    +        uniqueID = getUniqueId();
    +    }
    +
    +    private void sendReport(String reportUri, String uniqueID, Map<String, Object> reportMap) {
    +
    +        GsonBuilder builder = new GsonBuilder();
    +
    +        AtomicGsonAdapter adapter = new AtomicGsonAdapter();
    +        builder.registerTypeAdapter(AtomicLongMap.class, adapter);
    +
    +        Gson gson = builder.create();
    +        String report = gson.toJson(reportMap);
    +
    +        int http_timeout = 15000;
    +
    +        try {
    +            s_logger.info("Usage Report will be send to: " + reportUri);
    +            s_logger.debug("REPORT: " + report);
    +
    +            URL url = new URL(reportUri + "/" + uniqueID);
    +
    +            HttpsURLConnection conn = (HttpsURLConnection) url.openConnection();
    +            conn.setConnectTimeout(http_timeout);
    +            conn.setReadTimeout(http_timeout);
    +            conn.setRequestMethod("POST");
    +            conn.setDoOutput(true);
    +            conn.setRequestProperty("Content-Type", "application/json");
    +            conn.setRequestProperty("Accept", "application/json");
    +
    +            OutputStreamWriter osw = new OutputStreamWriter(conn.getOutputStream());
    +            osw.write(report);
    +            osw.flush();
    +            osw.close();
    +
    +            int resp_code = conn.getResponseCode();
    +
    +            if (resp_code == HttpsURLConnection.HTTP_OK){
    +                s_logger.info("Usage Report succesfully send to: " + reportUri);
    +            } else {
    +                s_logger.warn("Failed to send Usage Report: " + conn.getResponseMessage());
    +            }
    +
    +        } catch (UnknownHostException e) {
    +            s_logger.warn("Failed to look up Usage Report host: " + e.getMessage());
    +        } catch (SocketTimeoutException e) {
    +            s_logger.warn("Sending Usage Report to " + reportUri + " timed out: " + e.getMessage());
    +        } catch (MalformedURLException e) {
    +            s_logger.warn(reportUri + " is a invalid URL for sending Usage Report to: "+ e.getMessage());
    +        } catch (ProtocolException e) {
    +            s_logger.warn("Sending Usage Report failed due to a invalid protocol: " + e.getMessage());
    +        } catch (IOException e) {
    +            s_logger.warn("Failed to write Usage Report due to a IOException: " + e.getMessage());
    +        }
    +    }
    +
    +    @DB
    +    private String getUniqueId() {
    +        String unique = null;
    +        Connection conn = null;
    +
    +        try {
    +            conn = TransactionLegacy.getStandaloneConnection();
    +
    +            PreparedStatement pstmt = conn.prepareStatement("SELECT version,updated FROM version ORDER BY id ASC LIMIT 1");
    --- End diff --
    
    Is this resource as well closed? Do we want to close resources in exception path as well.


---
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] cloudstack pull request: CLOUDSTACK-8677: Call-home functionality ...

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

    https://github.com/apache/cloudstack/pull/625#discussion_r35971855
  
    --- Diff: pom.xml ---
    @@ -64,6 +64,7 @@
         <cs.ehcache.version>2.6.9</cs.ehcache.version>
         <cs.gson.version>1.7.2</cs.gson.version>
         <cs.guava-testlib.version>18.0</cs.guava-testlib.version>
    +    <cs.gson.version>2.3.1</cs.gson.version>
    --- End diff --
    
    cs.gson.version is defined a second time 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] cloudstack pull request: CLOUDSTACK-8677: Call-home functionality ...

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

    https://github.com/apache/cloudstack/pull/625#issuecomment-125199454
  
    any way to verify 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] cloudstack pull request: CLOUDSTACK-8677: Call-home functionality ...

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

    https://github.com/apache/cloudstack/pull/625#issuecomment-125505270
  
    @wido you are introducing two findbugs findings, can you look at those?


---
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] cloudstack pull request: CLOUDSTACK-8677: Call-home functionality ...

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

    https://github.com/apache/cloudstack/pull/625#discussion_r35605790
  
    --- Diff: reporter/usage-report-collector.py ---
    @@ -0,0 +1,64 @@
    +#!/usr/bin/env python
    +# 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.
    +
    +from flask import abort, Flask, request, Response
    +from elasticsearch import Elasticsearch
    +import json
    +import time
    +
    +def json_response(response):
    +    return json.dumps(response, indent=2) + "\n", 200, {'Content-Type': 'application/json; charset=utf-8'}
    +
    +def generate_app(config=None):
    +    app = Flask(__name__)
    +
    +    @app.route('/report/<unique_id>', methods=['POST'])
    +    def report(unique_id):
    +        # We expect JSON data, so if the Content-Type doesn't match JSON data we throw an error
    +        if 'Content-Type' in request.headers:
    +            if request.headers['Content-Type'] != 'application/json':
    +                abort(417, "No or incorrect Content-Type header was supplied")
    +
    +        index = "cloudstack-%s" % time.strftime("%Y.%m.%d", time.gmtime())
    +        timestamp = time.strftime("%Y-%m-%dT%H:%M:%SZ", time.gmtime())
    +
    +        es = Elasticsearch()
    +        es.indices.create(index=index, ignore=400)
    +
    +        report = json.loads(request.data)
    +        report["unique_id"] = unique_id
    +        report["timestamp"] = timestamp
    +
    +        es.index(index=index, doc_type="usage-report", body=json.dumps(report), timestamp=timestamp, refresh=True)
    +
    +        response = {}
    +        return json_response(response)
    +
    +    return app
    +
    +
    +app = generate_app()
    +
    +# Only run the App if this script is invoked from a Shell
    +if __name__ == '__main__':
    +    app.debug = True
    +    app.run(host='0.0.0.0', port=8088)
    +
    +# Otherwise provide a variable called 'application' for mod_wsgi
    +else:
    --- End diff --
    
    Where is this application var used? Its not global var as well?


---
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] cloudstack pull request: CLOUDSTACK-8677: Call-home functionality ...

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

    https://github.com/apache/cloudstack/pull/625#discussion_r35605842
  
    --- Diff: reporter/usage-report-collector.py ---
    @@ -0,0 +1,64 @@
    +#!/usr/bin/env python
    +# 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.
    +
    +from flask import abort, Flask, request, Response
    --- End diff --
    
    This is good work i would say, but just add some comments on this service purpose, may be for people seeing it later can understand it easily. Any reason to have this service written in python and other usage code to run along with cs in 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] cloudstack pull request: CLOUDSTACK-8677: Call-home functionality ...

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

    https://github.com/apache/cloudstack/pull/625#issuecomment-126674015
  
    I still don't know why travis fails. It fails on a storage part, but this commit doesn't touch anything from the storage. So I can't see why.


---
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] cloudstack pull request: CLOUDSTACK-8677: Call-home functionality ...

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

    https://github.com/apache/cloudstack/pull/625#discussion_r35633526
  
    --- Diff: server/src/org/apache/cloudstack/report/UsageReporter.java ---
    @@ -0,0 +1,487 @@
    +// 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.cloudstack.report;
    +
    +import java.util.concurrent.Executors;
    +import java.util.concurrent.ScheduledExecutorService;
    +import java.util.concurrent.TimeUnit;
    +import java.util.ArrayList;
    +import java.util.List;
    +import java.util.Map;
    +import java.util.HashMap;
    +import java.text.DateFormat;
    +import java.text.SimpleDateFormat;
    +import java.sql.Connection;
    +import java.sql.PreparedStatement;
    +import java.sql.ResultSet;
    +import java.sql.SQLException;
    +import java.net.URL;
    +import java.net.SocketTimeoutException;
    +import java.net.MalformedURLException;
    +import java.net.ProtocolException;
    +import java.net.UnknownHostException;
    +import java.io.OutputStreamWriter;
    +import java.io.IOException;
    +
    +import javax.inject.Inject;
    +import javax.net.ssl.HttpsURLConnection;
    +
    +import org.apache.log4j.Logger;
    +import org.springframework.stereotype.Component;
    +
    +import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
    +import org.apache.cloudstack.managed.context.ManagedContextRunnable;
    +
    +import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao;
    +import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
    +
    +import org.apache.commons.codec.digest.DigestUtils;
    +
    +import com.cloud.host.HostVO;
    +import com.cloud.host.dao.HostDao;
    +import com.cloud.dc.ClusterVO;
    +import com.cloud.dc.dao.ClusterDao;
    +import com.cloud.dc.DataCenterVO;
    +import com.cloud.dc.dao.DataCenterDao;
    +import com.cloud.vm.UserVmVO;
    +import com.cloud.vm.dao.UserVmDao;
    +import com.cloud.vm.VMInstanceVO;
    +import com.cloud.vm.dao.VMInstanceDao;
    +import com.cloud.utils.db.SearchCriteria;
    +import com.cloud.utils.NumbersUtil;
    +import com.cloud.utils.component.ManagerBase;
    +import com.cloud.utils.component.ComponentMethodInterceptable;
    +import com.cloud.utils.concurrency.NamedThreadFactory;
    +import com.cloud.utils.db.DB;
    +import com.cloud.utils.db.TransactionLegacy;
    +import com.cloud.upgrade.dao.VersionDao;
    +import com.cloud.upgrade.dao.VersionVO;
    +import com.cloud.storage.dao.DiskOfferingDao;
    +import com.cloud.storage.DiskOfferingVO;
    +import com.google.gson.Gson;
    +import com.google.gson.GsonBuilder;
    +import com.google.common.util.concurrent.AtomicLongMap;
    +
    +@Component
    +public class UsageReporter extends ManagerBase implements ComponentMethodInterceptable {
    +    public static final Logger s_logger = Logger.getLogger(UsageReporter.class.getName());
    +
    +    /* !FIX ME! This should point to a Apache Infra host with SSL! */
    +    private String reportHost = "https://call-home.cloudstack.org/report";
    +
    +    private String uniqueID = null;
    +
    +    private static UsageReporter s_instance = null;
    +
    +    private ScheduledExecutorService _executor = null;
    +
    +    @Inject
    +    private ConfigurationDao _configDao;
    +    @Inject
    +    private HostDao _hostDao;
    +    @Inject
    +    private ClusterDao _clusterDao;
    +    @Inject
    +    private PrimaryDataStoreDao _storagePoolDao;
    +    @Inject
    +    private DataCenterDao _dataCenterDao;
    +    @Inject
    +    private UserVmDao _userVmDao;
    +    @Inject
    +    private VMInstanceDao _vmInstance;
    +    @Inject
    +    private VersionDao _versionDao;
    +    @Inject
    +    private DiskOfferingDao _diskOfferingDao;
    +
    +    int usageReportInterval = -1;
    +
    +    public static UsageReporter getInstance() {
    +        return s_instance;
    +    }
    +
    +    public static UsageReporter getInstance(Map<String, String> configs) {
    +        s_instance.init(configs);
    +        return s_instance;
    +    }
    +
    +    public UsageReporter() {
    +        s_instance = this;
    +    }
    +
    +    @Override
    +    public boolean start() {
    +        init(_configDao.getConfiguration());
    +        return true;
    +    }
    +
    +    private void init(Map<String, String> configs) {
    +        _executor = Executors.newScheduledThreadPool(1, new NamedThreadFactory("UsageReporter"));
    +
    +        usageReportInterval = NumbersUtil.parseInt(configs.get("usage.report.interval"), 7);
    +
    +        if (usageReportInterval > 0) {
    +            _executor.scheduleWithFixedDelay(new UsageCollector(), 7, usageReportInterval, TimeUnit.DAYS);
    +        }
    +
    +        uniqueID = getUniqueId();
    +    }
    +
    +    private void sendReport(String reportUri, String uniqueID, Map<String, Object> reportMap) {
    +
    +        GsonBuilder builder = new GsonBuilder();
    +
    +        AtomicGsonAdapter adapter = new AtomicGsonAdapter();
    +        builder.registerTypeAdapter(AtomicLongMap.class, adapter);
    +
    +        Gson gson = builder.create();
    +        String report = gson.toJson(reportMap);
    +
    +        HttpsURLConnection conn = null;
    +        OutputStreamWriter osw = null;
    +
    +        int http_timeout = 15000;
    +
    +        try {
    +            s_logger.info("Usage Report will be send to: " + reportUri);
    +            s_logger.debug("REPORT: " + report);
    +
    +            URL url = new URL(reportUri + "/" + uniqueID);
    +
    +            conn = (HttpsURLConnection) url.openConnection();
    +            conn.setConnectTimeout(http_timeout);
    +            conn.setReadTimeout(http_timeout);
    +            conn.setRequestMethod("POST");
    +            conn.setDoOutput(true);
    +            conn.setRequestProperty("Content-Type", "application/json");
    +            conn.setRequestProperty("Accept", "application/json");
    +
    +            osw = new OutputStreamWriter(conn.getOutputStream());
    +            osw.write(report);
    +            osw.flush();
    +
    +            int resp_code = conn.getResponseCode();
    +
    +            if (resp_code == HttpsURLConnection.HTTP_OK){
    +                s_logger.info("Usage Report succesfully send to: " + reportUri);
    +            } else {
    +                s_logger.warn("Failed to send Usage Report: " + conn.getResponseMessage());
    +            }
    +
    +        } catch (UnknownHostException e) {
    +            s_logger.warn("Failed to look up Usage Report host: " + e.getMessage());
    +        } catch (SocketTimeoutException e) {
    +            s_logger.warn("Sending Usage Report to " + reportUri + " timed out: " + e.getMessage());
    +        } catch (MalformedURLException e) {
    +            s_logger.warn(reportUri + " is a invalid URL for sending Usage Report to: "+ e.getMessage());
    +        } catch (ProtocolException e) {
    +            s_logger.warn("Sending Usage Report failed due to a invalid protocol: " + e.getMessage());
    +        } catch (IOException e) {
    +            s_logger.warn("Failed to write Usage Report due to a IOException: " + e.getMessage());
    +        } finally {
    +            if (osw != null) {
    +                try {
    --- End diff --
    
    I would like to see:
    diff --git a/server/src/org/apache/cloudstack/report/UsageReporter.java b/server/src/org/apache/cloudstack/report/UsageReporter.java
    index 79b5ccf..3854653 100644
    --- a/server/src/org/apache/cloudstack/report/UsageReporter.java
    +++ b/server/src/org/apache/cloudstack/report/UsageReporter.java
    @@ -152,7 +152,6 @@
             String report = gson.toJson(reportMap);
     
             HttpsURLConnection conn = null;
    -        OutputStreamWriter osw = null;
     
             int http_timeout = 15000;
     
    @@ -170,10 +169,10 @@
                 conn.setRequestProperty("Content-Type", "application/json");
                 conn.setRequestProperty("Accept", "application/json");
     
    -            osw = new OutputStreamWriter(conn.getOutputStream());
    -            osw.write(report);
    -            osw.flush();
    -
    +            try (OutputStreamWriter osw = new OutputStreamWriter(conn.getOutputStream(), com.cloud.utils.StringUtils.getPreferredCharset());) {
    +                osw.write(report);
    +                osw.flush();
    +            }
                 int resp_code = conn.getResponseCode();
     
                 if (resp_code == HttpsURLConnection.HTTP_OK){
    @@ -193,14 +192,6 @@
             } catch (IOException e) {
                 s_logger.warn("Failed to write Usage Report due to a IOException: " + e.getMessage());
             } finally {
    -            if (osw != null) {
    -                try {
    -                    osw.close();
    -                } catch (IOException e) {
    -                    s_logger.debug("Failed to close output stream of HTTPS connection: " + e.getMessage());
    -                }
    -            }
    -
                 if (conn != null) {
                     conn.disconnect();
                 }


---
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] cloudstack pull request: CLOUDSTACK-8677: Call-home functionality ...

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

    https://github.com/apache/cloudstack/pull/625#discussion_r35605705
  
    --- Diff: server/src/org/apache/cloudstack/report/UsageReporter.java ---
    @@ -0,0 +1,487 @@
    +// 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.cloudstack.report;
    +
    +import java.util.concurrent.Executors;
    +import java.util.concurrent.ScheduledExecutorService;
    +import java.util.concurrent.TimeUnit;
    +import java.util.ArrayList;
    +import java.util.List;
    +import java.util.Map;
    +import java.util.HashMap;
    +import java.text.DateFormat;
    +import java.text.SimpleDateFormat;
    +import java.sql.Connection;
    +import java.sql.PreparedStatement;
    +import java.sql.ResultSet;
    +import java.sql.SQLException;
    +import java.net.URL;
    +import java.net.SocketTimeoutException;
    +import java.net.MalformedURLException;
    +import java.net.ProtocolException;
    +import java.net.UnknownHostException;
    +import java.io.OutputStreamWriter;
    +import java.io.IOException;
    +
    +import javax.inject.Inject;
    +import javax.net.ssl.HttpsURLConnection;
    +
    +import org.apache.log4j.Logger;
    +import org.springframework.stereotype.Component;
    +
    +import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
    +import org.apache.cloudstack.managed.context.ManagedContextRunnable;
    +
    +import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao;
    +import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
    +
    +import org.apache.commons.codec.digest.DigestUtils;
    +
    +import com.cloud.host.HostVO;
    +import com.cloud.host.dao.HostDao;
    +import com.cloud.dc.ClusterVO;
    +import com.cloud.dc.dao.ClusterDao;
    +import com.cloud.dc.DataCenterVO;
    +import com.cloud.dc.dao.DataCenterDao;
    +import com.cloud.vm.UserVmVO;
    +import com.cloud.vm.dao.UserVmDao;
    +import com.cloud.vm.VMInstanceVO;
    +import com.cloud.vm.dao.VMInstanceDao;
    +import com.cloud.utils.db.SearchCriteria;
    +import com.cloud.utils.NumbersUtil;
    +import com.cloud.utils.component.ManagerBase;
    +import com.cloud.utils.component.ComponentMethodInterceptable;
    +import com.cloud.utils.concurrency.NamedThreadFactory;
    +import com.cloud.utils.db.DB;
    +import com.cloud.utils.db.TransactionLegacy;
    +import com.cloud.upgrade.dao.VersionDao;
    +import com.cloud.upgrade.dao.VersionVO;
    +import com.cloud.storage.dao.DiskOfferingDao;
    +import com.cloud.storage.DiskOfferingVO;
    +import com.google.gson.Gson;
    +import com.google.gson.GsonBuilder;
    +import com.google.common.util.concurrent.AtomicLongMap;
    +
    +@Component
    +public class UsageReporter extends ManagerBase implements ComponentMethodInterceptable {
    +    public static final Logger s_logger = Logger.getLogger(UsageReporter.class.getName());
    +
    +    /* !FIX ME! This should point to a Apache Infra host with SSL! */
    +    private String reportHost = "https://call-home.cloudstack.org/report";
    +
    +    private String uniqueID = null;
    +
    +    private static UsageReporter s_instance = null;
    +
    +    private ScheduledExecutorService _executor = null;
    +
    +    @Inject
    +    private ConfigurationDao _configDao;
    +    @Inject
    +    private HostDao _hostDao;
    +    @Inject
    +    private ClusterDao _clusterDao;
    +    @Inject
    +    private PrimaryDataStoreDao _storagePoolDao;
    +    @Inject
    +    private DataCenterDao _dataCenterDao;
    +    @Inject
    +    private UserVmDao _userVmDao;
    +    @Inject
    +    private VMInstanceDao _vmInstance;
    +    @Inject
    +    private VersionDao _versionDao;
    +    @Inject
    +    private DiskOfferingDao _diskOfferingDao;
    +
    +    int usageReportInterval = -1;
    +
    +    public static UsageReporter getInstance() {
    --- End diff --
    
    It seems we are using a singleton pattern here, but it seems not thread safe?


---
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] cloudstack pull request: CLOUDSTACK-8677: Call-home functionality ...

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

    https://github.com/apache/cloudstack/pull/625#discussion_r35649303
  
    --- Diff: server/src/org/apache/cloudstack/report/UsageReporter.java ---
    @@ -0,0 +1,490 @@
    +// 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.cloudstack.report;
    +
    +import java.util.concurrent.Executors;
    +import java.util.concurrent.ScheduledExecutorService;
    +import java.util.concurrent.TimeUnit;
    +import java.util.ArrayList;
    +import java.util.List;
    +import java.util.Map;
    +import java.util.HashMap;
    +import java.text.DateFormat;
    +import java.text.SimpleDateFormat;
    +import java.sql.Connection;
    +import java.sql.PreparedStatement;
    +import java.sql.ResultSet;
    +import java.sql.SQLException;
    +import java.net.URL;
    +import java.net.SocketTimeoutException;
    +import java.net.MalformedURLException;
    +import java.net.ProtocolException;
    +import java.net.UnknownHostException;
    +import java.io.OutputStreamWriter;
    +import java.io.IOException;
    +
    +import javax.inject.Inject;
    +import javax.net.ssl.HttpsURLConnection;
    +
    +import org.apache.log4j.Logger;
    +import org.springframework.stereotype.Component;
    +
    +import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
    +import org.apache.cloudstack.managed.context.ManagedContextRunnable;
    +
    +import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao;
    +import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
    +
    +import org.apache.commons.codec.digest.DigestUtils;
    +
    +import com.cloud.host.HostVO;
    +import com.cloud.host.dao.HostDao;
    +import com.cloud.dc.ClusterVO;
    +import com.cloud.dc.dao.ClusterDao;
    +import com.cloud.dc.DataCenterVO;
    +import com.cloud.dc.dao.DataCenterDao;
    +import com.cloud.vm.UserVmVO;
    +import com.cloud.vm.dao.UserVmDao;
    +import com.cloud.vm.VMInstanceVO;
    +import com.cloud.vm.dao.VMInstanceDao;
    +import com.cloud.utils.db.SearchCriteria;
    +import com.cloud.utils.NumbersUtil;
    +import com.cloud.utils.component.ManagerBase;
    +import com.cloud.utils.component.ComponentMethodInterceptable;
    +import com.cloud.utils.concurrency.NamedThreadFactory;
    +import com.cloud.utils.db.DB;
    +import com.cloud.utils.db.TransactionLegacy;
    +import com.cloud.upgrade.dao.VersionDao;
    +import com.cloud.upgrade.dao.VersionVO;
    +import com.cloud.storage.dao.DiskOfferingDao;
    +import com.cloud.storage.DiskOfferingVO;
    +import com.google.gson.Gson;
    +import com.google.gson.GsonBuilder;
    +import com.google.common.util.concurrent.AtomicLongMap;
    +
    +@Component
    +public class UsageReporter extends ManagerBase implements ComponentMethodInterceptable {
    +    public static final Logger s_logger = Logger.getLogger(UsageReporter.class.getName());
    +
    +    /* !FIX ME! This should point to a Apache Infra host with SSL! */
    +    private String reportHost = "https://call-home.cloudstack.org/report";
    +
    +    private String uniqueID = null;
    +
    +    private static UsageReporter s_instance = null;
    +
    +    private ScheduledExecutorService _executor = null;
    +
    +    @Inject
    +    private ConfigurationDao _configDao;
    +    @Inject
    +    private HostDao _hostDao;
    +    @Inject
    +    private ClusterDao _clusterDao;
    +    @Inject
    +    private PrimaryDataStoreDao _storagePoolDao;
    +    @Inject
    +    private DataCenterDao _dataCenterDao;
    +    @Inject
    +    private UserVmDao _userVmDao;
    +    @Inject
    +    private VMInstanceDao _vmInstance;
    +    @Inject
    +    private VersionDao _versionDao;
    +    @Inject
    +    private DiskOfferingDao _diskOfferingDao;
    +
    +    int usageReportInterval = -1;
    +
    +    public static UsageReporter getInstance(Map<String, String> configs) {
    +        if (s_instance == null) {
    +            s_instance = new UsageReporter();
    +            s_instance.init(configs);
    --- End diff --
    
    so findbugs doesn't like my solution either and it is right ;)
               new_instance = new UsageReporter();
               new_instance.init(configs);
               s_instance = new_instance;
    would be better. maybe even in a synchronised block to prevent to synchronous call to getInstance resulting in two instances of a singleton present.


---
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] cloudstack pull request: CLOUDSTACK-8677: Call-home functionality ...

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

    https://github.com/apache/cloudstack/pull/625#issuecomment-134922676
  
    @wido can you implement this as a plugin instead of core feature? Also do we want cloudstack to report statistics on each run; I suggest to have it as a plugin with a small db entry (global setting or a separate db) to track the last time+version we sent the payload/report and we can report that on each upgrade and perform checks every time cloudstack runs. This also needs to have a global lock in case of multiple mgmt servers.


---
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] cloudstack pull request: CLOUDSTACK-8677: Call-home functionality ...

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

    https://github.com/apache/cloudstack/pull/625#discussion_r35535873
  
    --- Diff: server/src/org/apache/cloudstack/report/UsageReporter.java ---
    @@ -0,0 +1,473 @@
    +// 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.cloudstack.report;
    +
    +import java.util.concurrent.Executors;
    +import java.util.concurrent.ScheduledExecutorService;
    +import java.util.concurrent.TimeUnit;
    +import java.util.ArrayList;
    +import java.util.List;
    +import java.util.Map;
    +import java.util.HashMap;
    +import java.text.DateFormat;
    +import java.text.SimpleDateFormat;
    +import java.sql.Connection;
    +import java.sql.PreparedStatement;
    +import java.sql.ResultSet;
    +import java.sql.SQLException;
    +import java.net.URL;
    +import java.net.SocketTimeoutException;
    +import java.net.MalformedURLException;
    +import java.net.ProtocolException;
    +import java.net.UnknownHostException;
    +import java.io.OutputStreamWriter;
    +import java.io.IOException;
    +
    +import javax.inject.Inject;
    +import javax.net.ssl.HttpsURLConnection;
    +
    +import org.apache.log4j.Logger;
    +import org.springframework.stereotype.Component;
    +
    +import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
    +import org.apache.cloudstack.managed.context.ManagedContextRunnable;
    +
    +import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao;
    +import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
    +
    +import org.apache.commons.codec.digest.DigestUtils;
    +
    +import com.cloud.host.HostVO;
    +import com.cloud.host.dao.HostDao;
    +import com.cloud.dc.ClusterVO;
    +import com.cloud.dc.dao.ClusterDao;
    +import com.cloud.dc.DataCenterVO;
    +import com.cloud.dc.dao.DataCenterDao;
    +import com.cloud.vm.UserVmVO;
    +import com.cloud.vm.dao.UserVmDao;
    +import com.cloud.vm.VMInstanceVO;
    +import com.cloud.vm.dao.VMInstanceDao;
    +import com.cloud.utils.db.SearchCriteria;
    +import com.cloud.utils.NumbersUtil;
    +import com.cloud.utils.component.ManagerBase;
    +import com.cloud.utils.component.ComponentMethodInterceptable;
    +import com.cloud.utils.concurrency.NamedThreadFactory;
    +import com.cloud.utils.db.DB;
    +import com.cloud.utils.db.TransactionLegacy;
    +import com.cloud.upgrade.dao.VersionDao;
    +import com.cloud.upgrade.dao.VersionVO;
    +import com.cloud.storage.dao.DiskOfferingDao;
    +import com.cloud.storage.DiskOfferingVO;
    +import com.google.gson.Gson;
    +import com.google.gson.GsonBuilder;
    +import com.google.common.util.concurrent.AtomicLongMap;
    +
    +@Component
    +public class UsageReporter extends ManagerBase implements ComponentMethodInterceptable {
    +    public static final Logger s_logger = Logger.getLogger(UsageReporter.class.getName());
    +
    +    /* !FIX ME! This should point to a Apache Infra host with SSL! */
    +    private String reportHost = "https://call-home.cloudstack.org/report";
    +
    +    private String uniqueID = null;
    +
    +    private static UsageReporter s_instance = null;
    +
    +    private ScheduledExecutorService _executor = null;
    +
    +    @Inject
    +    private ConfigurationDao _configDao;
    +    @Inject
    +    private HostDao _hostDao;
    +    @Inject
    +    private ClusterDao _clusterDao;
    +    @Inject
    +    private PrimaryDataStoreDao _storagePoolDao;
    +    @Inject
    +    private DataCenterDao _dataCenterDao;
    +    @Inject
    +    private UserVmDao _userVmDao;
    +    @Inject
    +    private VMInstanceDao _vmInstance;
    +    @Inject
    +    private VersionDao _versionDao;
    +    @Inject
    +    private DiskOfferingDao _diskOfferingDao;
    +
    +    int usageReportInterval = -1;
    +
    +    public static UsageReporter getInstance() {
    +        return s_instance;
    +    }
    +
    +    public static UsageReporter getInstance(Map<String, String> configs) {
    +        s_instance.init(configs);
    +        return s_instance;
    +    }
    +
    +    public UsageReporter() {
    +        s_instance = this;
    +    }
    +
    +    @Override
    +    public boolean start() {
    +        init(_configDao.getConfiguration());
    +        return true;
    +    }
    +
    +    private void init(Map<String, String> configs) {
    +        _executor = Executors.newScheduledThreadPool(1, new NamedThreadFactory("UsageReporter"));
    +
    +        usageReportInterval = NumbersUtil.parseInt(configs.get("usage.report.interval"), 7);
    +
    +        if (usageReportInterval > 0) {
    +            _executor.scheduleWithFixedDelay(new UsageCollector(), 7, usageReportInterval, TimeUnit.DAYS);
    +        }
    +
    +        uniqueID = getUniqueId();
    +    }
    +
    +    private void sendReport(String reportUri, String uniqueID, Map<String, Object> reportMap) {
    +
    +        GsonBuilder builder = new GsonBuilder();
    +
    +        AtomicGsonAdapter adapter = new AtomicGsonAdapter();
    +        builder.registerTypeAdapter(AtomicLongMap.class, adapter);
    +
    +        Gson gson = builder.create();
    +        String report = gson.toJson(reportMap);
    +
    +        int http_timeout = 15000;
    +
    +        try {
    +            s_logger.info("Usage Report will be send to: " + reportUri);
    +            s_logger.debug("REPORT: " + report);
    +
    +            URL url = new URL(reportUri + "/" + uniqueID);
    +
    +            HttpsURLConnection conn = (HttpsURLConnection) url.openConnection();
    --- End diff --
    
    Good point, added that in the 2nd commit


---
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] cloudstack pull request: CLOUDSTACK-8677: Call-home functionality ...

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

    https://github.com/apache/cloudstack/pull/625#discussion_r35625832
  
    --- Diff: reporter/usage-report-collector.py ---
    @@ -0,0 +1,64 @@
    +#!/usr/bin/env python
    +# 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.
    +
    +from flask import abort, Flask, request, Response
    +from elasticsearch import Elasticsearch
    +import json
    +import time
    +
    +def json_response(response):
    +    return json.dumps(response, indent=2) + "\n", 200, {'Content-Type': 'application/json; charset=utf-8'}
    +
    +def generate_app(config=None):
    +    app = Flask(__name__)
    +
    +    @app.route('/report/<unique_id>', methods=['POST'])
    +    def report(unique_id):
    +        # We expect JSON data, so if the Content-Type doesn't match JSON data we throw an error
    +        if 'Content-Type' in request.headers:
    +            if request.headers['Content-Type'] != 'application/json':
    +                abort(417, "No or incorrect Content-Type header was supplied")
    +
    +        index = "cloudstack-%s" % time.strftime("%Y.%m.%d", time.gmtime())
    +        timestamp = time.strftime("%Y-%m-%dT%H:%M:%SZ", time.gmtime())
    +
    +        es = Elasticsearch()
    +        es.indices.create(index=index, ignore=400)
    +
    +        report = json.loads(request.data)
    +        report["unique_id"] = unique_id
    +        report["timestamp"] = timestamp
    +
    +        es.index(index=index, doc_type="usage-report", body=json.dumps(report), timestamp=timestamp, refresh=True)
    +
    +        response = {}
    +        return json_response(response)
    +
    +    return app
    +
    +
    +app = generate_app()
    +
    +# Only run the App if this script is invoked from a Shell
    +if __name__ == '__main__':
    +    app.debug = True
    +    app.run(host='0.0.0.0', port=8088)
    +
    +# Otherwise provide a variable called 'application' for mod_wsgi
    +else:
    --- End diff --
    
    It is for mod_wsgi. It will read "app" and run the application from there


---
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] cloudstack pull request: CLOUDSTACK-8677: Call-home functionality ...

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

    https://github.com/apache/cloudstack/pull/625#discussion_r35626289
  
    --- Diff: server/src/org/apache/cloudstack/report/UsageReporter.java ---
    @@ -0,0 +1,487 @@
    +// 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.cloudstack.report;
    +
    +import java.util.concurrent.Executors;
    +import java.util.concurrent.ScheduledExecutorService;
    +import java.util.concurrent.TimeUnit;
    +import java.util.ArrayList;
    +import java.util.List;
    +import java.util.Map;
    +import java.util.HashMap;
    +import java.text.DateFormat;
    +import java.text.SimpleDateFormat;
    +import java.sql.Connection;
    +import java.sql.PreparedStatement;
    +import java.sql.ResultSet;
    +import java.sql.SQLException;
    +import java.net.URL;
    +import java.net.SocketTimeoutException;
    +import java.net.MalformedURLException;
    +import java.net.ProtocolException;
    +import java.net.UnknownHostException;
    +import java.io.OutputStreamWriter;
    +import java.io.IOException;
    +
    +import javax.inject.Inject;
    +import javax.net.ssl.HttpsURLConnection;
    +
    +import org.apache.log4j.Logger;
    +import org.springframework.stereotype.Component;
    +
    +import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
    +import org.apache.cloudstack.managed.context.ManagedContextRunnable;
    +
    +import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao;
    +import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
    +
    +import org.apache.commons.codec.digest.DigestUtils;
    +
    +import com.cloud.host.HostVO;
    +import com.cloud.host.dao.HostDao;
    +import com.cloud.dc.ClusterVO;
    +import com.cloud.dc.dao.ClusterDao;
    +import com.cloud.dc.DataCenterVO;
    +import com.cloud.dc.dao.DataCenterDao;
    +import com.cloud.vm.UserVmVO;
    +import com.cloud.vm.dao.UserVmDao;
    +import com.cloud.vm.VMInstanceVO;
    +import com.cloud.vm.dao.VMInstanceDao;
    +import com.cloud.utils.db.SearchCriteria;
    +import com.cloud.utils.NumbersUtil;
    +import com.cloud.utils.component.ManagerBase;
    +import com.cloud.utils.component.ComponentMethodInterceptable;
    +import com.cloud.utils.concurrency.NamedThreadFactory;
    +import com.cloud.utils.db.DB;
    +import com.cloud.utils.db.TransactionLegacy;
    +import com.cloud.upgrade.dao.VersionDao;
    +import com.cloud.upgrade.dao.VersionVO;
    +import com.cloud.storage.dao.DiskOfferingDao;
    +import com.cloud.storage.DiskOfferingVO;
    +import com.google.gson.Gson;
    +import com.google.gson.GsonBuilder;
    +import com.google.common.util.concurrent.AtomicLongMap;
    +
    +@Component
    +public class UsageReporter extends ManagerBase implements ComponentMethodInterceptable {
    +    public static final Logger s_logger = Logger.getLogger(UsageReporter.class.getName());
    +
    +    /* !FIX ME! This should point to a Apache Infra host with SSL! */
    +    private String reportHost = "https://call-home.cloudstack.org/report";
    +
    +    private String uniqueID = null;
    +
    +    private static UsageReporter s_instance = null;
    +
    +    private ScheduledExecutorService _executor = null;
    +
    +    @Inject
    +    private ConfigurationDao _configDao;
    +    @Inject
    +    private HostDao _hostDao;
    +    @Inject
    +    private ClusterDao _clusterDao;
    +    @Inject
    +    private PrimaryDataStoreDao _storagePoolDao;
    +    @Inject
    +    private DataCenterDao _dataCenterDao;
    +    @Inject
    +    private UserVmDao _userVmDao;
    +    @Inject
    +    private VMInstanceDao _vmInstance;
    +    @Inject
    +    private VersionDao _versionDao;
    +    @Inject
    +    private DiskOfferingDao _diskOfferingDao;
    +
    +    int usageReportInterval = -1;
    +
    +    public static UsageReporter getInstance() {
    +        return s_instance;
    +    }
    +
    +    public static UsageReporter getInstance(Map<String, String> configs) {
    +        s_instance.init(configs);
    +        return s_instance;
    +    }
    +
    +    public UsageReporter() {
    +        s_instance = this;
    --- End diff --
    
    A public (instance) constructor writes to a static variable. I think you can make the constructor private and assign the static from getInstance(). First check for null and then assign if needed or return the existing object.
    
    public static UsageReporter getInstance(Map<String, String> configs) {
        if (s_instance == null) {
            s_instance = new UsageReporter(); //make sure it is private so it can only be called from here
            s_instance.init(configs);
        }
        return s_instance;
    }


---
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] cloudstack pull request: CLOUDSTACK-8677: Call-home functionality ...

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

    https://github.com/apache/cloudstack/pull/625#issuecomment-132254920
  
    This will be turned ON by default ?
    I think you should bring this up on the list, at least to give a heads up and an official public record.


---
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] cloudstack pull request: CLOUDSTACK-8677: Call-home functionality ...

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

    https://github.com/apache/cloudstack/pull/625#discussion_r35627322
  
    --- Diff: server/src/org/apache/cloudstack/report/UsageReporter.java ---
    @@ -0,0 +1,487 @@
    +// 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.cloudstack.report;
    +
    +import java.util.concurrent.Executors;
    +import java.util.concurrent.ScheduledExecutorService;
    +import java.util.concurrent.TimeUnit;
    +import java.util.ArrayList;
    +import java.util.List;
    +import java.util.Map;
    +import java.util.HashMap;
    +import java.text.DateFormat;
    +import java.text.SimpleDateFormat;
    +import java.sql.Connection;
    +import java.sql.PreparedStatement;
    +import java.sql.ResultSet;
    +import java.sql.SQLException;
    +import java.net.URL;
    +import java.net.SocketTimeoutException;
    +import java.net.MalformedURLException;
    +import java.net.ProtocolException;
    +import java.net.UnknownHostException;
    +import java.io.OutputStreamWriter;
    +import java.io.IOException;
    +
    +import javax.inject.Inject;
    +import javax.net.ssl.HttpsURLConnection;
    +
    +import org.apache.log4j.Logger;
    +import org.springframework.stereotype.Component;
    +
    +import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
    +import org.apache.cloudstack.managed.context.ManagedContextRunnable;
    +
    +import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao;
    +import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
    +
    +import org.apache.commons.codec.digest.DigestUtils;
    +
    +import com.cloud.host.HostVO;
    +import com.cloud.host.dao.HostDao;
    +import com.cloud.dc.ClusterVO;
    +import com.cloud.dc.dao.ClusterDao;
    +import com.cloud.dc.DataCenterVO;
    +import com.cloud.dc.dao.DataCenterDao;
    +import com.cloud.vm.UserVmVO;
    +import com.cloud.vm.dao.UserVmDao;
    +import com.cloud.vm.VMInstanceVO;
    +import com.cloud.vm.dao.VMInstanceDao;
    +import com.cloud.utils.db.SearchCriteria;
    +import com.cloud.utils.NumbersUtil;
    +import com.cloud.utils.component.ManagerBase;
    +import com.cloud.utils.component.ComponentMethodInterceptable;
    +import com.cloud.utils.concurrency.NamedThreadFactory;
    +import com.cloud.utils.db.DB;
    +import com.cloud.utils.db.TransactionLegacy;
    +import com.cloud.upgrade.dao.VersionDao;
    +import com.cloud.upgrade.dao.VersionVO;
    +import com.cloud.storage.dao.DiskOfferingDao;
    +import com.cloud.storage.DiskOfferingVO;
    +import com.google.gson.Gson;
    +import com.google.gson.GsonBuilder;
    +import com.google.common.util.concurrent.AtomicLongMap;
    +
    +@Component
    +public class UsageReporter extends ManagerBase implements ComponentMethodInterceptable {
    +    public static final Logger s_logger = Logger.getLogger(UsageReporter.class.getName());
    +
    +    /* !FIX ME! This should point to a Apache Infra host with SSL! */
    +    private String reportHost = "https://call-home.cloudstack.org/report";
    +
    +    private String uniqueID = null;
    +
    +    private static UsageReporter s_instance = null;
    +
    +    private ScheduledExecutorService _executor = null;
    +
    +    @Inject
    +    private ConfigurationDao _configDao;
    +    @Inject
    +    private HostDao _hostDao;
    +    @Inject
    +    private ClusterDao _clusterDao;
    +    @Inject
    +    private PrimaryDataStoreDao _storagePoolDao;
    +    @Inject
    +    private DataCenterDao _dataCenterDao;
    +    @Inject
    +    private UserVmDao _userVmDao;
    +    @Inject
    +    private VMInstanceDao _vmInstance;
    +    @Inject
    +    private VersionDao _versionDao;
    +    @Inject
    +    private DiskOfferingDao _diskOfferingDao;
    +
    +    int usageReportInterval = -1;
    +
    +    public static UsageReporter getInstance() {
    --- End diff --
    
    It seems the purpose is to make only one instance of usagereporter available per process and so we added getInstance() method, where user calls the getInstance() to get the object reference, in that scenario, 
    1. Make the constructor private. create the  object only when s_instance is null. Otherwise, s_instance will be new reference with each new object creation.
    2. It seems we have two overloaded variants of getInstance, not sure which one is called and when to use?
    2. Then, if getInstance is called to get an instance of UsageReporter and is static, do we want to use lock getinstance code to make it critical section, synchronize and avoid race conditions, if multiple threads are calling getInstance at the same time, its like two threads using getInstance at the sametime first time to create an object.


---
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] cloudstack pull request: CLOUDSTACK-8677: Call-home functionality ...

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

    https://github.com/apache/cloudstack/pull/625#discussion_r35626976
  
    --- Diff: server/src/org/apache/cloudstack/report/UsageReporter.java ---
    @@ -0,0 +1,487 @@
    +// 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.cloudstack.report;
    +
    +import java.util.concurrent.Executors;
    +import java.util.concurrent.ScheduledExecutorService;
    +import java.util.concurrent.TimeUnit;
    +import java.util.ArrayList;
    +import java.util.List;
    +import java.util.Map;
    +import java.util.HashMap;
    +import java.text.DateFormat;
    +import java.text.SimpleDateFormat;
    +import java.sql.Connection;
    +import java.sql.PreparedStatement;
    +import java.sql.ResultSet;
    +import java.sql.SQLException;
    +import java.net.URL;
    +import java.net.SocketTimeoutException;
    +import java.net.MalformedURLException;
    +import java.net.ProtocolException;
    +import java.net.UnknownHostException;
    +import java.io.OutputStreamWriter;
    +import java.io.IOException;
    +
    +import javax.inject.Inject;
    +import javax.net.ssl.HttpsURLConnection;
    +
    +import org.apache.log4j.Logger;
    +import org.springframework.stereotype.Component;
    +
    +import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
    +import org.apache.cloudstack.managed.context.ManagedContextRunnable;
    +
    +import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao;
    +import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
    +
    +import org.apache.commons.codec.digest.DigestUtils;
    +
    +import com.cloud.host.HostVO;
    +import com.cloud.host.dao.HostDao;
    +import com.cloud.dc.ClusterVO;
    +import com.cloud.dc.dao.ClusterDao;
    +import com.cloud.dc.DataCenterVO;
    +import com.cloud.dc.dao.DataCenterDao;
    +import com.cloud.vm.UserVmVO;
    +import com.cloud.vm.dao.UserVmDao;
    +import com.cloud.vm.VMInstanceVO;
    +import com.cloud.vm.dao.VMInstanceDao;
    +import com.cloud.utils.db.SearchCriteria;
    +import com.cloud.utils.NumbersUtil;
    +import com.cloud.utils.component.ManagerBase;
    +import com.cloud.utils.component.ComponentMethodInterceptable;
    +import com.cloud.utils.concurrency.NamedThreadFactory;
    +import com.cloud.utils.db.DB;
    +import com.cloud.utils.db.TransactionLegacy;
    +import com.cloud.upgrade.dao.VersionDao;
    +import com.cloud.upgrade.dao.VersionVO;
    +import com.cloud.storage.dao.DiskOfferingDao;
    +import com.cloud.storage.DiskOfferingVO;
    +import com.google.gson.Gson;
    +import com.google.gson.GsonBuilder;
    +import com.google.common.util.concurrent.AtomicLongMap;
    +
    +@Component
    +public class UsageReporter extends ManagerBase implements ComponentMethodInterceptable {
    +    public static final Logger s_logger = Logger.getLogger(UsageReporter.class.getName());
    +
    +    /* !FIX ME! This should point to a Apache Infra host with SSL! */
    +    private String reportHost = "https://call-home.cloudstack.org/report";
    +
    +    private String uniqueID = null;
    +
    +    private static UsageReporter s_instance = null;
    +
    +    private ScheduledExecutorService _executor = null;
    +
    +    @Inject
    +    private ConfigurationDao _configDao;
    +    @Inject
    +    private HostDao _hostDao;
    +    @Inject
    +    private ClusterDao _clusterDao;
    +    @Inject
    +    private PrimaryDataStoreDao _storagePoolDao;
    +    @Inject
    +    private DataCenterDao _dataCenterDao;
    +    @Inject
    +    private UserVmDao _userVmDao;
    +    @Inject
    +    private VMInstanceDao _vmInstance;
    +    @Inject
    +    private VersionDao _versionDao;
    +    @Inject
    +    private DiskOfferingDao _diskOfferingDao;
    +
    +    int usageReportInterval = -1;
    +
    +    public static UsageReporter getInstance() {
    +        return s_instance;
    +    }
    +
    +    public static UsageReporter getInstance(Map<String, String> configs) {
    +        s_instance.init(configs);
    +        return s_instance;
    +    }
    +
    +    public UsageReporter() {
    +        s_instance = this;
    +    }
    +
    +    @Override
    +    public boolean start() {
    +        init(_configDao.getConfiguration());
    +        return true;
    +    }
    +
    +    private void init(Map<String, String> configs) {
    +        _executor = Executors.newScheduledThreadPool(1, new NamedThreadFactory("UsageReporter"));
    +
    +        usageReportInterval = NumbersUtil.parseInt(configs.get("usage.report.interval"), 7);
    +
    +        if (usageReportInterval > 0) {
    +            _executor.scheduleWithFixedDelay(new UsageCollector(), 7, usageReportInterval, TimeUnit.DAYS);
    +        }
    +
    +        uniqueID = getUniqueId();
    +    }
    +
    +    private void sendReport(String reportUri, String uniqueID, Map<String, Object> reportMap) {
    +
    +        GsonBuilder builder = new GsonBuilder();
    +
    +        AtomicGsonAdapter adapter = new AtomicGsonAdapter();
    +        builder.registerTypeAdapter(AtomicLongMap.class, adapter);
    +
    +        Gson gson = builder.create();
    +        String report = gson.toJson(reportMap);
    +
    +        HttpsURLConnection conn = null;
    +        OutputStreamWriter osw = null;
    +
    +        int http_timeout = 15000;
    +
    +        try {
    +            s_logger.info("Usage Report will be send to: " + reportUri);
    +            s_logger.debug("REPORT: " + report);
    +
    +            URL url = new URL(reportUri + "/" + uniqueID);
    +
    +            conn = (HttpsURLConnection) url.openConnection();
    +            conn.setConnectTimeout(http_timeout);
    +            conn.setReadTimeout(http_timeout);
    +            conn.setRequestMethod("POST");
    +            conn.setDoOutput(true);
    +            conn.setRequestProperty("Content-Type", "application/json");
    +            conn.setRequestProperty("Accept", "application/json");
    +
    +            osw = new OutputStreamWriter(conn.getOutputStream());
    --- End diff --
    
    osw = new OutputStreamWriter(conn.getOutputStream(), com.cloud.utils.StringUtils.getPreferredCharset());


---
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] cloudstack pull request: CLOUDSTACK-8677: Call-home functionality ...

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

    https://github.com/apache/cloudstack/pull/625#issuecomment-126684715
  
    @DaanHoogland I see, but I'm not even getting near that code. So it's unclear to me why this PR seems unstable.


---
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] cloudstack pull request: CLOUDSTACK-8677: Call-home functionality ...

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

    https://github.com/apache/cloudstack/pull/625#issuecomment-132472682
  
    @runseb Yes, indeed. But the PR is not ready to merge yet, it's blocked by a GSON issue which I don't know how to resolve.
    
    This is what @DaanHoogland found out:
    
    "the gson issue is with primitive types. support was explicitely dropped and these are used in the tests. We should make sure they are not used in the real code and adjust the test. (ref  PR625 and CLOUDSTACK-8708)"
    
    But yes, this should go on the ML and very, very clearly in the Release Notes.


---
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] cloudstack pull request: CLOUDSTACK-8677: Call-home functionality ...

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

    https://github.com/apache/cloudstack/pull/625#issuecomment-125505835
  
    @DaanHoogland I will. Although I don't understand why these are happening


---
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] cloudstack pull request: CLOUDSTACK-8677: Call-home functionality ...

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

    https://github.com/apache/cloudstack/pull/625#issuecomment-126677074
  
    @wido not sure if these are related but please have a look at https://builds.apache.org/job/cloudstack-pull-analysis/83/org.apache.cloudstack$cloud-core/testReport/
    it contains some unit test failure reports. I don't think you touched this directly either.


---
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] cloudstack pull request: CLOUDSTACK-8677: Call-home functionality ...

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

    https://github.com/apache/cloudstack/pull/625#issuecomment-127377070
  
    I just don't know why the tests keep failing on this one. They seem to fail on code not touched by this PR at all. Really want to get this in to 4.6.


---
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] cloudstack pull request: CLOUDSTACK-8677: Call-home functionality ...

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

    https://github.com/apache/cloudstack/pull/625#discussion_r35606107
  
    --- Diff: server/src/org/apache/cloudstack/report/UsageReporter.java ---
    @@ -0,0 +1,487 @@
    +// 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.cloudstack.report;
    +
    +import java.util.concurrent.Executors;
    +import java.util.concurrent.ScheduledExecutorService;
    +import java.util.concurrent.TimeUnit;
    +import java.util.ArrayList;
    +import java.util.List;
    +import java.util.Map;
    +import java.util.HashMap;
    +import java.text.DateFormat;
    +import java.text.SimpleDateFormat;
    +import java.sql.Connection;
    +import java.sql.PreparedStatement;
    +import java.sql.ResultSet;
    +import java.sql.SQLException;
    +import java.net.URL;
    +import java.net.SocketTimeoutException;
    +import java.net.MalformedURLException;
    +import java.net.ProtocolException;
    +import java.net.UnknownHostException;
    +import java.io.OutputStreamWriter;
    +import java.io.IOException;
    +
    +import javax.inject.Inject;
    +import javax.net.ssl.HttpsURLConnection;
    +
    +import org.apache.log4j.Logger;
    +import org.springframework.stereotype.Component;
    +
    +import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
    +import org.apache.cloudstack.managed.context.ManagedContextRunnable;
    +
    +import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao;
    +import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
    +
    +import org.apache.commons.codec.digest.DigestUtils;
    +
    +import com.cloud.host.HostVO;
    +import com.cloud.host.dao.HostDao;
    +import com.cloud.dc.ClusterVO;
    +import com.cloud.dc.dao.ClusterDao;
    +import com.cloud.dc.DataCenterVO;
    +import com.cloud.dc.dao.DataCenterDao;
    +import com.cloud.vm.UserVmVO;
    +import com.cloud.vm.dao.UserVmDao;
    +import com.cloud.vm.VMInstanceVO;
    +import com.cloud.vm.dao.VMInstanceDao;
    +import com.cloud.utils.db.SearchCriteria;
    +import com.cloud.utils.NumbersUtil;
    +import com.cloud.utils.component.ManagerBase;
    +import com.cloud.utils.component.ComponentMethodInterceptable;
    +import com.cloud.utils.concurrency.NamedThreadFactory;
    +import com.cloud.utils.db.DB;
    +import com.cloud.utils.db.TransactionLegacy;
    +import com.cloud.upgrade.dao.VersionDao;
    +import com.cloud.upgrade.dao.VersionVO;
    +import com.cloud.storage.dao.DiskOfferingDao;
    +import com.cloud.storage.DiskOfferingVO;
    +import com.google.gson.Gson;
    +import com.google.gson.GsonBuilder;
    +import com.google.common.util.concurrent.AtomicLongMap;
    +
    +@Component
    +public class UsageReporter extends ManagerBase implements ComponentMethodInterceptable {
    +    public static final Logger s_logger = Logger.getLogger(UsageReporter.class.getName());
    +
    +    /* !FIX ME! This should point to a Apache Infra host with SSL! */
    +    private String reportHost = "https://call-home.cloudstack.org/report";
    +
    +    private String uniqueID = null;
    +
    +    private static UsageReporter s_instance = null;
    +
    +    private ScheduledExecutorService _executor = null;
    +
    +    @Inject
    +    private ConfigurationDao _configDao;
    +    @Inject
    +    private HostDao _hostDao;
    +    @Inject
    +    private ClusterDao _clusterDao;
    +    @Inject
    +    private PrimaryDataStoreDao _storagePoolDao;
    +    @Inject
    +    private DataCenterDao _dataCenterDao;
    +    @Inject
    +    private UserVmDao _userVmDao;
    +    @Inject
    +    private VMInstanceDao _vmInstance;
    +    @Inject
    +    private VersionDao _versionDao;
    +    @Inject
    +    private DiskOfferingDao _diskOfferingDao;
    +
    +    int usageReportInterval = -1;
    +
    +    public static UsageReporter getInstance() {
    +        return s_instance;
    +    }
    +
    +    public static UsageReporter getInstance(Map<String, String> configs) {
    +        s_instance.init(configs);
    +        return s_instance;
    +    }
    +
    +    public UsageReporter() {
    +        s_instance = this;
    +    }
    +
    +    @Override
    +    public boolean start() {
    +        init(_configDao.getConfiguration());
    +        return true;
    +    }
    +
    +    private void init(Map<String, String> configs) {
    +        _executor = Executors.newScheduledThreadPool(1, new NamedThreadFactory("UsageReporter"));
    +
    +        usageReportInterval = NumbersUtil.parseInt(configs.get("usage.report.interval"), 7);
    +
    +        if (usageReportInterval > 0) {
    +            _executor.scheduleWithFixedDelay(new UsageCollector(), 7, usageReportInterval, TimeUnit.DAYS);
    +        }
    +
    +        uniqueID = getUniqueId();
    +    }
    +
    +    private void sendReport(String reportUri, String uniqueID, Map<String, Object> reportMap) {
    +
    +        GsonBuilder builder = new GsonBuilder();
    +
    +        AtomicGsonAdapter adapter = new AtomicGsonAdapter();
    +        builder.registerTypeAdapter(AtomicLongMap.class, adapter);
    +
    +        Gson gson = builder.create();
    +        String report = gson.toJson(reportMap);
    +
    +        HttpsURLConnection conn = null;
    +        OutputStreamWriter osw = null;
    +
    +        int http_timeout = 15000;
    +
    +        try {
    +            s_logger.info("Usage Report will be send to: " + reportUri);
    +            s_logger.debug("REPORT: " + report);
    +
    +            URL url = new URL(reportUri + "/" + uniqueID);
    +
    +            conn = (HttpsURLConnection) url.openConnection();
    +            conn.setConnectTimeout(http_timeout);
    +            conn.setReadTimeout(http_timeout);
    +            conn.setRequestMethod("POST");
    +            conn.setDoOutput(true);
    +            conn.setRequestProperty("Content-Type", "application/json");
    +            conn.setRequestProperty("Accept", "application/json");
    +
    +            osw = new OutputStreamWriter(conn.getOutputStream());
    +            osw.write(report);
    +            osw.flush();
    +
    +            int resp_code = conn.getResponseCode();
    +
    +            if (resp_code == HttpsURLConnection.HTTP_OK){
    +                s_logger.info("Usage Report succesfully send to: " + reportUri);
    +            } else {
    +                s_logger.warn("Failed to send Usage Report: " + conn.getResponseMessage());
    +            }
    +
    +        } catch (UnknownHostException e) {
    +            s_logger.warn("Failed to look up Usage Report host: " + e.getMessage());
    +        } catch (SocketTimeoutException e) {
    +            s_logger.warn("Sending Usage Report to " + reportUri + " timed out: " + e.getMessage());
    +        } catch (MalformedURLException e) {
    +            s_logger.warn(reportUri + " is a invalid URL for sending Usage Report to: "+ e.getMessage());
    +        } catch (ProtocolException e) {
    +            s_logger.warn("Sending Usage Report failed due to a invalid protocol: " + e.getMessage());
    +        } catch (IOException e) {
    +            s_logger.warn("Failed to write Usage Report due to a IOException: " + e.getMessage());
    +        } finally {
    +            if (osw != null) {
    +                try {
    +                    osw.close();
    +                } catch (IOException e) {
    +                    s_logger.debug("Failed to close output stream of HTTPS connection: " + e.getMessage());
    +                }
    +            }
    +
    +            if (conn != null) {
    +                conn.disconnect();
    --- End diff --
    
    Just to be on safer side, to confirm for all, will it also flush out the resources and close, sometimes resources are not flushed and staled till some connection objects are explicitly closed.


---
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] cloudstack pull request: CLOUDSTACK-8677: Call-home functionality ...

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

    https://github.com/apache/cloudstack/pull/625#discussion_r35627154
  
    --- Diff: server/src/org/apache/cloudstack/report/UsageReporter.java ---
    @@ -0,0 +1,487 @@
    +// 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.cloudstack.report;
    +
    +import java.util.concurrent.Executors;
    +import java.util.concurrent.ScheduledExecutorService;
    +import java.util.concurrent.TimeUnit;
    +import java.util.ArrayList;
    +import java.util.List;
    +import java.util.Map;
    +import java.util.HashMap;
    +import java.text.DateFormat;
    +import java.text.SimpleDateFormat;
    +import java.sql.Connection;
    +import java.sql.PreparedStatement;
    +import java.sql.ResultSet;
    +import java.sql.SQLException;
    +import java.net.URL;
    +import java.net.SocketTimeoutException;
    +import java.net.MalformedURLException;
    +import java.net.ProtocolException;
    +import java.net.UnknownHostException;
    +import java.io.OutputStreamWriter;
    +import java.io.IOException;
    +
    +import javax.inject.Inject;
    +import javax.net.ssl.HttpsURLConnection;
    +
    +import org.apache.log4j.Logger;
    +import org.springframework.stereotype.Component;
    +
    +import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
    +import org.apache.cloudstack.managed.context.ManagedContextRunnable;
    +
    +import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao;
    +import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
    +
    +import org.apache.commons.codec.digest.DigestUtils;
    +
    +import com.cloud.host.HostVO;
    +import com.cloud.host.dao.HostDao;
    +import com.cloud.dc.ClusterVO;
    +import com.cloud.dc.dao.ClusterDao;
    +import com.cloud.dc.DataCenterVO;
    +import com.cloud.dc.dao.DataCenterDao;
    +import com.cloud.vm.UserVmVO;
    +import com.cloud.vm.dao.UserVmDao;
    +import com.cloud.vm.VMInstanceVO;
    +import com.cloud.vm.dao.VMInstanceDao;
    +import com.cloud.utils.db.SearchCriteria;
    +import com.cloud.utils.NumbersUtil;
    +import com.cloud.utils.component.ManagerBase;
    +import com.cloud.utils.component.ComponentMethodInterceptable;
    +import com.cloud.utils.concurrency.NamedThreadFactory;
    +import com.cloud.utils.db.DB;
    +import com.cloud.utils.db.TransactionLegacy;
    +import com.cloud.upgrade.dao.VersionDao;
    +import com.cloud.upgrade.dao.VersionVO;
    +import com.cloud.storage.dao.DiskOfferingDao;
    +import com.cloud.storage.DiskOfferingVO;
    +import com.google.gson.Gson;
    +import com.google.gson.GsonBuilder;
    +import com.google.common.util.concurrent.AtomicLongMap;
    +
    +@Component
    +public class UsageReporter extends ManagerBase implements ComponentMethodInterceptable {
    +    public static final Logger s_logger = Logger.getLogger(UsageReporter.class.getName());
    +
    +    /* !FIX ME! This should point to a Apache Infra host with SSL! */
    +    private String reportHost = "https://call-home.cloudstack.org/report";
    +
    +    private String uniqueID = null;
    +
    +    private static UsageReporter s_instance = null;
    +
    +    private ScheduledExecutorService _executor = null;
    +
    +    @Inject
    +    private ConfigurationDao _configDao;
    +    @Inject
    +    private HostDao _hostDao;
    +    @Inject
    +    private ClusterDao _clusterDao;
    +    @Inject
    +    private PrimaryDataStoreDao _storagePoolDao;
    +    @Inject
    +    private DataCenterDao _dataCenterDao;
    +    @Inject
    +    private UserVmDao _userVmDao;
    +    @Inject
    +    private VMInstanceDao _vmInstance;
    +    @Inject
    +    private VersionDao _versionDao;
    +    @Inject
    +    private DiskOfferingDao _diskOfferingDao;
    +
    +    int usageReportInterval = -1;
    +
    +    public static UsageReporter getInstance() {
    --- End diff --
    
    @wido look at my comment at line 123. you are writing a field that can be written from several locations at once. It needs guarding against programmers not communicating while coding.


---
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] cloudstack pull request: CLOUDSTACK-8677: Call-home functionality ...

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

    https://github.com/apache/cloudstack/pull/625#discussion_r53039381
  
    --- Diff: server/src/org/apache/cloudstack/report/AtomicGsonAdapter.java ---
    @@ -0,0 +1,48 @@
    +// 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.cloudstack.report;
    +
    +import com.google.gson.TypeAdapter;
    +import com.google.gson.stream.JsonReader;
    +import com.google.gson.stream.JsonWriter;
    +import com.google.common.util.concurrent.AtomicLongMap;
    +import java.util.Map;
    +import java.io.IOException;
    +
    +public class AtomicGsonAdapter extends TypeAdapter<AtomicLongMap> {
    +
    +    public AtomicLongMap<Object> read(JsonReader reader) throws IOException {
    --- End diff --
    
    Hi @wido,
    Why don't you turn this method "void", so you could remove the "return null;" line.


---
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] cloudstack pull request: CLOUDSTACK-8677: Call-home functionality ...

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

    https://github.com/apache/cloudstack/pull/625


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