Skip to content

Commit

Permalink
♻️ webserver: fixes mypy issues in diagnostics plugin (#4187)
Browse files Browse the repository at this point in the history
  • Loading branch information
pcrespov authored May 3, 2023
1 parent 290f0f7 commit 3460ffb
Show file tree
Hide file tree
Showing 13 changed files with 59 additions and 66 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
from .catalog import setup_catalog
from .clusters.plugin import setup_clusters
from .db import setup_db
from .diagnostics import setup_diagnostics
from .diagnostics.plugin import setup_diagnostics
from .director.plugin import setup_director
from .director_v2 import setup_director_v2
from .email import setup_email
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
from ._constants import APP_SETTINGS_KEY
from ._meta import API_VERSION, API_VTAG, APP_NAME
from .catalog_settings import CatalogSettings
from .diagnostics_settings import DiagnosticsSettings
from .diagnostics.settings import DiagnosticsSettings
from .director.settings import DirectorSettings
from .director_v2_settings import DirectorV2Settings
from .exporter.settings import ExporterSettings
Expand Down
Empty file.
Original file line number Diff line number Diff line change
Expand Up @@ -4,20 +4,20 @@
import asyncio
import logging
from contextlib import suppress
from typing import Any, Dict
from typing import Any

from aiohttp import ClientError, ClientSession, web
from models_library.app_diagnostics import AppStatusCheck
from servicelib.aiohttp.client_session import get_client_session
from servicelib.utils import logged_gather

from . import catalog_client, db, director_v2_api, storage_api
from ._meta import API_VERSION, APP_NAME, api_version_prefix
from .login.decorators import login_required
from .security_decorators import permission_required
from .utils import get_task_info, get_tracemalloc_info
from .. import catalog_client, db, director_v2_api, storage_api
from .._meta import API_VERSION, APP_NAME, api_version_prefix
from ..login.decorators import login_required
from ..security_decorators import permission_required
from ..utils import get_task_info, get_tracemalloc_info

log = logging.getLogger(__name__)
_logger = logging.getLogger(__name__)

routes = web.RouteTableDef()

Expand All @@ -31,7 +31,7 @@ async def get_app_diagnostics(request: web.Request):
/v0/status/diagnostics?top_tracemalloc=10 with display top 10 files allocating the most memory
"""
# tasks in loop
data: Dict[str, Any] = {
data: dict[str, Any] = {
"loop_tasks": [get_task_info(task) for task in asyncio.all_tasks()]
}

Expand All @@ -58,7 +58,7 @@ def _get_url_for(operation_id, **kwargs):

def _get_client_session_info():
client: ClientSession = get_client_session(request.app)
info: Dict[str, Any] = {"instance": str(client)}
info: dict[str, Any] = {"instance": str(client)}

if not client.closed:
info.update(
Expand Down Expand Up @@ -111,7 +111,7 @@ async def _check_catalog():
_check_storage(),
_check_director2(),
_check_catalog(),
log=log,
log=_logger,
reraise=False,
)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,14 @@
import statistics
import time
from dataclasses import dataclass, field
from typing import List, Optional

from aiohttp import web
from servicelib.aiohttp.incidents import LimitedOrderedStack, SlowCallback

from .diagnostics_settings import get_plugin_settings
from .rest_healthcheck import HealthCheckFailed
from ..rest_healthcheck import HealthCheckFailed
from .settings import get_plugin_settings

log = logging.getLogger(__name__)
_logger = logging.getLogger(__name__)

# APP KEYS ---
kINCIDENTS_REGISTRY = f"{__name__}.incidents_registry"
Expand All @@ -26,7 +25,8 @@

class IncidentsRegistry(LimitedOrderedStack[SlowCallback]):
def max_delay(self) -> float:
return self.max_item.delay_secs if self else 0
delay: float = self.max_item.delay_secs or 0.0
return delay


@dataclass
Expand All @@ -38,7 +38,7 @@ class DelayWindowProbe:

min_threshold_secs: float = 0.3
max_window: int = 100
last_delays: List = field(default_factory=list)
last_delays: list = field(default_factory=list)

def observe(self, delay: float):
# Mean latency of the last N request slower than min_threshold_secs sec
Expand All @@ -49,9 +49,10 @@ def observe(self, delay: float):
fifo.pop(0)

def value(self) -> float:
delay: float = 0.0
if self.last_delays:
return statistics.mean(self.last_delays)
return 0
delay = statistics.mean(self.last_delays)
return delay


logged_once = False
Expand All @@ -67,7 +68,7 @@ def is_sensing_enabled(app: web.Application):
time_elapsed_since_setup = time.time() - app[kPLUGIN_START_TIME]
enabled = time_elapsed_since_setup > settings.DIAGNOSTICS_START_SENSING_DELAY
if enabled and not logged_once:
log.debug(
_logger.debug(
"Diagnostics starts sensing after waiting %3.2f secs [> %3.2f secs] since submodule init",
time_elapsed_since_setup,
settings.DIAGNOSTICS_START_SENSING_DELAY,
Expand All @@ -86,9 +87,8 @@ def assert_healthy_app(app: web.Application) -> None:
settings = get_plugin_settings(app)

# CRITERIA 1:
incidents: Optional[IncidentsRegistry] = app.get(kINCIDENTS_REGISTRY)
incidents: IncidentsRegistry | None = app.get(kINCIDENTS_REGISTRY)
if incidents:

if not is_sensing_enabled(app):
# NOTE: this is the only way to avoid accounting
# before sensing is enabled
Expand All @@ -97,7 +97,7 @@ def assert_healthy_app(app: web.Application) -> None:
max_delay_allowed: float = settings.DIAGNOSTICS_MAX_TASK_DELAY
max_delay: float = incidents.max_delay()

log.debug(
_logger.debug(
"Max. blocking delay was %s secs [max allowed %s secs]",
max_delay,
max_delay_allowed,
Expand All @@ -111,12 +111,12 @@ def assert_healthy_app(app: web.Application) -> None:
raise HealthCheckFailed(msg)

# CRITERIA 2: Mean latency of the last N request slower than 1 sec
probe: Optional[DelayWindowProbe] = app.get(kLATENCY_PROBE)
probe: DelayWindowProbe | None = app.get(kLATENCY_PROBE)
if probe:
latency = probe.value()
max_latency_allowed = settings.DIAGNOSTICS_MAX_AVG_LATENCY

log.debug(
_logger.debug(
"Mean slow latency of last requests is %s secs [max allowed %s secs]",
latency,
max_latency_allowed,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,10 @@
from servicelib.aiohttp.monitoring import get_collector_registry
from servicelib.aiohttp.monitoring import setup_monitoring as service_lib_setup

from . import _meta
from .diagnostics_healthcheck import (
DelayWindowProbe,
is_sensing_enabled,
kLATENCY_PROBE,
)

log = logging.getLogger(__name__)
from .. import _meta
from ._healthcheck import DelayWindowProbe, is_sensing_enabled, kLATENCY_PROBE

_logger = logging.getLogger(__name__)

#
# CAUTION CAUTION CAUTION NOTE:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,33 +6,34 @@
from servicelib.aiohttp import monitor_slow_callbacks
from servicelib.aiohttp.application_setup import ModuleCategory, app_module_setup

from . import diagnostics_handlers
from .diagnostics_healthcheck import (
from ..rest import setup_rest
from ..rest_healthcheck import HealthCheck
from . import _handlers
from ._healthcheck import (
IncidentsRegistry,
assert_healthy_app,
kINCIDENTS_REGISTRY,
kPLUGIN_START_TIME,
)
from .diagnostics_monitoring import setup_monitoring
from .diagnostics_settings import DiagnosticsSettings, get_plugin_settings
from .rest import HealthCheck
from ._monitoring import setup_monitoring
from .settings import DiagnosticsSettings, get_plugin_settings

log = logging.getLogger(__name__)
_logger = logging.getLogger(__name__)


@app_module_setup(
__name__,
ModuleCategory.ADDON,
settings_name="WEBSERVER_DIAGNOSTICS",
depends=["simcore_service_webserver.rest"],
logger=log,
logger=_logger,
)
def setup_diagnostics(
app: web.Application,
):
setup_rest(app)

settings: DiagnosticsSettings = get_plugin_settings(app)

# TODO: redesign ... too convoluted!!
incidents_registry = IncidentsRegistry(order_by=attrgetter("delay_secs"))
app[kINCIDENTS_REGISTRY] = incidents_registry

Expand All @@ -49,9 +50,9 @@ def setup_diagnostics(
async def _on_healthcheck_async_adapter(app: web.Application):
assert_healthy_app(app)

healthcheck.on_healthcheck.append(_on_healthcheck_async_adapter)
healthcheck.on_healthcheck.append(_on_healthcheck_async_adapter) # type: ignore

# adds other diagnostic routes: healthcheck, etc
app.router.add_routes(diagnostics_handlers.routes)
app.router.add_routes(_handlers.routes)

app[kPLUGIN_START_TIME] = time.time()
Original file line number Diff line number Diff line change
@@ -1,14 +1,10 @@
# pylint: disable=no-self-use
# pylint: disable=no-self-argument

from aiohttp.web import Application
from pydantic import Field, NonNegativeFloat, PositiveFloat, validator
from servicelib.aiohttp.application_keys import APP_SETTINGS_KEY
from settings_library.base import BaseCustomSettings


class DiagnosticsSettings(BaseCustomSettings):

DIAGNOSTICS_SLOW_DURATION_SECS: PositiveFloat = Field(
1.0,
description=(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,26 +6,28 @@

from aiohttp import web
from servicelib.aiohttp.application_setup import ModuleCategory, app_module_setup
from simcore_service_webserver.diagnostics.plugin import setup_diagnostics

from ..db import setup_db
from ..diagnostics.plugin import setup_diagnostics
from ..rabbitmq import setup_rabbitmq
from ..socketio.plugin import setup_socketio
from ._db_comp_tasks_listening_task import create_comp_tasks_listening_task
from ._rabbitmq_consumers import setup_rabbitmq_consumers

log = logging.getLogger(__name__)
_logger = logging.getLogger(__name__)


@app_module_setup(
__name__,
ModuleCategory.ADDON,
settings_name="WEBSERVER_NOTIFICATIONS",
logger=log,
depends=[
"simcore_service_webserver.diagnostics",
], # depends on diagnostics for setting the instrumentation
logger=_logger,
)
def setup_notifications(app: web.Application):
# depends on diagnostics for setting the instrumentation
setup_diagnostics(app)

setup_rabbitmq(app)
setup_socketio(app)
# Subscribe to rabbit upon startup for logs, progress and other
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
from simcore_service_webserver._meta import API_VTAG
from simcore_service_webserver.application_settings import setup_settings
from simcore_service_webserver.db import setup_db
from simcore_service_webserver.diagnostics import setup_diagnostics
from simcore_service_webserver.diagnostics.plugin import setup_diagnostics
from simcore_service_webserver.director_v2 import setup_director_v2
from simcore_service_webserver.login.plugin import setup_login
from simcore_service_webserver.notifications._utils import DB_TO_RUNNING_STATE
Expand Down
2 changes: 1 addition & 1 deletion services/web/server/tests/integration/02/test_rabbit.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
from simcore_postgres_database.models.comp_tasks import NodeClass
from simcore_service_webserver.application_settings import setup_settings
from simcore_service_webserver.db import setup_db
from simcore_service_webserver.diagnostics import setup_diagnostics
from simcore_service_webserver.diagnostics.plugin import setup_diagnostics
from simcore_service_webserver.director_v2 import setup_director_v2
from simcore_service_webserver.login.plugin import setup_login
from simcore_service_webserver.notifications.plugin import setup_notifications
Expand Down
11 changes: 5 additions & 6 deletions services/web/server/tests/unit/isolated/test_diagnostics.py
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
# pylint: disable=redefined-outer-name
# pylint: disable=unused-argument
# pylint: disable=unused-variable
# pylint: disable=too-many-arguments

from typing import Dict
from unittest.mock import Mock

import pytest
from servicelib.aiohttp.application_setup import APP_SETUP_COMPLETED_KEY
from simcore_service_webserver import diagnostics_handlers
from simcore_service_webserver.application_settings import setup_settings
from simcore_service_webserver.diagnostics import setup_diagnostics
from simcore_service_webserver.diagnostics import _handlers
from simcore_service_webserver.diagnostics.plugin import setup_diagnostics
from simcore_service_webserver.rest import api_version_prefix, setup_rest


Expand Down Expand Up @@ -46,7 +46,7 @@ def app_mock(openapi_specs):


def test_unique_application_keys(
app_mock, openapi_specs, mock_env_devel_environment: Dict[str, str]
app_mock, openapi_specs, mock_env_devel_environment: dict[str, str]
):
setup_settings(app_mock)
setup_rest(app_mock)
Expand All @@ -61,9 +61,8 @@ def test_unique_application_keys(
app_mock.assert_none_overriden()


@pytest.mark.parametrize("route", diagnostics_handlers.routes, ids=lambda r: r.path)
@pytest.mark.parametrize("route", _handlers.routes, ids=lambda r: r.path)
def test_route_against_openapi_specs(route, openapi_specs):

assert route.path.startswith(f"/{api_version_prefix}")
path = route.path.replace(f"/{api_version_prefix}", "")

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,13 @@
from servicelib.aiohttp.application import create_safe_application
from simcore_service_webserver._constants import APP_SETTINGS_KEY
from simcore_service_webserver.application_settings import setup_settings
from simcore_service_webserver.diagnostics import setup_diagnostics
from simcore_service_webserver.diagnostics_healthcheck import (
from simcore_service_webserver.diagnostics._healthcheck import (
HealthCheckFailed,
assert_healthy_app,
kLATENCY_PROBE,
)
from simcore_service_webserver.diagnostics_settings import DiagnosticsSettings
from simcore_service_webserver.diagnostics.plugin import setup_diagnostics
from simcore_service_webserver.diagnostics.settings import DiagnosticsSettings
from simcore_service_webserver.rest import setup_rest
from simcore_service_webserver.security import setup_security
from tenacity import before_log, retry, stop_after_attempt, wait_fixed
Expand Down Expand Up @@ -89,7 +89,6 @@ def client(
api_version_prefix,
mock_environment: None,
):

routes = web.RouteTableDef()

@routes.get("/error")
Expand Down

0 comments on commit 3460ffb

Please sign in to comment.