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

✨Add simcore_user_agent and product_name to user service labels #3990

Merged
merged 24 commits into from
Mar 23, 2023
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions packages/models-library/src/models_library/docker.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
from typing import Optional

from models_library.generated_models.docker_rest_api import Task
from models_library.products import ProductName
from models_library.projects import ProjectID
from models_library.projects_nodes import NodeID
from models_library.users import UserID
Expand Down Expand Up @@ -37,6 +38,8 @@ class SimcoreServiceDockerLabelKeys(BaseModel):
user_id: UserID = Field(..., alias="user_id")
project_id: ProjectID = Field(..., alias="study_id")
node_id: NodeID = Field(..., alias="uuid")
product_name: Optional[ProductName] = None
GitHK marked this conversation as resolved.
Show resolved Hide resolved
user_agent: Optional[str] = None

def to_docker_labels(self) -> dict[str, str]:
"""returns a dictionary of strings as required by docker"""
Expand Down
43 changes: 31 additions & 12 deletions packages/models-library/tests/test_docker.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
# pylint: disable=unused-variable


from typing import Any

import pytest
from faker import Faker
from models_library.docker import (
Expand All @@ -12,6 +14,8 @@
)
from pydantic import ValidationError, parse_obj_as

faker = Faker()
GitHK marked this conversation as resolved.
Show resolved Hide resolved


@pytest.mark.parametrize(
"label_key, valid",
Expand Down Expand Up @@ -101,18 +105,33 @@ def test_docker_generic_tag(image_name: str, valid: bool):
parse_obj_as(DockerGenericTag, image_name)


@pytest.fixture
def osparc_docker_label_keys(
faker: Faker,
) -> SimcoreServiceDockerLabelKeys:
return SimcoreServiceDockerLabelKeys.parse_obj(
dict(user_id=faker.pyint(), project_id=faker.uuid4(), node_id=faker.uuid4())
@pytest.mark.parametrize(
"obj_data",
[
pytest.param(
{
"user_id": faker.pyint(),
"project_id": faker.uuid4(),
"node_id": faker.uuid4(),
},
id="parse_existing_service_labels",
),
pytest.param(
{
"user_id": faker.pyint(),
"project_id": faker.uuid4(),
"node_id": faker.uuid4(),
"product": "test_p",
"user_agent": "test/python",
},
id="parse_new_service_labels",
),
],
)
def test_simcore_service_docker_label_keys(obj_data: dict[str, Any]):
simcore_service_docker_label_keys = SimcoreServiceDockerLabelKeys.parse_obj(
obj_data
)


def test_osparc_docker_label_keys_to_docker_labels(
osparc_docker_label_keys: SimcoreServiceDockerLabelKeys,
):
exported_dict = osparc_docker_label_keys.to_docker_labels()
exported_dict = simcore_service_docker_label_keys.to_docker_labels()
assert all(isinstance(v, str) for v in exported_dict.values())
assert parse_obj_as(SimcoreServiceDockerLabelKeys, exported_dict)
Original file line number Diff line number Diff line change
Expand Up @@ -101,13 +101,13 @@ async def create_dynamic_service(
service: DynamicServiceCreate,
x_dynamic_sidecar_request_dns: str = Header(...),
x_dynamic_sidecar_request_scheme: str = Header(...),
x_dynamic_sidecar_request_user_agent: str = Header(...),
GitHK marked this conversation as resolved.
Show resolved Hide resolved
director_v0_client: DirectorV0Client = Depends(get_director_v0_client),
dynamic_services_settings: DynamicServicesSettings = Depends(
get_dynamic_services_settings
),
scheduler: DynamicSidecarsScheduler = Depends(get_scheduler),
) -> Union[DynamicServiceGet, RedirectResponse]:

simcore_service_labels: SimcoreServiceLabels = (
await director_v0_client.get_service_labels(
service=ServiceKeyVersion(key=service.key, version=service.version)
Expand Down Expand Up @@ -141,6 +141,7 @@ async def create_dynamic_service(
port=dynamic_services_settings.DYNAMIC_SIDECAR.DYNAMIC_SIDECAR_PORT,
request_dns=x_dynamic_sidecar_request_dns,
request_scheme=x_dynamic_sidecar_request_scheme,
request_user_agent=x_dynamic_sidecar_request_user_agent,
)

return cast(DynamicServiceGet, await scheduler.get_stack_status(service.node_uuid))
Expand All @@ -156,7 +157,6 @@ async def get_dynamic_sidecar_status(
director_v0_client: DirectorV0Client = Depends(get_director_v0_client),
scheduler: DynamicSidecarsScheduler = Depends(get_scheduler),
) -> Union[DynamicServiceGet, RedirectResponse]:

try:
return cast(DynamicServiceGet, await scheduler.get_stack_status(node_uuid))
except DynamicSidecarNotFoundError:
Expand Down Expand Up @@ -317,7 +317,6 @@ async def update_projects_networks(
director_v0_client: DirectorV0Client = Depends(get_director_v0_client),
rabbitmq_client: RabbitMQClient = Depends(get_rabbitmq_client),
) -> None:

await projects_networks.update_from_workbench(
projects_networks_repository=projects_networks_repository,
projects_repository=projects_repository,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -397,6 +397,9 @@ def endpoint(self) -> AnyHttpUrl:
request_scheme: str = Field(
..., description="used when configuring the CORS options on the proxy"
)
request_user_agent: str = Field(
..., description="used as label to filter out from the metrics"
GitHK marked this conversation as resolved.
Show resolved Hide resolved
)
proxy_service_name: str = Field(None, description="service name given to the proxy")

product_name: Optional[str] = Field(
GitHK marked this conversation as resolved.
Show resolved Hide resolved
Expand All @@ -414,6 +417,7 @@ def from_http_request(
port: PortInt,
request_dns: str,
request_scheme: str,
request_user_agent: str,
run_id: Optional[UUID] = None,
) -> "SchedulerData":
# This constructor method sets current product
Expand All @@ -440,6 +444,7 @@ def from_http_request(
request_dns=request_dns,
request_scheme=request_scheme,
proxy_service_name=names_helper.proxy_service_name,
request_user_agent=request_user_agent,
dynamic_sidecar={},
)
if run_id:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

from fastapi.applications import FastAPI
from models_library.docker import SimcoreServiceDockerLabelKeys
from models_library.products import ProductName
from models_library.projects import ProjectID
from models_library.projects_nodes_io import NodeID
from models_library.service_settings_labels import (
Expand Down Expand Up @@ -192,12 +193,18 @@ def _update_container_labels(
user_id: UserID,
project_id: ProjectID,
node_id: NodeID,
user_agent: str,
product_name: ProductName,
) -> None:
for spec in service_spec["services"].values():
labels: list[str] = spec.setdefault("labels", [])

label_keys = SimcoreServiceDockerLabelKeys(
user_id=user_id, study_id=project_id, uuid=node_id
user_id=user_id,
study_id=project_id,
uuid=node_id,
user_agent=user_agent,
product_name=product_name,
)
docker_labels = [f"{k}={v}" for k, v in label_keys.to_docker_labels().items()]

Expand All @@ -219,10 +226,11 @@ def assemble_spec(
service_resources: ServiceResourcesDict,
simcore_service_labels: SimcoreServiceLabels,
allow_internet_access: bool,
product_name: str,
product_name: ProductName,
user_id: UserID,
project_id: ProjectID,
node_id: NodeID,
user_agent: str,
) -> str:
"""
returns a docker-compose spec used by
Expand Down Expand Up @@ -286,6 +294,8 @@ def assemble_spec(
user_id=user_id,
project_id=project_id,
node_id=node_id,
product_name=product_name,
user_agent=user_agent,
)

# TODO: will be used in next PR
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ async def add_service(
port: PortInt,
request_dns: str,
request_scheme: str,
request_user_agent: str,
) -> None:
"""
Adds a new service.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,6 @@ async def will_trigger(cls, app: FastAPI, scheduler_data: SchedulerData) -> bool

@classmethod
async def action(cls, app: FastAPI, scheduler_data: SchedulerData) -> None:

# instrumentation
message = InstrumentationRabbitMessage(
metrics="service_started",
Expand Down Expand Up @@ -482,6 +481,7 @@ async def action(cls, app: FastAPI, scheduler_data: SchedulerData) -> None:
user_id=scheduler_data.user_id,
project_id=scheduler_data.project_id,
node_id=scheduler_data.node_uuid,
user_agent=scheduler_data.request_user_agent,
)

logger.debug(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,7 @@ async def add_service(
port: PortInt,
request_dns: str,
request_scheme: str,
request_user_agent: str,
) -> None:
"""Invoked before the service is started"""
scheduler_data = SchedulerData.from_http_request(
Expand All @@ -163,6 +164,7 @@ async def add_service(
port=port,
request_dns=request_dns,
request_scheme=request_scheme,
request_user_agent=request_user_agent,
)
await self._add_service(scheduler_data)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,9 +70,15 @@ async def add_service(
port: PortInt,
request_dns: str,
request_scheme: str,
request_user_agent: str,
) -> None:
return await self._scheduler.add_service(
service, simcore_service_labels, port, request_dns, request_scheme
service,
simcore_service_labels,
port,
request_dns,
request_scheme,
request_user_agent,
)

def is_service_tracked(self, node_uuid: NodeID) -> bool:
Expand Down
7 changes: 7 additions & 0 deletions services/director-v2/tests/unit/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -94,13 +94,19 @@ def request_scheme() -> str:
return "http"


@pytest.fixture
def request_user_agent() -> str:
return "python/test"


@pytest.fixture
def scheduler_data_from_http_request(
dynamic_service_create: DynamicServiceCreate,
simcore_service_labels: SimcoreServiceLabels,
dynamic_sidecar_port: int,
request_dns: str,
request_scheme: str,
request_user_agent: str,
run_id: RunID,
) -> SchedulerData:
return SchedulerData.from_http_request(
Expand All @@ -109,6 +115,7 @@ def scheduler_data_from_http_request(
port=dynamic_sidecar_port,
request_dns=request_dns,
request_scheme=request_scheme,
request_user_agent=request_user_agent,
run_id=run_id,
)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,8 @@ def test_regression_service_has_no_reservations():
USER_ID: UserID = 1
PROJECT_ID: ProjectID = uuid4()
NODE_ID: NodeID = uuid4()
USER_AGENT: str = "python/test"
PRODUCT_NAME: str = "osparc"


@pytest.mark.parametrize(
Expand Down Expand Up @@ -199,6 +201,8 @@ def test_regression_service_has_no_reservations():
f"user_id={USER_ID}",
f"study_id={PROJECT_ID}",
f"uuid={NODE_ID}",
f"user_agent={USER_AGENT}",
f"product_name={PRODUCT_NAME}",
]
},
}
Expand All @@ -211,6 +215,6 @@ async def test_update_container_labels(
service_spec: dict[str, Any], expected_result: dict[str, Any]
):
docker_compose_specs._update_container_labels(
service_spec, USER_ID, PROJECT_ID, NODE_ID
service_spec, USER_ID, PROJECT_ID, NODE_ID, USER_AGENT, PRODUCT_NAME
)
assert service_spec == expected_result
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
import logging
import urllib.parse
import warnings
from typing import Any, Dict, List, Optional, Tuple
from typing import Any, Optional

from aiohttp import ClientSession, web
from servicelib.aiohttp.client_session import get_client_session
Expand All @@ -21,7 +21,7 @@
warnings.warn("Director-v0 is deprecated, please use Director-v2", DeprecationWarning)


def _get_director_client(app: web.Application) -> Tuple[ClientSession, URL]:
def _get_director_client(app: web.Application) -> tuple[ClientSession, URL]:
settings: DirectorSettings = get_plugin_settings(app)
api_endpoint = settings.base_url
session = get_client_session(app)
Expand All @@ -32,7 +32,7 @@ async def get_running_interactive_services(
app: web.Application,
user_id: Optional[str] = None,
project_id: Optional[str] = None,
) -> List[Dict[str, Any]]:
) -> list[dict[str, Any]]:
session, api_endpoint = _get_director_client(app)

params = {}
Expand All @@ -59,7 +59,8 @@ async def start_service(
service_uuid: str,
request_dns: str,
request_scheme: str,
) -> Optional[Dict]:
request_user_agent: str,
) -> Optional[dict]:
session, api_endpoint = _get_director_client(app)

params = {
Expand All @@ -74,6 +75,7 @@ async def start_service(
headers = {
"X-Dynamic-Sidecar-Request-DNS": request_dns,
"X-Dynamic-Sidecar-Request-Scheme": request_scheme,
"X-Dynamic-Sidecar-Request-User-Agent": request_user_agent,
GitHK marked this conversation as resolved.
Show resolved Hide resolved
}

url = (api_endpoint / "running_interactive_services").with_query(params)
Expand Down Expand Up @@ -126,7 +128,7 @@ async def stop_services(

async def get_service_by_key_version(
app: web.Application, service_key: str, service_version: str
) -> Optional[Dict]:
) -> Optional[dict]:
session, api_endpoint = _get_director_client(app)

url = (
Expand All @@ -147,7 +149,7 @@ async def get_service_by_key_version(

async def get_services_extras(
app: web.Application, service_key: str, service_version: str
) -> Optional[Dict]:
) -> Optional[dict]:
session, api_endpoint = _get_director_client(app)

url = (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ async def get_dynamic_service(app: web.Application, node_uuid: str) -> DataType:

@log_decorator(logger=log)
async def run_dynamic_service(
*,
app: web.Application,
product_name: str,
user_id: PositiveInt,
Expand All @@ -83,6 +84,7 @@ async def run_dynamic_service(
service_uuid: str,
request_dns: str,
request_scheme: str,
request_user_agent: str,
service_resources: ServiceResourcesDict,
) -> DataType:
"""
Expand All @@ -106,6 +108,7 @@ async def run_dynamic_service(
headers = {
"X-Dynamic-Sidecar-Request-DNS": request_dns,
"X-Dynamic-Sidecar-Request-Scheme": request_scheme,
"X-Dynamic-Sidecar-Request-User-Agent": request_user_agent,
GitHK marked this conversation as resolved.
Show resolved Hide resolved
}

settings: DirectorV2Settings = get_plugin_settings(app)
Expand Down
Loading