Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[dashboard] New, add statsd incr to the API #9519

Merged
merged 12 commits into from
Apr 16, 2020
11 changes: 10 additions & 1 deletion superset/dashboards/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,11 @@
from superset.tasks.thumbnails import cache_dashboard_thumbnail
from superset.utils.screenshots import DashboardScreenshot
from superset.views.base import generate_download_headers
from superset.views.base_api import BaseSupersetModelRestApi, RelatedFieldFilter
from superset.views.base_api import (
BaseSupersetModelRestApi,
RelatedFieldFilter,
statsd_metrics,
)
from superset.views.filters import FilterRelatedOwners

logger = logging.getLogger(__name__)
Expand Down Expand Up @@ -139,6 +143,7 @@ def __init__(self) -> None:
@expose("/", methods=["POST"])
@protect()
@safe
@statsd_metrics
def post(self) -> Response:
"""Creates a new Dashboard
---
Expand Down Expand Up @@ -193,6 +198,7 @@ def post(self) -> Response:
@expose("/<pk>", methods=["PUT"])
@protect()
@safe
@statsd_metrics
def put( # pylint: disable=too-many-return-statements, arguments-differ
self, pk: int
) -> Response:
Expand Down Expand Up @@ -260,6 +266,7 @@ def put( # pylint: disable=too-many-return-statements, arguments-differ
@expose("/<pk>", methods=["DELETE"])
@protect()
@safe
@statsd_metrics
def delete(self, pk: int) -> Response: # pylint: disable=arguments-differ
"""Deletes a Dashboard
---
Expand Down Expand Up @@ -306,6 +313,7 @@ def delete(self, pk: int) -> Response: # pylint: disable=arguments-differ
@expose("/", methods=["DELETE"])
@protect()
@safe
@statsd_metrics
@rison(get_delete_ids_schema)
def bulk_delete(
self, **kwargs: Any
Expand Down Expand Up @@ -366,6 +374,7 @@ def bulk_delete(
@expose("/export/", methods=["GET"])
@protect()
@safe
@statsd_metrics
@rison(get_export_ids_schema)
def export(self, **kwargs: Any) -> Response:
"""Export dashboards
Expand Down
17 changes: 17 additions & 0 deletions superset/utils/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,10 @@
from email.utils import formatdate
from enum import Enum
from time import struct_time
from timeit import default_timer
from typing import (
Any,
Callable,
Dict,
Iterator,
List,
Expand Down Expand Up @@ -1229,6 +1231,21 @@ def create_ssl_cert_file(certificate: str) -> str:
return path


def time_function(func: Callable, *args, **kwargs) -> Tuple[float, Any]:
"""
Measures the amount of time a function takes to execute in ms

:param func: The function execution time to measure
:param args: args to be passed to the function
:param kwargs: kwargs to be passed to the function
:return: A tuple with the duration and response from the function
"""
start = default_timer()
response = func(*args, **kwargs)
stop = default_timer()
return stop - start, response


def MediumText() -> Variant:
return Text().with_variant(MEDIUMTEXT(), "mysql")

Expand Down
79 changes: 78 additions & 1 deletion superset/views/base_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,18 @@
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
import functools
import logging
from typing import cast, Dict, Set, Tuple, Type, Union
from typing import Any, cast, Dict, Optional, Set, Tuple, Type, Union

from flask import Response
from flask_appbuilder import ModelRestApi
from flask_appbuilder.api import expose, protect, rison, safe
from flask_appbuilder.models.filters import BaseFilter, Filters
from flask_appbuilder.models.sqla.filters import FilterStartsWith

from superset.stats_logger import BaseStatsLogger
from superset.utils.core import time_function

logger = logging.getLogger(__name__)
get_related_schema = {
Expand All @@ -35,6 +38,19 @@
}


def statsd_metrics(f):
"""
Handle sending all statsd metrics from the REST API
"""

def wraps(self, *args: Any, **kwargs: Any) -> Response:
duration, response = time_function(f, self, *args, **kwargs)
self.send_stats_metrics(response, f.__name__, duration)
return response

return functools.update_wrapper(wraps, f)


class RelatedFieldFilter:
# data class to specify what filter to use on a /related endpoint
# pylint: disable=too-few-public-methods
Expand Down Expand Up @@ -128,11 +144,71 @@ def _get_related_filter(self, datamodel, column_name: str, value: str) -> Filter
return filters

def incr_stats(self, action: str, func_name: str) -> None:
"""
Proxy function for statsd.incr to impose a key structure for REST API's

:param action: String with an action name eg: error, success
:param func_name: The function name
"""
self.stats_logger.incr(f"{self.__class__.__name__}.{func_name}.{action}")

def timing_stats(self, action: str, func_name: str, value: float) -> None:
"""
Proxy function for statsd.incr to impose a key structure for REST API's

:param action: String with an action name eg: error, success
:param func_name: The function name
:param value: A float with the time it took for the endpoint to execute
"""
self.stats_logger.timing(
f"{self.__class__.__name__}.{func_name}.{action}", value
)

def send_stats_metrics(
self, response: Response, key: str, time_delta: Optional[float] = None
) -> None:
"""
Helper function to handle sending statsd metrics

:param response: flask response object, will evaluate if it was an error
:param key: The function name
:param time_delta: Optional time it took for the endpoint to execute
"""
if 200 <= response.status_code < 400:
self.incr_stats("success", key)
else:
self.incr_stats("error", key)
if time_delta:
self.timing_stats("time", key, time_delta)

def info_headless(self, **kwargs) -> Response:
"""
Add statsd metrics to builtin FAB _info endpoint
"""
duration, response = time_function(super().info_headless, **kwargs)
self.send_stats_metrics(response, self.info.__name__, duration)
return response

def get_headless(self, pk, **kwargs) -> Response:
"""
Add statsd metrics to builtin FAB GET endpoint
"""
duration, response = time_function(super().get_headless, pk, **kwargs)
self.send_stats_metrics(response, self.get.__name__, duration)
return response

def get_list_headless(self, **kwargs) -> Response:
"""
Add statsd metrics to builtin FAB GET list endpoint
"""
duration, response = time_function(super().get_list_headless, **kwargs)
self.send_stats_metrics(response, self.get_list.__name__, duration)
return response

@expose("/related/<column_name>", methods=["GET"])
@protect()
@safe
@statsd_metrics
@rison(get_related_schema)
def related(self, column_name: str, **kwargs):
"""Get related fields data
Expand Down Expand Up @@ -185,6 +261,7 @@ def related(self, column_name: str, **kwargs):
$ref: '#/components/responses/500'
"""
if column_name not in self.allowed_rel_fields:
self.incr_stats("error", self.related.__name__)
return self.response_404()
args = kwargs.get("rison", {})
# handle pagination
Expand Down
84 changes: 82 additions & 2 deletions tests/base_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,11 @@
"""Unit tests for Superset"""
import imp
import json
from typing import Union
from unittest.mock import Mock
from typing import Union, Dict
from unittest.mock import Mock, patch

import pandas as pd
from flask import Response
from flask_appbuilder.security.sqla import models as ab_models
from flask_testing import TestCase

Expand All @@ -35,6 +36,7 @@
from superset.models.dashboard import Dashboard
from superset.models.datasource_access_request import DatasourceAccessRequest
from superset.utils.core import get_example_database
from superset.views.base_api import BaseSupersetModelRestApi

FAKE_DB_NAME = "fake_db_100"

Expand Down Expand Up @@ -328,3 +330,81 @@ def validate_sql(
def get_dash_by_slug(self, dash_slug):
sesh = db.session()
return sesh.query(Dashboard).filter_by(slug=dash_slug).first()

def get_assert_metric(self, uri: str, func_name: str) -> Response:
"""
Simple client get with an extra assertion for statsd metrics

:param uri: The URI to use for the HTTP GET
:param func_name: The function name that the HTTP GET triggers
for the statsd metric assertion
:return: HTTP Response
"""
with patch.object(
BaseSupersetModelRestApi, "incr_stats", return_value=None
) as mock_method:
rv = self.client.get(uri)
if 200 <= rv.status_code < 400:
mock_method.assert_called_once_with("success", func_name)
else:
mock_method.assert_called_once_with("error", func_name)
return rv

def delete_assert_metric(self, uri: str, func_name: str) -> Response:
"""
Simple client delete with an extra assertion for statsd metrics

:param uri: The URI to use for the HTTP DELETE
:param func_name: The function name that the HTTP DELETE triggers
for the statsd metric assertion
:return: HTTP Response
"""
with patch.object(
BaseSupersetModelRestApi, "incr_stats", return_value=None
) as mock_method:
rv = self.client.delete(uri)
if 200 <= rv.status_code < 400:
mock_method.assert_called_once_with("success", func_name)
else:
mock_method.assert_called_once_with("error", func_name)
return rv

def post_assert_metric(self, uri: str, data: Dict, func_name: str) -> Response:
"""
Simple client post with an extra assertion for statsd metrics

:param uri: The URI to use for the HTTP POST
:param data: The JSON data payload to be posted
:param func_name: The function name that the HTTP POST triggers
for the statsd metric assertion
:return: HTTP Response
"""
with patch.object(
BaseSupersetModelRestApi, "incr_stats", return_value=None
) as mock_method:
rv = self.client.post(uri, json=data)
if 200 <= rv.status_code < 400:
mock_method.assert_called_once_with("success", func_name)
else:
mock_method.assert_called_once_with("error", func_name)
return rv

def put_assert_metric(self, uri: str, data: Dict, func_name: str) -> Response:
"""
Simple client put with an extra assertion for statsd metrics

:param uri: The URI to use for the HTTP PUT
:param data: The JSON data payload to be posted
:param func_name: The function name that the HTTP PUT triggers
for the statsd metric assertion
:return: HTTP Response
"""
with patch.object(
BaseSupersetModelRestApi, "incr_stats", return_value=None
) as mock_method:
rv = self.client.put(uri, json=data)
if 200 <= rv.status_code < 400:
mock_method.assert_called_once_with("success", func_name)
else:
mock_method.assert_called_once_with("error", func_name)
return rv
Loading