Skip to content

Commit

Permalink
Show executor metrics (#1096)
Browse files Browse the repository at this point in the history
* Use dedicated exception class when metrics response is empty

* Add test for empty container metrics

When container metrics (`/v0/containers/<c-id>`) returns a 204 empty
response, we should still check the app metrics
(`/v0/containers/<c-id>/app`) endpoint, which may have data.

This test replicates the task metrics details test, but responds with a
204 Empty followed by a 200 OK. Without the accompanying fix, it will
fail.

* Catch empty metrics exception for container datapoints
  • Loading branch information
philipnrmn authored and bamarni committed Nov 23, 2017
1 parent 1e8d845 commit 46463a3
Show file tree
Hide file tree
Showing 2 changed files with 43 additions and 3 deletions.
21 changes: 18 additions & 3 deletions cli/dcoscli/metrics.py
Expand Up @@ -9,6 +9,11 @@
emitter = emitting.FlatEmitter()


class EmptyMetricsException(DCOSException):
def __init__(self):
super().__init__('No metrics found')


def _gib(n):
return n * pow(2, -30)

Expand All @@ -30,7 +35,7 @@ def _fetch_metrics_datapoints(url):
with contextlib.closing(http.get(url)) as r:

if r.status_code == 204:
raise DCOSException('No metrics found')
raise EmptyMetricsException()

if r.status_code != 200:
raise DCOSHTTPException(r)
Expand Down Expand Up @@ -258,8 +263,18 @@ def print_task_metrics(url, app_url, summary, json_):
:rtype: int
"""

datapoints = _fetch_metrics_datapoints(url) + _fetch_metrics_datapoints(
app_url)
container_datapoints = []
app_datapoints = []

# In the case of an executor, app data may exist when
# container data does not.
try:
container_datapoints = _fetch_metrics_datapoints(url)
except EmptyMetricsException:
pass

app_datapoints = _fetch_metrics_datapoints(app_url)
datapoints = container_datapoints + app_datapoints

if summary:
if json_:
Expand Down
25 changes: 25 additions & 0 deletions cli/tests/unit/test_task.py
Expand Up @@ -175,6 +175,31 @@ def test_dcos_task_metrics_agent_details(mocked_get_config_val,
'container_id')


@patch('dcos.http.get')
@patch('dcos.mesos.get_master')
@patch('dcos.config.get_config_val')
def test_dcos_task_metrics_agent_missing_container(
mocked_get_config_val, mocked_get_master, mocked_http_get
):
mocked_get_config_val.return_value = 'http://127.0.0.1'

mock_container_response = MagicMock()
mock_container_response.status_code = 204
mock_app_response = MagicMock()
mock_app_response.status_code = 200
mocked_http_get.side_effect = [mock_container_response, mock_app_response]

mock_master = MagicMock()
mock_master.task = lambda _: {'slave_id': 'slave_id'}
mock_master.get_container_id = lambda _: {
'parent': {},
'value': 'container_id'
}
mocked_get_master.return_value = mock_master

_metrics(True, 'task_id', False)


@patch('dcos.http.get')
@patch('dcos.mesos.get_master')
@patch('dcos.config.get_config_val')
Expand Down

0 comments on commit 46463a3

Please sign in to comment.