Merge pull request #115 from mesosphere/rollbar

rollbar exception handling

README.rst

@@ -45,7 +45,7 @@ Configure Environment and Run
 #. :code:`source` the setup file to add the :code:`dcos` command line
    interface to your :code:`PATH` and create an empty configuration file::
 
-    source env/bin/env-setup
+    source bin/env-setup-dev
 
 #. Configure Marathon, changing the values below as appropriate for your local
    installation::

cli/bin/env-setup-dev

@@ -0,0 +1,15 @@
+# 1. source env-setup
+# 2. export DCOS_PRODUCTION=false
+
+if [ -n "$BASH_SOURCE" ] ; then
+    BIN_DIR=$(dirname "$BASH_SOURCE")
+elif [ $(basename -- "$0") = "env-setup" ]; then
+    BIN_DIR=$(dirname "$0")
+else
+    BIN_DIR=$PWD/bin
+fi
+
+BASE_DIR="$BIN_DIR/.."
+
+source ${BASE_DIR}/env/bin/env-setup
+export DCOS_PRODUCTION=false

cli/dcoscli/constants.py

@@ -0,0 +1 @@
+ROLLBAR_SERVER_POST_KEY = '62f87c5df3674629b143a137de3d3244'

cli/dcoscli/main.py

@@ -26,14 +26,19 @@ Environment Variables:
 to read about a specific subcommand.
 """
 
-
+import json
+import logging
 import os
-import subprocess
+import sys
+from subprocess import PIPE, Popen
 
 import dcoscli
 import docopt
-from dcos.api import constants, emitting, subcommand, util
+import rollbar
+from dcos.api import config, constants, emitting, http, subcommand, util
+from dcoscli.constants import ROLLBAR_SERVER_POST_KEY
 
+logger = logging.getLogger(__name__)
 emitter = emitting.FlatEmitter()
 
 
@@ -55,6 +60,7 @@ def main():
         return 1
 
     command = args['<command>']
+    http.silence_requests_warnings()
 
     if not command:
         command = "help"
@@ -64,7 +70,81 @@ def main():
         emitter.publish(err)
         return 1
 
-    return subprocess.call([executable,  command] + args['<args>'])
+    subproc = Popen([executable,  command] + args['<args>'],
+                    stderr=PIPE)
+
+    prod = os.environ.get('DCOS_PRODUCTION', 'true') != 'false'
+    rollbar.init(ROLLBAR_SERVER_POST_KEY,
+                 'prod' if prod else 'dev')
+    return _wait_and_track(subproc)
+
+
+def _wait_and_capture(subproc):
+    """
+    :param subproc: Subprocess to capture
+    :type subproc: Popen
+    :returns: exit code of subproc
+    :rtype: int
+    """
+
+    # capture and print stderr
+    err = ''
+    while subproc.poll() is None:
+        err_buff = subproc.stderr.read().decode('utf-8')
+        sys.stderr.write(err_buff)
+        err += err_buff
+
+    exit_code = subproc.poll()
+
+    return exit_code, err
+
+
+def _wait_and_track(subproc):
+    """
+    :param subproc: Subprocess to capture
+    :type subproc: Popen
+    :returns: exit code of subproc
+    :rtype: int
+    """
+
+    exit_code, err = _wait_and_capture(subproc)
+
+    conf = config.load_from_path(
+        os.environ[constants.DCOS_CONFIG_ENV])
+
+    # We only want to catch exceptions, not other stderr messages
+    # (such as "task does not exist", so we look for the 'Traceback'
+    # string.  This only works for python, so we'll need to revisit
+    # this in the future when we support subcommands written in other
+    # languages.
+    if 'Traceback' in err and conf.get('core.reporting', True):
+        _track(exit_code, err, conf)
+
+    return exit_code
+
+
+def _track(exit_code, err, conf):
+    """
+    :param exit_code: exit code of tracked process
+    :type exit_code: int
+    :param err: stderr of tracked process
+    :type err: str
+    :param conf: dcos config file
+    :type conf: Toml
+    :rtype: None
+    """
+
+    # rollbar analytics
+    try:
+        rollbar.report_message(err, 'error', extra_data={
+            'cmd': ' '.join(sys.argv),
+            'exit_code': exit_code,
+            'python_version': str(sys.version_info),
+            'dcoscli.version': dcoscli.version,
+            'config': json.dumps(list(conf.property_items()))
+        })
+    except Exception as e:
+        logger.exception(e)
 
 
 def _config_log_level_environ(log_level):

cli/setup.py

@@ -67,6 +67,7 @@ setup(
         'pkginfo',
         'toml',
         'virtualenv',
+        'rollbar'
     ],
 
     # If there are data files included in your packages that need to be

cli/tests/data/analytics/dcos_no_reporting.toml

@@ -0,0 +1,2 @@
+[core]
+reporting = false

cli/tests/data/analytics/dcos_reporting.toml

@@ -0,0 +1,2 @@
+[core]
+reporting = true

cli/tests/data/dcos.toml

@@ -1,10 +1,10 @@
-[core]
-reporting = true
 [subcommand]
 pip_find_links = "../dist"
 [marathon]
-host = "localhost"
 port = 8080
+host = "localhost"
 [package]
-sources = [ "git://github.com/mesosphere/universe.git", "https://github.com/mesosphere/universe/archive/master.zip",]
 cache = "tmp/cache"
+sources = [ "git://github.com/mesosphere/universe.git", "https://github.com/mesosphere/universe/archive/master.zip",]
+[core]
+reporting = true

cli/tests/integrations/cli/test_analytics.py

@@ -0,0 +1,114 @@
+import json
+import os
+import sys
+
+import dcoscli
+import rollbar
+from dcos.api import config, constants, util
+from dcoscli.constants import ROLLBAR_SERVER_POST_KEY
+from dcoscli.main import main
+
+from mock import Mock, patch
+
+
+def test_no_exc():
+    '''Tests that a command which does not raise an exception does not
+    report an exception.
+
+    '''
+
+    args = [util.which('dcos')]
+    exit_code = _mock_analytics_run(args)
+
+    assert rollbar.report_message.call_count == 0
+    assert exit_code == 0
+
+
+def test_exc():
+    '''Tests that a command which does raise an exception does report an
+    exception.
+
+    '''
+
+    args = [util.which('dcos')]
+    exit_code = _mock_analytics_run_exc(args)
+
+    props = _analytics_properties(args, exit_code=1)
+    rollbar.report_message.assert_called_with('Traceback', 'error',
+                                              extra_data=props)
+    assert exit_code == 1
+
+
+def test_config_reporting_false():
+    '''Test that "core.reporting = false" blocks exception reporting.'''
+
+    args = [util.which('dcos')]
+    exit_code = _mock_analytics_run_exc(args, False)
+
+    assert rollbar.report_message.call_count == 0
+    assert exit_code == 1
+
+
+def test_production_setting_true():
+    '''Test that env var DCOS_PRODUCTION=true sends exceptions to
+    the 'prod' environment.
+
+    '''
+
+    args = [util.which('dcos')]
+    with patch.dict(os.environ, {'DCOS_PRODUCTION': 'true'}):
+        _mock_analytics_run(args)
+        rollbar.init.assert_called_with(ROLLBAR_SERVER_POST_KEY, 'prod')
+
+
+def test_production_setting_false():
+    '''Test that env var DCOS_PRODUCTION=false sends exceptions to
+    the 'dev' environment.
+
+    '''
+
+    args = [util.which('dcos')]
+    with patch.dict(os.environ, {'DCOS_PRODUCTION': 'false'}):
+        _mock_analytics_run(args)
+        rollbar.init.assert_called_with(ROLLBAR_SERVER_POST_KEY, 'dev')
+
+
+def _config_path_reporting():
+    return os.path.join('tests', 'data', 'analytics', 'dcos_reporting.toml')
+
+
+def _config_path_no_reporting():
+    return os.path.join('tests', 'data', 'analytics', 'dcos_no_reporting.toml')
+
+
+def _env_reporting():
+    return {constants.DCOS_CONFIG_ENV: _config_path_reporting()}
+
+
+def _env_no_reporting():
+    return {constants.DCOS_CONFIG_ENV: _config_path_no_reporting()}
+
+
+def _mock_analytics_run_exc(args, reporting=True):
+    dcoscli.main._wait_and_capture = Mock(return_value=(1, 'Traceback'))
+    return _mock_analytics_run(args, reporting)
+
+
+def _mock_analytics_run(args, reporting=True):
+    env = _env_reporting() if reporting else _env_no_reporting()
+
+    with patch('sys.argv', args), patch.dict(os.environ, env):
+        rollbar.init = Mock()
+        rollbar.report_message = Mock()
+        return main()
+
+
+def _analytics_properties(sysargs, **kwargs):
+    conf = config.load_from_path(_config_path_reporting())
+    defaults = {'cmd': ' '.join(sysargs),
+                'exit_code': 0,
+                'dcoscli.version': dcoscli.version,
+                'python_version': str(sys.version_info),
+                'config': json.dumps(list(conf.property_items()))}
+    defaults.update(kwargs)
+    return defaults

cli/tests/integrations/cli/test_dcos.py

@@ -103,7 +103,6 @@ def test_log_level_flag():
 
     assert returncode == 0
     assert stdout == b"Get and set DCOS command line options\n"
-    assert stderr == b''
 
 
 def test_capital_log_level_flag():
@@ -112,7 +111,6 @@ def test_capital_log_level_flag():
 
     assert returncode == 0
     assert stdout == b"Get and set DCOS command line options\n"
-    assert stderr == b''
 
 
 def test_invalid_log_level_flag():

cli/tox.ini

@@ -5,6 +5,7 @@ envlist = py{27,34}-integration, syntax
 deps =
   pytest
   pytest-cov
+  mock
   -e..
 
 [testenv:syntax]

dcos/api/http.py

@@ -175,3 +175,8 @@ def delete(url, to_error=_default_to_error, **kwargs):
     """
 
     return request('delete', url, to_error=to_error, **kwargs)
+
+
+def silence_requests_warnings():
+    """Silence warnings from requests.packages.urllib3.  See DCOS-1007."""
+    requests.packages.urllib3.disable_warnings()

dcos/api/subcommand.py

@@ -66,7 +66,6 @@ def list_paths(dcos_path):
     subcommands = [
         os.path.join(subcommand_directory, package, BIN_DIRECTORY, filename)
 
-
         for package in distributions(dcos_path)
 
         for filename in os.listdir(

dcos/api/util.py

@@ -88,6 +88,7 @@ def dcos_path():
     :returns: the real path to the DCOS path
     :rtype: str
     """
+
     dcos_bin_dir = os.path.realpath(sys.argv[0])
     dcos_dir = os.path.dirname(os.path.dirname(dcos_bin_dir))
     return dcos_dir