From 4b0e260ac5e45167a33322fe7bdef63211d9fd53 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=9D=9E=E6=B3=95=E6=93=8D=E4=BD=9C?= Date: Mon, 7 Sep 2026 02:47:00 +0000 Subject: [PATCH] refactor(console): decouple app tracing configuration (#41576) --- api/.importlinter | 15 + api/controllers/console/app/error.py | 36 +- api/controllers/console/app/ops_trace.py | 249 ++++--- api/extensions/ext_application_services.py | 8 + api/openapi/markdown/console-openapi.md | 17 +- .../app_tracing_config_repository.py | 128 ++++ api/services/app_tracing_config_gateway.py | 184 ++++++ api/services/app_tracing_config_service.py | 230 +++++++ api/services/ops_service.py | 282 -------- .../.ruff.toml | 1 - .../pyrefly.toml | 1 - .../services/test_ops_service.py | 385 ----------- .../controllers/console/app/test_app_apis.py | 63 +- .../console/app/test_ops_trace_api.py | 624 +++++++++++++----- .../test_ext_application_services.py | 19 + .../test_app_tracing_config_repository.py | 145 ++++ .../test_app_tracing_config_gateway.py | 360 ++++++++++ .../test_app_tracing_config_service.py | 229 +++++++ .../generated/api/console/apps/types.gen.ts | 9 + 19 files changed, 2064 insertions(+), 921 deletions(-) create mode 100644 api/repositories/app_tracing_config_repository.py create mode 100644 api/services/app_tracing_config_gateway.py create mode 100644 api/services/app_tracing_config_service.py delete mode 100644 api/services/ops_service.py delete mode 100644 api/tests/test_containers_integration_tests/services/test_ops_service.py create mode 100644 api/tests/unit_tests/repositories/test_app_tracing_config_repository.py create mode 100644 api/tests/unit_tests/services/test_app_tracing_config_gateway.py create mode 100644 api/tests/unit_tests/services/test_app_tracing_config_service.py diff --git a/api/.importlinter b/api/.importlinter index 3933ee443ed..b29dd9cc4ae 100644 --- a/api/.importlinter +++ b/api/.importlinter @@ -264,6 +264,21 @@ forbidden_modules = sqlalchemy werkzeug +[importlinter:contract:app-tracing-config-service-boundary] +name = App tracing configuration service is framework and persistence neutral +type = forbidden +source_modules = + services.app_tracing_config_service +forbidden_modules = + configs + controllers + extensions + flask + models + repositories + sqlalchemy + werkzeug + [importlinter:contract:webapp-access-query-service-boundary] name = Web app access query application service is framework and persistence neutral type = forbidden diff --git a/api/controllers/console/app/error.py b/api/controllers/console/app/error.py index 2a84336596e..dd9725291ee 100644 --- a/api/controllers/console/app/error.py +++ b/api/controllers/console/app/error.py @@ -107,24 +107,42 @@ class DraftWorkflowNotSync(BaseHTTPException): code = 409 -class TracingConfigNotExist(BaseHTTPException): - error_code = "trace_config_not_exist" - description = "Trace config not exist." +class TracingConfigNotFoundError(BaseHTTPException): + error_code = "trace_config_not_found" + description = "Tracing configuration not found." + code = 404 + + +class TracingConfigAlreadyExistsError(BaseHTTPException): + error_code = "trace_config_already_exists" + description = "A tracing configuration already exists for this provider." + code = 409 + + +class UnsupportedTracingProviderError(BaseHTTPException): + error_code = "unsupported_tracing_provider" + description = "The tracing provider is not supported." code = 400 -class TracingConfigIsExist(BaseHTTPException): - error_code = "trace_config_is_exist" - description = "Trace config is exist." +class InvalidTracingConfigError(BaseHTTPException): + error_code = "invalid_tracing_config" + description = "The tracing configuration is invalid." code = 400 -class TracingConfigCheckError(BaseHTTPException): - error_code = "trace_config_check_error" - description = "Invalid Credentials." +class TracingConfigVerificationFailedError(BaseHTTPException): + error_code = "tracing_config_verification_failed" + description = "The tracing configuration could not be verified." code = 400 +class TracingConfigProcessingError(BaseHTTPException): + error_code = "tracing_config_processing_failed" + description = "The tracing configuration could not be processed." + code = 500 + + class InvokeRateLimitError(BaseHTTPException): """Raised when the Invoke returns rate limit error.""" diff --git a/api/controllers/console/app/ops_trace.py b/api/controllers/console/app/ops_trace.py index 2332f164cd9..9f9cdfb48f2 100644 --- a/api/controllers/console/app/ops_trace.py +++ b/api/controllers/console/app/ops_trace.py @@ -1,27 +1,49 @@ from typing import Any +from uuid import UUID from flask_restx import Resource from pydantic import BaseModel, Field -from werkzeug.exceptions import BadRequest from controllers.common.schema import query_params_from_model, register_response_schema_models, register_schema_models from controllers.console import console_ns -from controllers.console.app.error import TracingConfigCheckError, TracingConfigIsExist, TracingConfigNotExist -from controllers.console.app.wraps import get_app_model +from controllers.console.app.error import ( + AppNotFoundError, + InvalidTracingConfigError, + TracingConfigAlreadyExistsError, + TracingConfigNotFoundError, + TracingConfigProcessingError, + TracingConfigVerificationFailedError, + UnsupportedTracingProviderError, +) +from controllers.console.flask_admission import console_account_admission from controllers.console.wraps import ( RBACPermission, RBACResourceScope, - account_initialization_required, - edit_permission_required, model_validate, - rbac_permission_required, - setup_required, ) -from extensions.ext_database import db +from extensions.ext_application_services import application_services from fields.base import ResponseModel -from libs.login import login_required -from models import App -from services.ops_service import OpsService +from libs.helper import dump_response +from machinery.context import RequestContext +from models.account import TenantAccountRole +from services.app_tracing_config_service import ( + AppTracingConfigAlreadyExistsError, + AppTracingConfigAppNotFoundError, + AppTracingConfigInvalidConfigurationError, + AppTracingConfigInvalidProviderError, + AppTracingConfigNotFoundError, + AppTracingConfigProcessingError, + AppTracingConfigRecord, + AppTracingConfigVerificationFailedError, +) + +_APP_TRACING_CONFIG_EDIT_ROLES = frozenset( + { + TenantAccountRole.OWNER, + TenantAccountRole.ADMIN, + TenantAccountRole.EDITOR, + } +) class TraceProviderQuery(BaseModel): @@ -48,6 +70,18 @@ class TraceAppConfigResponse(ResponseModel): created_at: str | None = None updated_at: str | None = None + @classmethod + def from_record(cls, record: AppTracingConfigRecord) -> "TraceAppConfigResponse": + return cls( + id=record.id, + app_id=record.app_id, + tracing_provider=record.tracing_provider, + tracing_config=record.tracing_config, + is_active=record.is_active, + created_at=str(record.created_at), + updated_at=str(record.updated_at), + ) + register_schema_models(console_ns, TraceProviderQuery, TraceConfigPayload) register_response_schema_models(console_ns, TraceAppConfigResponse) @@ -68,23 +102,42 @@ class TraceAppConfigApi(Resource): "Tracing configuration retrieved successfully", console_ns.models[TraceAppConfigResponse.__name__], ) - @console_ns.response(400, "Invalid request parameters") - @setup_required - @login_required - @account_initialization_required - @rbac_permission_required(RBACResourceScope.APP, RBACPermission.APP_TRACING_CONFIG) - @get_app_model + @console_ns.response(400, "Invalid request parameters or unsupported tracing provider") + @console_ns.response(404, "Application not found") + @console_ns.response(500, "Tracing configuration processing failed") + @console_account_admission( + rbac_resource_scope=RBACResourceScope.APP, + rbac_permission=RBACPermission.APP_TRACING_CONFIG, + ) @model_validate(TraceProviderQuery) - def get(self, req_data: TraceProviderQuery, app_model: App): + def get( + self, + req_data: TraceProviderQuery, + request_context: RequestContext, + app_id: UUID, + ): try: - trace_config = OpsService.get_tracing_app_config( - app_id=app_model.id, tracing_provider=req_data.tracing_provider, session=db.session() + trace_config = application_services().app_tracing_configs.get( + context=request_context, + app_id=str(app_id), + tracing_provider=req_data.tracing_provider, ) - if not trace_config: - return {"has_not_configured": True} - return trace_config - except Exception as e: - raise BadRequest(str(e)) + except AppTracingConfigAppNotFoundError as error: + raise AppNotFoundError() from error + except AppTracingConfigInvalidProviderError as error: + raise UnsupportedTracingProviderError() from error + except AppTracingConfigProcessingError as error: + raise TracingConfigProcessingError() from error + except ValueError as error: + raise TracingConfigProcessingError() from error + + if trace_config is None: + return dump_response(TraceAppConfigResponse, {"has_not_configured": True}, exclude_none=True) + return dump_response( + TraceAppConfigResponse, + TraceAppConfigResponse.from_record(trace_config), + exclude_none=True, + ) @console_ns.doc("create_trace_app_config") @console_ns.doc(description="Create a new tracing configuration for an application") @@ -95,31 +148,47 @@ class TraceAppConfigApi(Resource): "Tracing configuration created successfully", console_ns.models[TraceAppConfigResponse.__name__], ) - @console_ns.response(400, "Invalid request parameters or configuration already exists") + @console_ns.response(400, "Invalid request parameters or tracing configuration") @console_ns.response(403, "Insufficient permissions") - @setup_required - @login_required - @account_initialization_required - @edit_permission_required - @rbac_permission_required(RBACResourceScope.APP, RBACPermission.APP_TRACING_CONFIG) - @get_app_model + @console_ns.response(404, "Application not found") + @console_ns.response(409, "Tracing configuration already exists") + @console_ns.response(500, "Tracing configuration processing failed") + @console_account_admission( + allowed_roles=_APP_TRACING_CONFIG_EDIT_ROLES, + rbac_resource_scope=RBACResourceScope.APP, + rbac_permission=RBACPermission.APP_TRACING_CONFIG, + ) @model_validate(TraceConfigPayload) - def post(self, req_data: TraceConfigPayload, app_model: App): + def post( + self, + req_data: TraceConfigPayload, + request_context: RequestContext, + app_id: UUID, + ): """Create a new trace app configuration""" try: - result = OpsService.create_tracing_app_config( - app_id=app_model.id, + application_services().app_tracing_configs.create( + context=request_context, + app_id=str(app_id), tracing_provider=req_data.tracing_provider, tracing_config=req_data.tracing_config, - session=db.session(), ) - if not result: - raise TracingConfigIsExist() - if result.get("error"): - raise TracingConfigCheckError() - return result - except Exception as e: - raise BadRequest(str(e)) + except AppTracingConfigAppNotFoundError as error: + raise AppNotFoundError() from error + except AppTracingConfigAlreadyExistsError as error: + raise TracingConfigAlreadyExistsError() from error + except AppTracingConfigInvalidProviderError as error: + raise UnsupportedTracingProviderError() from error + except AppTracingConfigInvalidConfigurationError as error: + raise InvalidTracingConfigError() from error + except AppTracingConfigVerificationFailedError as error: + raise TracingConfigVerificationFailedError() from error + except AppTracingConfigProcessingError as error: + raise TracingConfigProcessingError() from error + except ValueError as error: + raise TracingConfigProcessingError() from error + + return dump_response(TraceAppConfigResponse, {"result": "success"}, exclude_none=True), 201 @console_ns.doc("update_trace_app_config") @console_ns.doc(description="Update an existing tracing configuration for an application") @@ -130,52 +199,84 @@ class TraceAppConfigApi(Resource): "Tracing configuration updated successfully", console_ns.models[TraceAppConfigResponse.__name__], ) - @console_ns.response(400, "Invalid request parameters or configuration not found") + @console_ns.response(400, "Invalid request parameters or tracing configuration") @console_ns.response(403, "Insufficient permissions") - @setup_required - @login_required - @account_initialization_required - @edit_permission_required - @rbac_permission_required(RBACResourceScope.APP, RBACPermission.APP_TRACING_CONFIG) - @get_app_model + @console_ns.response(404, "Application or tracing configuration not found") + @console_ns.response(500, "Tracing configuration processing failed") + @console_account_admission( + allowed_roles=_APP_TRACING_CONFIG_EDIT_ROLES, + rbac_resource_scope=RBACResourceScope.APP, + rbac_permission=RBACPermission.APP_TRACING_CONFIG, + ) @model_validate(TraceConfigPayload) - def patch(self, req_data: TraceConfigPayload, app_model: App): + def patch( + self, + req_data: TraceConfigPayload, + request_context: RequestContext, + app_id: UUID, + ): """Update an existing trace app configuration""" try: - result = OpsService.update_tracing_app_config( - app_id=app_model.id, + application_services().app_tracing_configs.update( + context=request_context, + app_id=str(app_id), tracing_provider=req_data.tracing_provider, tracing_config=req_data.tracing_config, - session=db.session(), ) - if not result: - raise TracingConfigNotExist() - return {"result": "success"} - except Exception as e: - raise BadRequest(str(e)) + except AppTracingConfigAppNotFoundError as error: + raise AppNotFoundError() from error + except AppTracingConfigNotFoundError as error: + raise TracingConfigNotFoundError() from error + except AppTracingConfigInvalidProviderError as error: + raise UnsupportedTracingProviderError() from error + except AppTracingConfigInvalidConfigurationError as error: + raise InvalidTracingConfigError() from error + except AppTracingConfigVerificationFailedError as error: + raise TracingConfigVerificationFailedError() from error + except AppTracingConfigProcessingError as error: + raise TracingConfigProcessingError() from error + except ValueError as error: + raise TracingConfigProcessingError() from error + + return dump_response(TraceAppConfigResponse, {"result": "success"}, exclude_none=True) @console_ns.doc("delete_trace_app_config") @console_ns.doc(description="Delete an existing tracing configuration for an application") @console_ns.doc(params={"app_id": "Application ID"}) @console_ns.doc(params=query_params_from_model(TraceProviderQuery)) @console_ns.response(204, "Tracing configuration deleted successfully") - @console_ns.response(400, "Invalid request parameters or configuration not found") + @console_ns.response(400, "Invalid request parameters or unsupported tracing provider") @console_ns.response(403, "Insufficient permissions") - @setup_required - @login_required - @account_initialization_required - @edit_permission_required - @rbac_permission_required(RBACResourceScope.APP, RBACPermission.APP_TRACING_CONFIG) - @get_app_model + @console_ns.response(404, "Application or tracing configuration not found") + @console_ns.response(500, "Tracing configuration processing failed") + @console_account_admission( + allowed_roles=_APP_TRACING_CONFIG_EDIT_ROLES, + rbac_resource_scope=RBACResourceScope.APP, + rbac_permission=RBACPermission.APP_TRACING_CONFIG, + ) @model_validate(TraceProviderQuery) - def delete(self, req_data: TraceProviderQuery, app_model: App): + def delete( + self, + req_data: TraceProviderQuery, + request_context: RequestContext, + app_id: UUID, + ): """Delete an existing trace app configuration""" try: - result = OpsService.delete_tracing_app_config( - app_id=app_model.id, tracing_provider=req_data.tracing_provider, session=db.session() + application_services().app_tracing_configs.delete( + context=request_context, + app_id=str(app_id), + tracing_provider=req_data.tracing_provider, ) - if not result: - raise TracingConfigNotExist() - return "", 204 - except Exception as e: - raise BadRequest(str(e)) + except AppTracingConfigAppNotFoundError as error: + raise AppNotFoundError() from error + except AppTracingConfigNotFoundError as error: + raise TracingConfigNotFoundError() from error + except AppTracingConfigInvalidProviderError as error: + raise UnsupportedTracingProviderError() from error + except AppTracingConfigProcessingError as error: + raise TracingConfigProcessingError() from error + except ValueError as error: + raise TracingConfigProcessingError() from error + + return "", 204 diff --git a/api/extensions/ext_application_services.py b/api/extensions/ext_application_services.py index d4ed5580e9c..b68354b194d 100644 --- a/api/extensions/ext_application_services.py +++ b/api/extensions/ext_application_services.py @@ -41,6 +41,7 @@ from repositories.account_repository import SQLAlchemyAccountRepository from repositories.app_definition_query_repository import AppDefinitionQueryRepository from repositories.app_site_command_repository import AppSiteCommandRepository from repositories.app_statistic_query_repository import AppStatisticQueryRepository +from repositories.app_tracing_config_repository import SQLAlchemyAppTracingConfigRepository from repositories.data_source_api_key_auth_repository import SQLAlchemyDataSourceApiKeyAuthBindingRepository from repositories.data_source_oauth_binding_repository import SQLAlchemyDataSourceOAuthBindingRepository from repositories.explore_banner_query_repository import ExploreBannerQueryRepository @@ -134,6 +135,8 @@ from services.account_profile_service import AccountProfileService from services.app_definition_query_service import AppDefinitionQueryService from services.app_site_service import AppSiteService from services.app_statistic_query import AppStatisticQuery +from services.app_tracing_config_gateway import OpsTraceManagerGateway +from services.app_tracing_config_service import AppTracingConfigService from services.auth.data_source_api_key_auth_gateways import ( ProviderApiKeyAuthCredentialValidator, TenantApiKeyAuthCredentialEncryptor, @@ -244,6 +247,7 @@ class ApplicationServices: app_definitions: AppDefinitionQueryService app_sites: AppSiteService app_statistics: AppStatisticQuery + app_tracing_configs: AppTracingConfigService billing_portal: BillingPortalService compliance_downloads: ComplianceDownloadService data_source_api_key_auth: DataSourceApiKeyAuthService @@ -592,6 +596,10 @@ def build_application_services( sites=AppSiteCommandRepository(session_factory=database_client), ), app_statistics=AppStatisticQueryRepository(session_factory=database_client), + app_tracing_configs=AppTracingConfigService( + configs=SQLAlchemyAppTracingConfigRepository(session_factory=database_client), + provider=OpsTraceManagerGateway(), + ), billing_portal=BillingPortalService( accounts=accounts, get_subscription=BillingService.get_subscription, diff --git a/api/openapi/markdown/console-openapi.md b/api/openapi/markdown/console-openapi.md index 0fa45b01005..2bf4c995c66 100644 --- a/api/openapi/markdown/console-openapi.md +++ b/api/openapi/markdown/console-openapi.md @@ -3228,8 +3228,10 @@ Delete an existing tracing configuration for an application | Code | Description | | ---- | ----------- | | 204 | Tracing configuration deleted successfully | -| 400 | Invalid request parameters or configuration not found | +| 400 | Invalid request parameters or unsupported tracing provider | | 403 | Insufficient permissions | +| 404 | Application or tracing configuration not found | +| 500 | Tracing configuration processing failed | ### [GET] /apps/{app_id}/trace-config Get tracing configuration for an application @@ -3246,7 +3248,9 @@ Get tracing configuration for an application | Code | Description | Schema | | ---- | ----------- | ------ | | 200 | Tracing configuration retrieved successfully | **application/json**: [TraceAppConfigResponse](#traceappconfigresponse)
| -| 400 | Invalid request parameters | | +| 400 | Invalid request parameters or unsupported tracing provider | | +| 404 | Application not found | | +| 500 | Tracing configuration processing failed | | ### [PATCH] /apps/{app_id}/trace-config **Update an existing trace app configuration** @@ -3270,8 +3274,10 @@ Update an existing tracing configuration for an application | Code | Description | Schema | | ---- | ----------- | ------ | | 200 | Tracing configuration updated successfully | **application/json**: [TraceAppConfigResponse](#traceappconfigresponse)
| -| 400 | Invalid request parameters or configuration not found | | +| 400 | Invalid request parameters or tracing configuration | | | 403 | Insufficient permissions | | +| 404 | Application or tracing configuration not found | | +| 500 | Tracing configuration processing failed | | ### [POST] /apps/{app_id}/trace-config **Create a new trace app configuration** @@ -3295,8 +3301,11 @@ Create a new tracing configuration for an application | Code | Description | Schema | | ---- | ----------- | ------ | | 201 | Tracing configuration created successfully | **application/json**: [TraceAppConfigResponse](#traceappconfigresponse)
| -| 400 | Invalid request parameters or configuration already exists | | +| 400 | Invalid request parameters or tracing configuration | | | 403 | Insufficient permissions | | +| 404 | Application not found | | +| 409 | Tracing configuration already exists | | +| 500 | Tracing configuration processing failed | | ### [POST] /apps/{app_id}/trigger-enable **Update app trigger (enable/disable)** diff --git a/api/repositories/app_tracing_config_repository.py b/api/repositories/app_tracing_config_repository.py new file mode 100644 index 00000000000..1d40ad947c3 --- /dev/null +++ b/api/repositories/app_tracing_config_repository.py @@ -0,0 +1,128 @@ +"""SQLAlchemy persistence adapter for app tracing provider configurations.""" + +from typing import Any, override + +from sqlalchemy import select +from sqlalchemy.orm import Session, sessionmaker + +from models.enums import AppStatus +from models.model import App, TraceAppConfig +from services.app_tracing_config_service import ( + AppTracingConfigAppNotFoundError, + AppTracingConfigRecord, + AppTracingConfigStore, +) + + +class SQLAlchemyAppTracingConfigRepository(AppTracingConfigStore): + def __init__(self, *, session_factory: sessionmaker[Session]) -> None: + self._session_factory = session_factory + + @override + def get( + self, + *, + workspace_id: str, + app_id: str, + tracing_provider: str, + ) -> AppTracingConfigRecord | None: + with self._session_factory() as session: + self._require_app(session, workspace_id, app_id) + config = self._get_config(session, app_id, tracing_provider) + return self._to_record(config) if config is not None else None + + @override + def create( + self, + *, + workspace_id: str, + app_id: str, + tracing_provider: str, + tracing_config: dict[str, Any], + ) -> bool: + with self._session_factory.begin() as session: + self._require_app(session, workspace_id, app_id) + if self._get_config(session, app_id, tracing_provider) is not None: + return False + + session.add( + TraceAppConfig( + app_id=app_id, + tracing_provider=tracing_provider, + tracing_config=dict(tracing_config), + ) + ) + return True + + @override + def update( + self, + *, + workspace_id: str, + app_id: str, + tracing_provider: str, + tracing_config: dict[str, Any], + ) -> bool: + with self._session_factory.begin() as session: + self._require_app(session, workspace_id, app_id) + config = self._get_config(session, app_id, tracing_provider) + if config is None: + return False + + config.tracing_config = dict(tracing_config) + return True + + @override + def delete( + self, + *, + workspace_id: str, + app_id: str, + tracing_provider: str, + ) -> bool: + with self._session_factory.begin() as session: + self._require_app(session, workspace_id, app_id) + config = self._get_config(session, app_id, tracing_provider) + if config is None: + return False + + session.delete(config) + return True + + @staticmethod + def _require_app(session: Session, workspace_id: str, app_id: str) -> None: + app_exists = session.scalar( + select(App.id) + .where( + App.id == app_id, + App.tenant_id == workspace_id, + App.status == AppStatus.NORMAL, + ) + .limit(1) + ) + if app_exists is None: + raise AppTracingConfigAppNotFoundError + + @staticmethod + def _get_config(session: Session, app_id: str, tracing_provider: str) -> TraceAppConfig | None: + return session.scalar( + select(TraceAppConfig) + .where( + TraceAppConfig.app_id == app_id, + TraceAppConfig.tracing_provider == tracing_provider, + ) + .limit(1) + ) + + @staticmethod + def _to_record(config: TraceAppConfig) -> AppTracingConfigRecord: + tracing_config = dict(config.tracing_config) if config.tracing_config is not None else None + return AppTracingConfigRecord( + id=config.id, + app_id=config.app_id, + tracing_provider=config.tracing_provider, + tracing_config=tracing_config, + is_active=config.is_active, + created_at=config.created_at, + updated_at=config.updated_at, + ) diff --git a/api/services/app_tracing_config_gateway.py b/api/services/app_tracing_config_gateway.py new file mode 100644 index 00000000000..637260df48d --- /dev/null +++ b/api/services/app_tracing_config_gateway.py @@ -0,0 +1,184 @@ +"""Compatibility gateway for the legacy tracing provider implementations.""" + +import logging +from typing import Any, override + +from pydantic import ValidationError + +from core.ops.entities.config_entity import BaseTracingConfig +from core.ops.ops_trace_manager import OpsTraceManager, TracingProviderConfigEntry, provider_config_map +from services.app_tracing_config_service import ( + AppTracingConfigInvalidConfigurationError, + AppTracingConfigInvalidProviderError, + AppTracingConfigProcessingError, + AppTracingConfigVerificationFailedError, + TracingConfigProviderGateway, +) + +logger = logging.getLogger(__name__) + +_PROJECT_URL_FALLBACKS = { + "arize": "https://app.arize.com/", + "phoenix": "https://app.phoenix.arize.com/projects/", + "langsmith": "https://smith.langchain.com/", + "opik": "https://www.comet.com/opik/", + "weave": "https://wandb.ai/", + "aliyun": "https://arms.console.aliyun.com/", + "tencent": "https://console.cloud.tencent.com/apm", + "mlflow": "http://localhost:5000/", + "databricks": "https://www.databricks.com/", +} + + +class OpsTraceManagerGateway(TracingConfigProviderGateway): + @override + def validate_provider(self, tracing_provider: str) -> None: + self._provider_config(tracing_provider) + + @override + def prepare_new_config( + self, + *, + workspace_id: str, + tracing_provider: str, + tracing_config: dict[str, Any], + ) -> dict[str, Any]: + provider_config = self._provider_config(tracing_provider) + normalized_config = self._normalize_config(provider_config, tracing_config) + self._verify_config(normalized_config, tracing_provider) + + project_url = self._get_project_url_for_create(normalized_config, tracing_provider) + try: + encrypted_config = OpsTraceManager.encrypt_tracing_config( + workspace_id, + tracing_provider, + normalized_config, + ) + except Exception as error: + raise AppTracingConfigProcessingError from error + if project_url: + encrypted_config["project_url"] = project_url + return encrypted_config + + @override + def prepare_updated_config( + self, + *, + workspace_id: str, + tracing_provider: str, + tracing_config: dict[str, Any], + current_tracing_config: dict[str, Any] | None, + ) -> dict[str, Any]: + provider_config = self._provider_config(tracing_provider) + self._validate_config(provider_config, tracing_config) + try: + encrypted_config = OpsTraceManager.encrypt_tracing_config( + workspace_id, + tracing_provider, + dict(tracing_config), + current_tracing_config, + ) + decrypted_config = OpsTraceManager.decrypt_tracing_config( + workspace_id, + tracing_provider, + encrypted_config, + ) + except Exception as error: + raise AppTracingConfigProcessingError from error + + self._verify_config(decrypted_config, tracing_provider) + return encrypted_config + + @override + def present_config( + self, + *, + workspace_id: str, + tracing_provider: str, + tracing_config: dict[str, Any] | None, + ) -> dict[str, Any]: + if tracing_config is None: + raise AppTracingConfigProcessingError + + try: + decrypted_config = OpsTraceManager.decrypt_tracing_config( + workspace_id, + tracing_provider, + tracing_config, + ) + presented_config = OpsTraceManager.obfuscated_decrypt_token(tracing_provider, decrypted_config) + except Exception as error: + raise AppTracingConfigProcessingError from error + + if tracing_provider == "langfuse" and not decrypted_config.get("project_key"): + try: + project_key = OpsTraceManager.get_trace_config_project_key(decrypted_config, tracing_provider) + presented_config["project_url"] = f"{decrypted_config.get('host')}/project/{project_key}" + except Exception: + presented_config["project_url"] = f"{decrypted_config.get('host')}/" + elif tracing_provider in _PROJECT_URL_FALLBACKS and not decrypted_config.get("project_url"): + try: + presented_config["project_url"] = OpsTraceManager.get_trace_config_project_url( + decrypted_config, + tracing_provider, + ) + except Exception: + presented_config["project_url"] = _PROJECT_URL_FALLBACKS[tracing_provider] + + return presented_config + + @staticmethod + def _provider_config(tracing_provider: str) -> TracingProviderConfigEntry: + try: + return provider_config_map[tracing_provider] + except KeyError as error: + raise AppTracingConfigInvalidProviderError(tracing_provider) from error + + @staticmethod + def _normalize_config( + provider_config: TracingProviderConfigEntry, + tracing_config: dict[str, Any], + ) -> dict[str, Any]: + normalized_config = dict(tracing_config) + default_config = OpsTraceManagerGateway._validate_config(provider_config, normalized_config) + + default_values = default_config.model_dump() + for key in provider_config["other_keys"]: + if normalized_config.get(key) == "": + normalized_config[key] = default_values.get(key) + return normalized_config + + @staticmethod + def _validate_config( + provider_config: TracingProviderConfigEntry, + tracing_config: dict[str, Any], + ) -> BaseTracingConfig: + config_class: type[BaseTracingConfig] = provider_config["config_class"] + try: + return config_class.model_validate(tracing_config) + except ValidationError as error: + raise AppTracingConfigInvalidConfigurationError from error + + @staticmethod + def _verify_config(tracing_config: dict[str, Any], tracing_provider: str) -> None: + try: + is_effective = OpsTraceManager.check_trace_config_is_effective(tracing_config, tracing_provider) + except ValueError as error: + logger.warning("Tracing configuration verification failed for provider %s", tracing_provider, exc_info=True) + raise AppTracingConfigVerificationFailedError from error + if not is_effective: + raise AppTracingConfigVerificationFailedError + + @staticmethod + def _get_project_url_for_create(tracing_config: dict[str, Any], tracing_provider: str) -> str | None: + try: + if tracing_provider in ("arize", "phoenix"): + return OpsTraceManager.get_trace_config_project_url(tracing_config, tracing_provider) + if tracing_provider == "langfuse": + project_key = OpsTraceManager.get_trace_config_project_key(tracing_config, tracing_provider) + return f"{tracing_config.get('host')}/project/{project_key}" + if tracing_provider in ("langsmith", "opik", "mlflow", "databricks", "tencent"): + return OpsTraceManager.get_trace_config_project_url(tracing_config, tracing_provider) + except Exception: + return None + return None diff --git a/api/services/app_tracing_config_service.py b/api/services/app_tracing_config_service.py new file mode 100644 index 00000000000..10d3eac61a8 --- /dev/null +++ b/api/services/app_tracing_config_service.py @@ -0,0 +1,230 @@ +"""Application boundary for app tracing provider configurations.""" + +from dataclasses import dataclass, replace +from datetime import datetime +from typing import Any, Protocol + +from machinery.context import RequestContext + + +@dataclass(frozen=True, slots=True) +class AppTracingConfigRecord: + id: str + app_id: str + tracing_provider: str | None + tracing_config: dict[str, Any] | None + is_active: bool + created_at: datetime + updated_at: datetime + + +class AppTracingConfigStore(Protocol): + def get( + self, + *, + workspace_id: str, + app_id: str, + tracing_provider: str, + ) -> AppTracingConfigRecord | None: ... + + def create( + self, + *, + workspace_id: str, + app_id: str, + tracing_provider: str, + tracing_config: dict[str, Any], + ) -> bool: ... + + def update( + self, + *, + workspace_id: str, + app_id: str, + tracing_provider: str, + tracing_config: dict[str, Any], + ) -> bool: ... + + def delete( + self, + *, + workspace_id: str, + app_id: str, + tracing_provider: str, + ) -> bool: ... + + +class TracingConfigProviderGateway(Protocol): + def validate_provider(self, tracing_provider: str) -> None: ... + + def prepare_new_config( + self, + *, + workspace_id: str, + tracing_provider: str, + tracing_config: dict[str, Any], + ) -> dict[str, Any]: ... + + def prepare_updated_config( + self, + *, + workspace_id: str, + tracing_provider: str, + tracing_config: dict[str, Any], + current_tracing_config: dict[str, Any] | None, + ) -> dict[str, Any]: ... + + def present_config( + self, + *, + workspace_id: str, + tracing_provider: str, + tracing_config: dict[str, Any] | None, + ) -> dict[str, Any]: ... + + +class AppTracingConfigError(Exception): + """Base class for framework-neutral app tracing configuration failures.""" + + +class AppTracingConfigAppNotFoundError(AppTracingConfigError): + def __init__(self) -> None: + super().__init__("App not found") + + +class AppTracingConfigAlreadyExistsError(AppTracingConfigError): + def __init__(self) -> None: + super().__init__("Trace config is exist.") + + +class AppTracingConfigNotFoundError(AppTracingConfigError): + def __init__(self) -> None: + super().__init__("Trace config not exist.") + + +class AppTracingConfigInvalidProviderError(AppTracingConfigError): + def __init__(self, tracing_provider: str) -> None: + super().__init__(f"Invalid tracing provider: {tracing_provider}") + + +class AppTracingConfigInvalidConfigurationError(AppTracingConfigError): + """The submitted provider configuration does not match its schema.""" + + def __init__(self) -> None: + super().__init__("Invalid tracing configuration") + + +class AppTracingConfigVerificationFailedError(AppTracingConfigError): + """The submitted configuration could not be verified by its provider.""" + + def __init__(self) -> None: + super().__init__("Tracing configuration verification failed") + + +class AppTracingConfigProcessingError(AppTracingConfigError): + """A validated or stored configuration could not be processed internally.""" + + def __init__(self) -> None: + super().__init__("Tracing configuration processing failed") + + +class AppTracingConfigService: + def __init__(self, *, configs: AppTracingConfigStore, provider: TracingConfigProviderGateway) -> None: + self._configs = configs + self._provider = provider + + def get( + self, + context: RequestContext, + app_id: str, + tracing_provider: str, + ) -> AppTracingConfigRecord | None: + workspace_id = context.active_workspace_id + self._provider.validate_provider(tracing_provider) + record = self._configs.get( + workspace_id=workspace_id, + app_id=app_id, + tracing_provider=tracing_provider, + ) + if record is None: + return None + + tracing_config = self._provider.present_config( + workspace_id=workspace_id, + tracing_provider=tracing_provider, + tracing_config=record.tracing_config, + ) + return replace(record, tracing_config=tracing_config) + + def create( + self, + context: RequestContext, + app_id: str, + tracing_provider: str, + tracing_config: dict[str, Any], + ) -> None: + workspace_id = context.active_workspace_id + current = self._configs.get( + workspace_id=workspace_id, + app_id=app_id, + tracing_provider=tracing_provider, + ) + if current is not None: + raise AppTracingConfigAlreadyExistsError + + encrypted_config = self._provider.prepare_new_config( + workspace_id=workspace_id, + tracing_provider=tracing_provider, + tracing_config=tracing_config, + ) + + created = self._configs.create( + workspace_id=workspace_id, + app_id=app_id, + tracing_provider=tracing_provider, + tracing_config=encrypted_config, + ) + if not created: + raise AppTracingConfigAlreadyExistsError + + def update( + self, + context: RequestContext, + app_id: str, + tracing_provider: str, + tracing_config: dict[str, Any], + ) -> None: + workspace_id = context.active_workspace_id + current = self._configs.get( + workspace_id=workspace_id, + app_id=app_id, + tracing_provider=tracing_provider, + ) + self._provider.validate_provider(tracing_provider) + if current is None: + raise AppTracingConfigNotFoundError + + encrypted_config = self._provider.prepare_updated_config( + workspace_id=workspace_id, + tracing_provider=tracing_provider, + tracing_config=tracing_config, + current_tracing_config=current.tracing_config, + ) + updated = self._configs.update( + workspace_id=workspace_id, + app_id=app_id, + tracing_provider=tracing_provider, + tracing_config=encrypted_config, + ) + if not updated: + raise AppTracingConfigNotFoundError + + def delete(self, context: RequestContext, app_id: str, tracing_provider: str) -> None: + self._provider.validate_provider(tracing_provider) + deleted = self._configs.delete( + workspace_id=context.active_workspace_id, + app_id=app_id, + tracing_provider=tracing_provider, + ) + if not deleted: + raise AppTracingConfigNotFoundError diff --git a/api/services/ops_service.py b/api/services/ops_service.py deleted file mode 100644 index b6f17168b3c..00000000000 --- a/api/services/ops_service.py +++ /dev/null @@ -1,282 +0,0 @@ -from typing import Any - -from sqlalchemy import select -from sqlalchemy.orm import Session - -from core.ops.entities.config_entity import BaseTracingConfig -from core.ops.ops_trace_manager import OpsTraceManager, TracingProviderConfigEntry, provider_config_map -from models.model import App, TraceAppConfig - - -class OpsService: - @classmethod - def get_tracing_app_config(cls, app_id: str, tracing_provider: str, session: Session): - """ - Get tracing app config - :param app_id: app id - :param tracing_provider: tracing provider - :return: - """ - trace_config_data: TraceAppConfig | None = session.scalar( - select(TraceAppConfig) - .where(TraceAppConfig.app_id == app_id, TraceAppConfig.tracing_provider == tracing_provider) - .limit(1) - ) - - if not trace_config_data: - return None - - # decrypt_token and obfuscated_token - app = session.get(App, app_id) - if not app: - return None - tenant_id = app.tenant_id - if trace_config_data.tracing_config is None: - raise ValueError("Tracing config cannot be None.") - decrypt_tracing_config = OpsTraceManager.decrypt_tracing_config( - tenant_id, tracing_provider, trace_config_data.tracing_config - ) - new_decrypt_tracing_config = OpsTraceManager.obfuscated_decrypt_token(tracing_provider, decrypt_tracing_config) - - if tracing_provider == "arize" and ( - "project_url" not in decrypt_tracing_config or not decrypt_tracing_config.get("project_url") - ): - try: - project_url = OpsTraceManager.get_trace_config_project_url(decrypt_tracing_config, tracing_provider) - new_decrypt_tracing_config.update({"project_url": project_url}) - except Exception: - new_decrypt_tracing_config.update({"project_url": "https://app.arize.com/"}) - - if tracing_provider == "phoenix" and ( - "project_url" not in decrypt_tracing_config or not decrypt_tracing_config.get("project_url") - ): - try: - project_url = OpsTraceManager.get_trace_config_project_url(decrypt_tracing_config, tracing_provider) - new_decrypt_tracing_config.update({"project_url": project_url}) - except Exception: - new_decrypt_tracing_config.update({"project_url": "https://app.phoenix.arize.com/projects/"}) - - if tracing_provider == "langfuse" and ( - "project_key" not in decrypt_tracing_config or not decrypt_tracing_config.get("project_key") - ): - try: - project_key = OpsTraceManager.get_trace_config_project_key(decrypt_tracing_config, tracing_provider) - new_decrypt_tracing_config.update( - { - "project_url": "{host}/project/{key}".format( - host=decrypt_tracing_config.get("host"), key=project_key - ) - } - ) - except Exception: - new_decrypt_tracing_config.update({"project_url": f"{decrypt_tracing_config.get('host')}/"}) - - if tracing_provider == "langsmith" and ( - "project_url" not in decrypt_tracing_config or not decrypt_tracing_config.get("project_url") - ): - try: - project_url = OpsTraceManager.get_trace_config_project_url(decrypt_tracing_config, tracing_provider) - new_decrypt_tracing_config.update({"project_url": project_url}) - except Exception: - new_decrypt_tracing_config.update({"project_url": "https://smith.langchain.com/"}) - - if tracing_provider == "opik" and ( - "project_url" not in decrypt_tracing_config or not decrypt_tracing_config.get("project_url") - ): - try: - project_url = OpsTraceManager.get_trace_config_project_url(decrypt_tracing_config, tracing_provider) - new_decrypt_tracing_config.update({"project_url": project_url}) - except Exception: - new_decrypt_tracing_config.update({"project_url": "https://www.comet.com/opik/"}) - if tracing_provider == "weave" and ( - "project_url" not in decrypt_tracing_config or not decrypt_tracing_config.get("project_url") - ): - try: - project_url = OpsTraceManager.get_trace_config_project_url(decrypt_tracing_config, tracing_provider) - new_decrypt_tracing_config.update({"project_url": project_url}) - except Exception: - new_decrypt_tracing_config.update({"project_url": "https://wandb.ai/"}) - - if tracing_provider == "aliyun" and ( - "project_url" not in decrypt_tracing_config or not decrypt_tracing_config.get("project_url") - ): - try: - project_url = OpsTraceManager.get_trace_config_project_url(decrypt_tracing_config, tracing_provider) - new_decrypt_tracing_config.update({"project_url": project_url}) - except Exception: - new_decrypt_tracing_config.update({"project_url": "https://arms.console.aliyun.com/"}) - - if tracing_provider == "tencent" and ( - "project_url" not in decrypt_tracing_config or not decrypt_tracing_config.get("project_url") - ): - try: - project_url = OpsTraceManager.get_trace_config_project_url(decrypt_tracing_config, tracing_provider) - new_decrypt_tracing_config.update({"project_url": project_url}) - except Exception: - new_decrypt_tracing_config.update({"project_url": "https://console.cloud.tencent.com/apm"}) - - if tracing_provider == "mlflow" and ( - "project_url" not in decrypt_tracing_config or not decrypt_tracing_config.get("project_url") - ): - try: - project_url = OpsTraceManager.get_trace_config_project_url(decrypt_tracing_config, tracing_provider) - new_decrypt_tracing_config.update({"project_url": project_url}) - except Exception: - new_decrypt_tracing_config.update({"project_url": "http://localhost:5000/"}) - - if tracing_provider == "databricks" and ( - "project_url" not in decrypt_tracing_config or not decrypt_tracing_config.get("project_url") - ): - try: - project_url = OpsTraceManager.get_trace_config_project_url(decrypt_tracing_config, tracing_provider) - new_decrypt_tracing_config.update({"project_url": project_url}) - except Exception: - new_decrypt_tracing_config.update({"project_url": "https://www.databricks.com/"}) - - trace_config_data.tracing_config = new_decrypt_tracing_config - return trace_config_data.to_dict() - - @classmethod - def create_tracing_app_config( - cls, app_id: str, tracing_provider: str, tracing_config: dict[str, Any], session: Session - ): - """ - Create tracing app config - :param app_id: app id - :param tracing_provider: tracing provider - :param tracing_config: tracing config - :return: - """ - try: - provider_config_map[tracing_provider] - except KeyError: - return {"error": f"Invalid tracing provider: {tracing_provider}"} - - provider_config: TracingProviderConfigEntry = provider_config_map[tracing_provider] - config_class: type[BaseTracingConfig] = provider_config["config_class"] - other_keys: list[str] = provider_config["other_keys"] - - default_config_instance = config_class.model_validate(tracing_config) - for key in other_keys: - if key in tracing_config and tracing_config[key] == "": - tracing_config[key] = getattr(default_config_instance, key, None) - - # api check - if not OpsTraceManager.check_trace_config_is_effective(tracing_config, tracing_provider): - return {"error": "Invalid Credentials"} - - # get project url - if tracing_provider in ("arize", "phoenix"): - try: - project_url = OpsTraceManager.get_trace_config_project_url(tracing_config, tracing_provider) - except Exception: - project_url = None - elif tracing_provider == "langfuse": - try: - project_key = OpsTraceManager.get_trace_config_project_key(tracing_config, tracing_provider) - project_url = f"{tracing_config.get('host')}/project/{project_key}" - except Exception: - project_url = None - elif tracing_provider in ("langsmith", "opik", "mlflow", "databricks", "tencent"): - try: - project_url = OpsTraceManager.get_trace_config_project_url(tracing_config, tracing_provider) - except Exception: - project_url = None - else: - project_url = None - - # check if trace config already exists - trace_config_data: TraceAppConfig | None = session.scalar( - select(TraceAppConfig) - .where(TraceAppConfig.app_id == app_id, TraceAppConfig.tracing_provider == tracing_provider) - .limit(1) - ) - - if trace_config_data: - return None - - # get tenant id - app = session.get(App, app_id) - if not app: - return None - tenant_id = app.tenant_id - tracing_config = OpsTraceManager.encrypt_tracing_config(tenant_id, tracing_provider, tracing_config) - if project_url: - tracing_config["project_url"] = project_url - trace_config_data = TraceAppConfig( - app_id=app_id, - tracing_provider=tracing_provider, - tracing_config=tracing_config, - ) - session.add(trace_config_data) - session.commit() - - return {"result": "success"} - - @classmethod - def update_tracing_app_config( - cls, app_id: str, tracing_provider: str, tracing_config: dict[str, Any], session: Session - ): - """ - Update tracing app config - :param app_id: app id - :param tracing_provider: tracing provider - :param tracing_config: tracing config - :return: - """ - try: - provider_config_map[tracing_provider] - except KeyError: - raise ValueError(f"Invalid tracing provider: {tracing_provider}") - - # check if trace config already exists - current_trace_config = session.scalar( - select(TraceAppConfig) - .where(TraceAppConfig.app_id == app_id, TraceAppConfig.tracing_provider == tracing_provider) - .limit(1) - ) - - if not current_trace_config: - return None - - # get tenant id - app = session.get(App, app_id) - if not app: - return None - tenant_id = app.tenant_id - tracing_config = OpsTraceManager.encrypt_tracing_config( - tenant_id, tracing_provider, tracing_config, current_trace_config.tracing_config - ) - - # api check - # decrypt_token - decrypt_tracing_config = OpsTraceManager.decrypt_tracing_config(tenant_id, tracing_provider, tracing_config) - if not OpsTraceManager.check_trace_config_is_effective(decrypt_tracing_config, tracing_provider): - raise ValueError("Invalid Credentials") - - current_trace_config.tracing_config = tracing_config - session.commit() - - return current_trace_config.to_dict() - - @classmethod - def delete_tracing_app_config(cls, app_id: str, tracing_provider: str, session: Session): - """ - Delete tracing app config - :param app_id: app id - :param tracing_provider: tracing provider - :return: - """ - trace_config = session.scalar( - select(TraceAppConfig) - .where(TraceAppConfig.app_id == app_id, TraceAppConfig.tracing_provider == tracing_provider) - .limit(1) - ) - - if not trace_config: - return None - - session.delete(trace_config) - session.commit() - - return True diff --git a/api/tests/test_containers_integration_tests/.ruff.toml b/api/tests/test_containers_integration_tests/.ruff.toml index a58180c68b1..4124d24b082 100644 --- a/api/tests/test_containers_integration_tests/.ruff.toml +++ b/api/tests/test_containers_integration_tests/.ruff.toml @@ -41,7 +41,6 @@ extend-select = ["ANN401", "ARG"] "services/test_metadata_service.py" = ["ARG002"] "services/test_model_load_balancing_service.py" = ["ARG002"] "services/test_model_provider_service.py" = ["ARG002"] -"services/test_ops_service.py" = ["ARG002"] "services/test_webapp_auth_service.py" = ["ARG002"] "services/test_webhook_service.py" = ["ARG002"] "services/test_workflow_draft_variable_service.py" = ["ARG002"] diff --git a/api/tests/test_containers_integration_tests/pyrefly.toml b/api/tests/test_containers_integration_tests/pyrefly.toml index cd654925428..bf1701447c7 100644 --- a/api/tests/test_containers_integration_tests/pyrefly.toml +++ b/api/tests/test_containers_integration_tests/pyrefly.toml @@ -80,7 +80,6 @@ project-excludes = [ "services/test_messages_clean_service.py", "services/test_model_load_balancing_service.py", "services/test_model_provider_service.py", - "services/test_ops_service.py", "services/test_restore_archived_workflow_run.py", "services/test_saved_message_service.py", "services/test_schedule_service.py", diff --git a/api/tests/test_containers_integration_tests/services/test_ops_service.py b/api/tests/test_containers_integration_tests/services/test_ops_service.py deleted file mode 100644 index 029c6ba9420..00000000000 --- a/api/tests/test_containers_integration_tests/services/test_ops_service.py +++ /dev/null @@ -1,385 +0,0 @@ -from __future__ import annotations - -import uuid -from unittest.mock import patch - -import pytest -from faker import Faker -from sqlalchemy import select -from sqlalchemy.orm import Session - -from core.ops.entities.config_entity import TracingProviderEnum -from models.model import TraceAppConfig -from services.account_service import AccountService, TenantService -from services.app_service import AppService, CreateAppParams -from services.ops_service import OpsService -from tests.test_containers_integration_tests.helpers import generate_valid_password - - -class TestOpsService: - @pytest.fixture - def mock_external_service_dependencies(self): - with ( - patch("services.app_service.SystemFeatureService") as mock_feature_service, - patch("services.app_service.EnterpriseService") as mock_enterprise_service, - patch("services.app_service.ModelManager.for_tenant") as mock_model_manager, - patch("services.account_service.SystemFeatureService") as mock_account_feature_service, - ): - mock_feature_service.is_webapp_auth_enabled.return_value = False - mock_enterprise_service.WebAppAuth.update_app_access_mode.return_value = None - mock_enterprise_service.WebAppAuth.cleanup_webapp.return_value = None - mock_account_feature_service.is_registration_allowed.return_value = True - mock_model_instance = mock_model_manager.return_value - mock_model_instance.get_default_model_instance.return_value = None - mock_model_instance.get_default_provider_model_name.return_value = ("openai", "gpt-3.5-turbo") - yield { - "feature_service": mock_feature_service, - "enterprise_service": mock_enterprise_service, - "model_manager": mock_model_manager, - "account_feature_service": mock_account_feature_service, - } - - @pytest.fixture - def mock_ops_trace_manager(self): - with patch("services.ops_service.OpsTraceManager") as mock: - yield mock - - def _create_app(self, db_session_with_containers: Session, mock_external_service_dependencies): - fake = Faker() - account = AccountService.create_account( - email=fake.email(), - name=fake.name(), - interface_language="en-US", - password=generate_valid_password(fake), - session=db_session_with_containers, - ) - TenantService.create_owner_tenant_if_not_exist(account, name=fake.company(), session=db_session_with_containers) - tenant = account.current_tenant - app_service = AppService() - app = app_service.create_app( - tenant.id, - CreateAppParams( - name=fake.company(), - description=fake.text(max_nb_chars=100), - mode="chat", - icon_type="emoji", - icon="🤖", - icon_background="#FF6B6B", - ), - account, - session=db_session_with_containers, - ) - return app, account - - _SENTINEL = object() - - def _insert_trace_config( - self, - db_session: Session, - app_id: str, - provider: str, - tracing_config: dict | None | object = _SENTINEL, - ) -> TraceAppConfig: - trace_config = TraceAppConfig( - app_id=app_id, - tracing_provider=provider, - tracing_config=tracing_config if tracing_config is not self._SENTINEL else {"some": "config"}, - ) - db_session.add(trace_config) - db_session.commit() - return trace_config - - # ── get_tracing_app_config ───────────────────────────────────────── - - def test_get_tracing_app_config_no_config(self, db_session_with_containers: Session, mock_ops_trace_manager): - result = OpsService.get_tracing_app_config(str(uuid.uuid4()), "arize", db_session_with_containers) - assert result is None - - def test_get_tracing_app_config_no_app(self, db_session_with_containers: Session, mock_ops_trace_manager): - fake_app_id = str(uuid.uuid4()) - self._insert_trace_config(db_session_with_containers, fake_app_id, "arize") - result = OpsService.get_tracing_app_config(fake_app_id, "arize", db_session_with_containers) - assert result is None - - def test_get_tracing_app_config_none_config( - self, db_session_with_containers: Session, mock_external_service_dependencies, mock_ops_trace_manager - ): - app, _ = self._create_app(db_session_with_containers, mock_external_service_dependencies) - self._insert_trace_config(db_session_with_containers, app.id, "arize", tracing_config=None) - - with pytest.raises(ValueError, match="Tracing config cannot be None."): - OpsService.get_tracing_app_config(app.id, "arize", db_session_with_containers) - - @pytest.mark.parametrize( - ("provider", "default_url"), - [ - ("arize", "https://app.arize.com/"), - ("phoenix", "https://app.phoenix.arize.com/projects/"), - ("langsmith", "https://smith.langchain.com/"), - ("opik", "https://www.comet.com/opik/"), - ("weave", "https://wandb.ai/"), - ("aliyun", "https://arms.console.aliyun.com/"), - ("tencent", "https://console.cloud.tencent.com/apm"), - ("mlflow", "http://localhost:5000/"), - ("databricks", "https://www.databricks.com/"), - ], - ) - def test_get_tracing_app_config_providers_exception( - self, db_session_with_containers: Session, mock_external_service_dependencies, provider, default_url - ): - with patch("services.ops_service.OpsTraceManager") as mock_otm: - mock_otm.decrypt_tracing_config.return_value = {} - mock_otm.obfuscated_decrypt_token.return_value = {} - mock_otm.get_trace_config_project_url.side_effect = Exception("error") - mock_otm.get_trace_config_project_key.side_effect = Exception("error") - - app, _ = self._create_app(db_session_with_containers, mock_external_service_dependencies) - self._insert_trace_config(db_session_with_containers, app.id, provider) - - result = OpsService.get_tracing_app_config(app.id, provider, db_session_with_containers) - - assert result is not None - assert result["tracing_config"]["project_url"] == default_url - - @pytest.mark.parametrize( - "provider", - ["arize", "phoenix", "langsmith", "opik", "weave", "aliyun", "tencent", "mlflow", "databricks"], - ) - def test_get_tracing_app_config_providers_success( - self, db_session_with_containers: Session, mock_external_service_dependencies, provider - ): - with patch("services.ops_service.OpsTraceManager") as mock_otm: - mock_otm.decrypt_tracing_config.return_value = {} - mock_otm.obfuscated_decrypt_token.return_value = {"project_url": "success_url"} - mock_otm.get_trace_config_project_url.return_value = "success_url" - - app, _ = self._create_app(db_session_with_containers, mock_external_service_dependencies) - self._insert_trace_config(db_session_with_containers, app.id, provider) - - result = OpsService.get_tracing_app_config(app.id, provider, db_session_with_containers) - - assert result is not None - assert result["tracing_config"]["project_url"] == "success_url" - - def test_get_tracing_app_config_langfuse_success( - self, db_session_with_containers: Session, mock_external_service_dependencies - ): - with patch("services.ops_service.OpsTraceManager") as mock_otm: - mock_otm.decrypt_tracing_config.return_value = {"host": "https://api.langfuse.com"} - mock_otm.obfuscated_decrypt_token.return_value = {"host": "https://api.langfuse.com"} - mock_otm.get_trace_config_project_key.return_value = "key" - - app, _ = self._create_app(db_session_with_containers, mock_external_service_dependencies) - self._insert_trace_config(db_session_with_containers, app.id, "langfuse") - - result = OpsService.get_tracing_app_config(app.id, "langfuse", db_session_with_containers) - - assert result is not None - assert result["tracing_config"]["project_url"] == "https://api.langfuse.com/project/key" - - def test_get_tracing_app_config_langfuse_exception( - self, db_session_with_containers: Session, mock_external_service_dependencies - ): - with patch("services.ops_service.OpsTraceManager") as mock_otm: - mock_otm.decrypt_tracing_config.return_value = {"host": "https://api.langfuse.com"} - mock_otm.obfuscated_decrypt_token.return_value = {"host": "https://api.langfuse.com"} - mock_otm.get_trace_config_project_key.side_effect = Exception("error") - - app, _ = self._create_app(db_session_with_containers, mock_external_service_dependencies) - self._insert_trace_config(db_session_with_containers, app.id, "langfuse") - - result = OpsService.get_tracing_app_config(app.id, "langfuse", db_session_with_containers) - - assert result is not None - assert result["tracing_config"]["project_url"] == "https://api.langfuse.com/" - - # ── create_tracing_app_config ────────────────────────────────────── - - def test_create_tracing_app_config_invalid_provider(self, db_session_with_containers: Session): - result = OpsService.create_tracing_app_config( - str(uuid.uuid4()), "invalid_provider", {}, db_session_with_containers - ) - assert result == {"error": "Invalid tracing provider: invalid_provider"} - - def test_create_tracing_app_config_invalid_credentials( - self, db_session_with_containers: Session, mock_ops_trace_manager - ): - mock_ops_trace_manager.check_trace_config_is_effective.return_value = False - result = OpsService.create_tracing_app_config( - str(uuid.uuid4()), - TracingProviderEnum.LANGFUSE, - {"public_key": "p", "secret_key": "s"}, - db_session_with_containers, - ) - assert result == {"error": "Invalid Credentials"} - - @pytest.mark.parametrize( - ("provider", "config"), - [ - (TracingProviderEnum.ARIZE, {}), - (TracingProviderEnum.LANGFUSE, {"public_key": "p", "secret_key": "s"}), - (TracingProviderEnum.LANGSMITH, {"api_key": "k", "project": "p"}), - (TracingProviderEnum.ALIYUN, {"license_key": "k", "endpoint": "https://aliyun.com"}), - ], - ) - def test_create_tracing_app_config_project_url_exception( - self, db_session_with_containers: Session, mock_external_service_dependencies, provider, config - ): - # Existing config causes the service to return None before reaching the DB insert - with patch("services.ops_service.OpsTraceManager") as mock_otm: - mock_otm.check_trace_config_is_effective.return_value = True - mock_otm.get_trace_config_project_url.side_effect = Exception("error") - mock_otm.get_trace_config_project_key.side_effect = Exception("error") - - app, _ = self._create_app(db_session_with_containers, mock_external_service_dependencies) - self._insert_trace_config(db_session_with_containers, app.id, str(provider)) - - result = OpsService.create_tracing_app_config(app.id, provider, config, db_session_with_containers) - - assert result is None - - def test_create_tracing_app_config_langfuse_success( - self, db_session_with_containers: Session, mock_external_service_dependencies - ): - with patch("services.ops_service.OpsTraceManager") as mock_otm: - mock_otm.check_trace_config_is_effective.return_value = True - mock_otm.get_trace_config_project_key.return_value = "key" - mock_otm.encrypt_tracing_config.return_value = {} - - app, _ = self._create_app(db_session_with_containers, mock_external_service_dependencies) - result = OpsService.create_tracing_app_config( - app.id, - TracingProviderEnum.LANGFUSE, - {"public_key": "p", "secret_key": "s", "host": "https://api.langfuse.com"}, - db_session_with_containers, - ) - - assert result == {"result": "success"} - - def test_create_tracing_app_config_already_exists( - self, db_session_with_containers: Session, mock_external_service_dependencies - ): - with patch("services.ops_service.OpsTraceManager") as mock_otm: - mock_otm.check_trace_config_is_effective.return_value = True - - app, _ = self._create_app(db_session_with_containers, mock_external_service_dependencies) - self._insert_trace_config(db_session_with_containers, app.id, str(TracingProviderEnum.ARIZE)) - - result = OpsService.create_tracing_app_config( - app.id, TracingProviderEnum.ARIZE, {}, db_session_with_containers - ) - - assert result is None - - def test_create_tracing_app_config_no_app(self, db_session_with_containers: Session, mock_ops_trace_manager): - mock_ops_trace_manager.check_trace_config_is_effective.return_value = True - result = OpsService.create_tracing_app_config( - str(uuid.uuid4()), TracingProviderEnum.ARIZE, {}, db_session_with_containers - ) - assert result is None - - def test_create_tracing_app_config_with_empty_other_keys( - self, db_session_with_containers: Session, mock_external_service_dependencies - ): - # "project" is in other_keys for Arize; providing "" triggers default substitution - with patch("services.ops_service.OpsTraceManager") as mock_otm: - mock_otm.check_trace_config_is_effective.return_value = True - mock_otm.get_trace_config_project_url.side_effect = Exception("no url") - mock_otm.encrypt_tracing_config.return_value = {} - - app, _ = self._create_app(db_session_with_containers, mock_external_service_dependencies) - result = OpsService.create_tracing_app_config( - app.id, TracingProviderEnum.ARIZE, {"project": ""}, db_session_with_containers - ) - - assert result == {"result": "success"} - - def test_create_tracing_app_config_success( - self, db_session_with_containers: Session, mock_external_service_dependencies - ): - with patch("services.ops_service.OpsTraceManager") as mock_otm: - mock_otm.check_trace_config_is_effective.return_value = True - mock_otm.get_trace_config_project_url.return_value = "http://project_url" - mock_otm.encrypt_tracing_config.return_value = {"encrypted": "config"} - - app, _ = self._create_app(db_session_with_containers, mock_external_service_dependencies) - result = OpsService.create_tracing_app_config( - app.id, TracingProviderEnum.ARIZE, {}, db_session_with_containers - ) - - assert result == {"result": "success"} - - # ── update_tracing_app_config ────────────────────────────────────── - - def test_update_tracing_app_config_invalid_provider(self, db_session_with_containers: Session): - with pytest.raises(ValueError, match="Invalid tracing provider: invalid_provider"): - OpsService.update_tracing_app_config(str(uuid.uuid4()), "invalid_provider", {}, db_session_with_containers) - - def test_update_tracing_app_config_no_config(self, db_session_with_containers: Session, mock_ops_trace_manager): - result = OpsService.update_tracing_app_config( - str(uuid.uuid4()), TracingProviderEnum.ARIZE, {}, db_session_with_containers - ) - assert result is None - - def test_update_tracing_app_config_no_app(self, db_session_with_containers: Session, mock_ops_trace_manager): - fake_app_id = str(uuid.uuid4()) - self._insert_trace_config(db_session_with_containers, fake_app_id, str(TracingProviderEnum.ARIZE)) - mock_ops_trace_manager.encrypt_tracing_config.return_value = {} - result = OpsService.update_tracing_app_config( - fake_app_id, TracingProviderEnum.ARIZE, {}, db_session_with_containers - ) - assert result is None - - def test_update_tracing_app_config_invalid_credentials( - self, db_session_with_containers: Session, mock_external_service_dependencies - ): - with patch("services.ops_service.OpsTraceManager") as mock_otm: - mock_otm.encrypt_tracing_config.return_value = {} - mock_otm.decrypt_tracing_config.return_value = {} - mock_otm.check_trace_config_is_effective.return_value = False - - app, _ = self._create_app(db_session_with_containers, mock_external_service_dependencies) - self._insert_trace_config(db_session_with_containers, app.id, str(TracingProviderEnum.ARIZE)) - - with pytest.raises(ValueError, match="Invalid Credentials"): - OpsService.update_tracing_app_config(app.id, TracingProviderEnum.ARIZE, {}, db_session_with_containers) - - def test_update_tracing_app_config_success( - self, db_session_with_containers: Session, mock_external_service_dependencies - ): - with patch("services.ops_service.OpsTraceManager") as mock_otm: - mock_otm.encrypt_tracing_config.return_value = {"updated": "config"} - mock_otm.decrypt_tracing_config.return_value = {} - mock_otm.check_trace_config_is_effective.return_value = True - - app, _ = self._create_app(db_session_with_containers, mock_external_service_dependencies) - self._insert_trace_config(db_session_with_containers, app.id, str(TracingProviderEnum.ARIZE)) - - result = OpsService.update_tracing_app_config( - app.id, TracingProviderEnum.ARIZE, {}, db_session_with_containers - ) - - assert result is not None - assert result["app_id"] == app.id - - # ── delete_tracing_app_config ────────────────────────────────────── - - def test_delete_tracing_app_config_no_config(self, db_session_with_containers: Session): - result = OpsService.delete_tracing_app_config(str(uuid.uuid4()), "arize", db_session_with_containers) - assert result is None - - def test_delete_tracing_app_config_success( - self, db_session_with_containers: Session, mock_external_service_dependencies - ): - app, _ = self._create_app(db_session_with_containers, mock_external_service_dependencies) - self._insert_trace_config(db_session_with_containers, app.id, "arize") - - result = OpsService.delete_tracing_app_config(app.id, "arize", db_session_with_containers) - - assert result is True - remaining = db_session_with_containers.scalar( - select(TraceAppConfig) - .where(TraceAppConfig.app_id == app.id, TraceAppConfig.tracing_provider == "arize") - .limit(1) - ) - assert remaining is None diff --git a/api/tests/unit_tests/controllers/console/app/test_app_apis.py b/api/tests/unit_tests/controllers/console/app/test_app_apis.py index 0e25b43928e..461afa9efd4 100644 --- a/api/tests/unit_tests/controllers/console/app/test_app_apis.py +++ b/api/tests/unit_tests/controllers/console/app/test_app_apis.py @@ -53,7 +53,11 @@ from controllers.console.app import ( wraps as wraps_module, ) from controllers.console.app.completion import ChatMessagePayload, CompletionMessagePayload -from controllers.console.app.error import AppNotFoundError +from controllers.console.app.error import ( + AppNotFoundError, + TracingConfigNotFoundError, + TracingConfigVerificationFailedError, +) from controllers.console.app.mcp_server import MCPServerCreatePayload, MCPServerUpdatePayload from controllers.console.app.ops_trace import TraceConfigPayload, TraceProviderQuery from controllers.console.app.site import AppSiteUpdatePayload @@ -77,6 +81,10 @@ from services.app_site_service import ( AppSiteCommandResult, AppSiteNotFoundError, ) +from services.app_tracing_config_service import ( + AppTracingConfigNotFoundError, + AppTracingConfigVerificationFailedError, +) from tests.unit_tests.config_override import apply_config_overrides APP_ID = "11111111-1111-1111-1111-111111111111" @@ -387,52 +395,71 @@ class TestOpsTraceEndpoints: def test_trace_app_config_get_empty(self, app: Flask, monkeypatch: pytest.MonkeyPatch): api = ops_trace_module.TraceAppConfigApi() method = unwrap(api.get) - + tracing_configs = MagicMock() + tracing_configs.get.return_value = None monkeypatch.setattr( - ops_trace_module.OpsService, - "get_tracing_app_config", - lambda **_kwargs: None, + ops_trace_module, + "application_services", + lambda: SimpleNamespace(app_tracing_configs=tracing_configs), ) with app.test_request_context("/?tracing_provider=langfuse"): - result = method(api, TraceProviderQuery(tracing_provider="langfuse"), _make_app()) + result = method( + api, + TraceProviderQuery(tracing_provider="langfuse"), + _make_request_context(), + uuid.UUID(APP_ID), + ) assert result == {"has_not_configured": True} + tracing_configs.get.assert_called_once_with( + context=_make_request_context(), + app_id=APP_ID, + tracing_provider="langfuse", + ) def test_trace_app_config_post_invalid(self, app: Flask, monkeypatch: pytest.MonkeyPatch): api = ops_trace_module.TraceAppConfigApi() method = unwrap(api.post) - + tracing_configs = MagicMock() + tracing_configs.create.side_effect = AppTracingConfigVerificationFailedError() monkeypatch.setattr( - ops_trace_module.OpsService, - "create_tracing_app_config", - lambda **_kwargs: {"error": True}, + ops_trace_module, + "application_services", + lambda: SimpleNamespace(app_tracing_configs=tracing_configs), ) with app.test_request_context( "/", json={"tracing_provider": "langfuse", "tracing_config": {"api_key": "k"}}, ): - with pytest.raises(BadRequest): + with pytest.raises(TracingConfigVerificationFailedError): method( api, TraceConfigPayload(tracing_provider="langfuse", tracing_config={"api_key": "k"}), - _make_app(), + _make_request_context(), + uuid.UUID(APP_ID), ) def test_trace_app_config_delete_not_found(self, app: Flask, monkeypatch: pytest.MonkeyPatch): api = ops_trace_module.TraceAppConfigApi() method = unwrap(api.delete) - + tracing_configs = MagicMock() + tracing_configs.delete.side_effect = AppTracingConfigNotFoundError() monkeypatch.setattr( - ops_trace_module.OpsService, - "delete_tracing_app_config", - lambda **_kwargs: False, + ops_trace_module, + "application_services", + lambda: SimpleNamespace(app_tracing_configs=tracing_configs), ) with app.test_request_context("/?tracing_provider=langfuse"): - with pytest.raises(BadRequest): - method(api, TraceProviderQuery(tracing_provider="langfuse"), _make_app()) + with pytest.raises(TracingConfigNotFoundError): + method( + api, + TraceProviderQuery(tracing_provider="langfuse"), + _make_request_context(), + uuid.UUID(APP_ID), + ) class TestSiteEndpoints: diff --git a/api/tests/unit_tests/controllers/console/app/test_ops_trace_api.py b/api/tests/unit_tests/controllers/console/app/test_ops_trace_api.py index 1c1ee83e19a..03922de106d 100644 --- a/api/tests/unit_tests/controllers/console/app/test_ops_trace_api.py +++ b/api/tests/unit_tests/controllers/console/app/test_ops_trace_api.py @@ -1,197 +1,527 @@ from __future__ import annotations -from contextlib import nullcontext -from unittest.mock import MagicMock, PropertyMock, patch +import inspect +from collections.abc import Callable +from datetime import datetime +from types import SimpleNamespace +from unittest.mock import MagicMock +from uuid import UUID import pytest from flask import Flask -from sqlalchemy.orm import Session from werkzeug.exceptions import Forbidden -from controllers.common import wraps as common_wraps -from controllers.console import console_ns -from controllers.console import wraps as console_wraps +from controllers.console import flask_admission from controllers.console.app import ops_trace as ops_trace_module -from controllers.console.app import wraps as app_wraps -from enums import DeploymentEdition -from libs import login as login_lib -from models import Tenant +from controllers.console.app.error import ( + AppNotFoundError, + InvalidTracingConfigError, + TracingConfigAlreadyExistsError, + TracingConfigNotFoundError, + TracingConfigProcessingError, + TracingConfigVerificationFailedError, + UnsupportedTracingProviderError, +) +from libs.exception import BaseHTTPException +from libs.login import AccountWithTenant +from machinery.context import RequestContext from models.account import Account, AccountStatus, TenantAccountRole -from models.model import App, AppMode, IconType +from services.app_tracing_config_service import ( + AppTracingConfigAlreadyExistsError, + AppTracingConfigAppNotFoundError, + AppTracingConfigInvalidConfigurationError, + AppTracingConfigInvalidProviderError, + AppTracingConfigNotFoundError, + AppTracingConfigProcessingError, + AppTracingConfigRecord, + AppTracingConfigVerificationFailedError, +) from tests.unit_tests.config_override import apply_config_overrides +APP_ID = "11111111-1111-1111-1111-111111111111" +WORKSPACE_ID = "22222222-2222-2222-2222-222222222222" +ACCOUNT_ID = "33333333-3333-3333-3333-333333333333" +PROVIDER = "langfuse" +_MUTATION_METHODS = ( + ops_trace_module.TraceAppConfigApi.post, + ops_trace_module.TraceAppConfigApi.patch, + ops_trace_module.TraceAppConfigApi.delete, +) +_CONTROLLER_METHODS: dict[str, Callable[..., object]] = { + "get": ops_trace_module.TraceAppConfigApi.get, + "post": ops_trace_module.TraceAppConfigApi.post, + "patch": ops_trace_module.TraceAppConfigApi.patch, + "delete": ops_trace_module.TraceAppConfigApi.delete, +} -def _make_account(role: TenantAccountRole) -> Account: - account = Account(name="tester", email="tester@example.com") - account.id = "account-123" # type: ignore[assignment] - account.status = AccountStatus.ACTIVE + +def _service_method(tracing_configs: MagicMock, method_name: str) -> MagicMock: + return { + "get": tracing_configs.get, + "post": tracing_configs.create, + "patch": tracing_configs.update, + "delete": tracing_configs.delete, + }[method_name] + + +def _account(role: TenantAccountRole) -> Account: + account = Account( + name="Trace User", + email=f"{role.value}@example.com", + status=AccountStatus.ACTIVE, + ) + account.id = ACCOUNT_ID account.role = role - tenant = Tenant(name="Test tenant") - tenant.id = "tenant-123" - account._current_tenant = tenant - account._get_current_object = lambda: account # type: ignore[attr-defined] return account -def _make_app() -> App: - return App( - id="app-123", - tenant_id="tenant-123", - name="Trace app", - description="", - mode=AppMode.CHAT, - icon_type=IconType.EMOJI, - icon="robot", - icon_background="#FFFFFF", - enable_site=True, - enable_api=True, - max_active_requests=None, +def _request_context() -> RequestContext: + return RequestContext( + request_id="request-1", + trace_id="trace-1", + account_id=ACCOUNT_ID, + active_workspace_id=WORKSPACE_ID, ) -def _patch_console_guards( - monkeypatch: pytest.MonkeyPatch, - account: Account, - app_model: App, - *, - rbac_enabled: bool = False, -) -> None: - apply_config_overrides( - monkeypatch, - LOGIN_DISABLED=True, - RBAC_ENABLED=rbac_enabled, - DEPLOYMENT_EDITION=DeploymentEdition.CLOUD, +def _original(method: Callable[..., object]) -> Callable[..., object]: + return inspect.unwrap(method) + + +def _admission_injector(method: Callable[..., object]) -> Callable[..., object]: + return inspect.unwrap( + method, + stop=lambda candidate: "allowed_roles" in inspect.getclosurevars(candidate).nonlocals, ) - monkeypatch.setattr(login_lib, "current_user", account) - monkeypatch.setattr(login_lib, "current_account_with_tenant", lambda: (account, account.current_tenant_id)) - monkeypatch.setattr(console_wraps, "current_account_with_tenant", lambda: (account, account.current_tenant_id)) - monkeypatch.setattr(common_wraps, "current_account_with_tenant", lambda: (account, account.current_tenant_id)) - monkeypatch.setattr(app_wraps, "_load_app_model_from_scoped_session", lambda _app_id: app_model) -def _patch_payload(payload: dict[str, object] | None): - if payload is None: - return nullcontext() - return patch.object(type(console_ns), "payload", new_callable=PropertyMock, return_value=payload) +@pytest.fixture +def tracing_configs(monkeypatch: pytest.MonkeyPatch) -> MagicMock: + service = MagicMock() + monkeypatch.setattr( + ops_trace_module, + "application_services", + lambda: SimpleNamespace(app_tracing_configs=service), + ) + return service + + +@pytest.mark.parametrize("method", _MUTATION_METHODS) +def test_trace_config_mutations_reject_read_only_member_when_rbac_is_disabled( + app: Flask, + monkeypatch: pytest.MonkeyPatch, + method: Callable[..., object], +) -> None: + account = _account(TenantAccountRole.NORMAL) + apply_config_overrides(monkeypatch, RBAC_ENABLED=False) + monkeypatch.setattr( + flask_admission, + "current_account_with_tenant", + lambda: AccountWithTenant(account=account, tenant_id=WORKSPACE_ID), + ) + + with app.test_request_context(), pytest.raises(Forbidden): + _admission_injector(method)(None, app_id=UUID(APP_ID)) + + +@pytest.mark.parametrize("method", _MUTATION_METHODS) +def test_trace_config_mutations_require_app_tracing_permission_when_rbac_is_enabled( + app: Flask, + monkeypatch: pytest.MonkeyPatch, + method: Callable[..., object], +) -> None: + account = _account(TenantAccountRole.NORMAL) + apply_config_overrides(monkeypatch, RBAC_ENABLED=True) + monkeypatch.setattr( + flask_admission, + "current_account_with_tenant", + lambda: AccountWithTenant(account=account, tenant_id=WORKSPACE_ID), + ) + denied = MagicMock(side_effect=Forbidden()) + monkeypatch.setattr(flask_admission, "enforce_rbac_access", denied) + + with app.test_request_context(), pytest.raises(Forbidden): + _admission_injector(method)(None, app_id=UUID(APP_ID)) + + denied.assert_called_once_with( + tenant_id=WORKSPACE_ID, + account_id=ACCOUNT_ID, + resource_type=ops_trace_module.RBACResourceScope.APP, + scene=ops_trace_module.RBACPermission.APP_TRACING_CONFIG, + resource_required=True, + path_args={"app_id": UUID(APP_ID)}, + ) + + +def test_trace_config_get_preserves_read_access_for_normal_member() -> None: + admission = _admission_injector(ops_trace_module.TraceAppConfigApi.get) + + assert inspect.getclosurevars(admission).nonlocals["allowed_roles"] is None + + +@pytest.mark.parametrize("method", _MUTATION_METHODS) +def test_trace_config_mutations_preserve_legacy_edit_roles(method: Callable[..., object]) -> None: + admission = _admission_injector(method) + + assert inspect.getclosurevars(admission).nonlocals["allowed_roles"] == frozenset( + { + TenantAccountRole.OWNER, + TenantAccountRole.ADMIN, + TenantAccountRole.EDITOR, + } + ) + + +def test_trace_app_config_get_empty_returns_exact_legacy_body( + app: Flask, + tracing_configs: MagicMock, +) -> None: + tracing_configs.get.return_value = None + + with app.test_request_context("/?tracing_provider=langfuse"): + result = _original(ops_trace_module.TraceAppConfigApi.get)( + ops_trace_module.TraceAppConfigApi(), + ops_trace_module.TraceProviderQuery(tracing_provider=PROVIDER), + _request_context(), + UUID(APP_ID), + ) + + assert result == {"has_not_configured": True} + tracing_configs.get.assert_called_once_with( + context=_request_context(), + app_id=APP_ID, + tracing_provider=PROVIDER, + ) + + +def test_trace_app_config_get_configured_returns_exact_legacy_body( + app: Flask, + tracing_configs: MagicMock, +) -> None: + tracing_configs.get.return_value = AppTracingConfigRecord( + id="trace-config-1", + app_id=APP_ID, + tracing_provider=PROVIDER, + tracing_config={"public_key": "pk", "secret_key": "******"}, + is_active=True, + created_at=datetime(2026, 1, 2, 3, 4, 5), + updated_at=datetime(2026, 1, 3, 4, 5, 6), + ) + + with app.test_request_context("/?tracing_provider=langfuse"): + result = _original(ops_trace_module.TraceAppConfigApi.get)( + ops_trace_module.TraceAppConfigApi(), + ops_trace_module.TraceProviderQuery(tracing_provider=PROVIDER), + _request_context(), + UUID(APP_ID), + ) + + assert result == { + "id": "trace-config-1", + "app_id": APP_ID, + "tracing_provider": PROVIDER, + "tracing_config": {"public_key": "pk", "secret_key": "******"}, + "is_active": True, + "created_at": "2026-01-02 03:04:05", + "updated_at": "2026-01-03 04:05:06", + } @pytest.mark.parametrize( - ("method_name", "path", "payload", "service_method_name", "service_result"), + ("method_name", "expected_result"), [ - ( - "post", - "/console/api/apps/app-123/trace-config", - {"tracing_provider": "mlflow", "tracing_config": {"endpoint": "https://trace.example.com"}}, - "create_tracing_app_config", - {"id": "trace-config-1"}, - ), - ( - "patch", - "/console/api/apps/app-123/trace-config", - {"tracing_provider": "mlflow", "tracing_config": {"endpoint": "https://trace.example.com"}}, - "update_tracing_app_config", - True, - ), - ( - "delete", - "/console/api/apps/app-123/trace-config?tracing_provider=mlflow", - None, - "delete_tracing_app_config", - True, - ), + pytest.param("post", ({"result": "success"}, 201), id="create"), + pytest.param("patch", {"result": "success"}, id="update"), ], ) -def test_trace_config_mutations_require_edit_permission( +def test_trace_app_config_write_returns_expected_response( app: Flask, - monkeypatch: pytest.MonkeyPatch, + tracing_configs: MagicMock, method_name: str, - path: str, - payload: dict[str, object] | None, - service_method_name: str, - service_result: object, + expected_result: object, ) -> None: - app.config.setdefault("RESTX_MASK_HEADER", "X-Fields") - account = _make_account(TenantAccountRole.NORMAL) - _patch_console_guards(monkeypatch, account, _make_app()) - service_mock = MagicMock(return_value=service_result) - monkeypatch.setattr(ops_trace_module.OpsService, service_method_name, service_mock) + payload = ops_trace_module.TraceConfigPayload( + tracing_provider=PROVIDER, + tracing_config={"public_key": "pk", "secret_key": "sk"}, + ) - with app.test_request_context(path, method=method_name.upper(), json=payload): - with _patch_payload(payload): - with pytest.raises(Forbidden): - getattr(ops_trace_module.TraceAppConfigApi(), method_name)(app_id="app-123") + with app.test_request_context("/", method=method_name.upper()): + result = _original(_CONTROLLER_METHODS[method_name])( + ops_trace_module.TraceAppConfigApi(), + payload, + _request_context(), + UUID(APP_ID), + ) - service_mock.assert_not_called() + assert result == expected_result + _service_method(tracing_configs, method_name).assert_called_once_with( + context=_request_context(), + app_id=APP_ID, + tracing_provider=PROVIDER, + tracing_config={"public_key": "pk", "secret_key": "sk"}, + ) + + +def test_trace_app_config_delete_returns_exact_204_response( + app: Flask, + tracing_configs: MagicMock, +) -> None: + with app.test_request_context("/?tracing_provider=langfuse", method="DELETE"): + result = _original(ops_trace_module.TraceAppConfigApi.delete)( + ops_trace_module.TraceAppConfigApi(), + ops_trace_module.TraceProviderQuery(tracing_provider=PROVIDER), + _request_context(), + UUID(APP_ID), + ) + + assert result == ("", 204) + tracing_configs.delete.assert_called_once_with( + context=_request_context(), + app_id=APP_ID, + tracing_provider=PROVIDER, + ) + + +@pytest.mark.parametrize("method_name", ["get", "post", "patch", "delete"]) +def test_trace_app_config_maps_missing_app_to_404( + app: Flask, + tracing_configs: MagicMock, + method_name: str, +) -> None: + service_method = _service_method(tracing_configs, method_name) + service_method.side_effect = AppTracingConfigAppNotFoundError() + query_or_payload = ( + ops_trace_module.TraceProviderQuery(tracing_provider=PROVIDER) + if method_name in {"get", "delete"} + else ops_trace_module.TraceConfigPayload(tracing_provider=PROVIDER, tracing_config={}) + ) + + with app.test_request_context("/"): + with pytest.raises(AppNotFoundError) as exc_info: + _original(_CONTROLLER_METHODS[method_name])( + ops_trace_module.TraceAppConfigApi(), + query_or_payload, + _request_context(), + UUID(APP_ID), + ) + + assert exc_info.value.code == 404 + assert exc_info.value.error_code == "app_not_found" @pytest.mark.parametrize( - ("method_name", "path", "payload", "service_method_name", "service_result"), + ("method_name", "service_error", "expected_http_error", "expected_status", "expected_code"), [ - ( + pytest.param( "post", - "/console/api/apps/app-123/trace-config", - {"tracing_provider": "mlflow", "tracing_config": {"endpoint": "https://trace.example.com"}}, - "create_tracing_app_config", - {"id": "trace-config-1"}, + AppTracingConfigAlreadyExistsError(), + TracingConfigAlreadyExistsError, + 409, + "trace_config_already_exists", + id="already-exists", ), - ( + pytest.param( "patch", - "/console/api/apps/app-123/trace-config", - {"tracing_provider": "mlflow", "tracing_config": {"endpoint": "https://trace.example.com"}}, - "update_tracing_app_config", - True, + AppTracingConfigNotFoundError(), + TracingConfigNotFoundError, + 404, + "trace_config_not_found", + id="patch-not-found", ), - ( + pytest.param( "delete", - "/console/api/apps/app-123/trace-config?tracing_provider=mlflow", - None, - "delete_tracing_app_config", - True, + AppTracingConfigNotFoundError(), + TracingConfigNotFoundError, + 404, + "trace_config_not_found", + id="delete-not-found", + ), + pytest.param( + "get", + AppTracingConfigInvalidProviderError("unknown"), + UnsupportedTracingProviderError, + 400, + "unsupported_tracing_provider", + id="get-unsupported-provider", + ), + pytest.param( + "post", + AppTracingConfigInvalidProviderError("unknown"), + UnsupportedTracingProviderError, + 400, + "unsupported_tracing_provider", + id="post-unsupported-provider", + ), + pytest.param( + "patch", + AppTracingConfigInvalidProviderError("unknown"), + UnsupportedTracingProviderError, + 400, + "unsupported_tracing_provider", + id="patch-unsupported-provider", + ), + pytest.param( + "delete", + AppTracingConfigInvalidProviderError("unknown"), + UnsupportedTracingProviderError, + 400, + "unsupported_tracing_provider", + id="delete-unsupported-provider", + ), + pytest.param( + "post", + AppTracingConfigInvalidConfigurationError(), + InvalidTracingConfigError, + 400, + "invalid_tracing_config", + id="post-invalid-config", + ), + pytest.param( + "patch", + AppTracingConfigInvalidConfigurationError(), + InvalidTracingConfigError, + 400, + "invalid_tracing_config", + id="patch-invalid-config", + ), + pytest.param( + "post", + AppTracingConfigVerificationFailedError(), + TracingConfigVerificationFailedError, + 400, + "tracing_config_verification_failed", + id="post-verification-failed", + ), + pytest.param( + "patch", + AppTracingConfigVerificationFailedError(), + TracingConfigVerificationFailedError, + 400, + "tracing_config_verification_failed", + id="patch-verification-failed", + ), + pytest.param( + "get", + AppTracingConfigProcessingError(), + TracingConfigProcessingError, + 500, + "tracing_config_processing_failed", + id="get-processing-failed", + ), + pytest.param( + "post", + AppTracingConfigProcessingError(), + TracingConfigProcessingError, + 500, + "tracing_config_processing_failed", + id="post-processing-failed", + ), + pytest.param( + "patch", + AppTracingConfigProcessingError(), + TracingConfigProcessingError, + 500, + "tracing_config_processing_failed", + id="patch-processing-failed", + ), + pytest.param( + "delete", + AppTracingConfigProcessingError(), + TracingConfigProcessingError, + 500, + "tracing_config_processing_failed", + id="delete-processing-failed", ), ], ) -def test_trace_config_mutations_require_rbac_permission( +def test_trace_app_config_maps_application_errors_at_the_controller_boundary( app: Flask, - monkeypatch: pytest.MonkeyPatch, + tracing_configs: MagicMock, method_name: str, - path: str, - payload: dict[str, object] | None, - service_method_name: str, - service_result: object, - sqlite_session: Session, + service_error: Exception, + expected_http_error: type[BaseHTTPException], + expected_status: int, + expected_code: str, ) -> None: - app.config.setdefault("RESTX_MASK_HEADER", "X-Fields") - account = _make_account(TenantAccountRole.NORMAL) - _patch_console_guards(monkeypatch, account, _make_app(), rbac_enabled=True) - owned_app = App() - owned_app.id = "app-123" - owned_app.tenant_id = "tenant-123" - owned_app.name = "Trace app" - owned_app.description = "" - owned_app.mode = AppMode.CHAT - owned_app.icon_type = IconType.EMOJI - owned_app.icon = "robot" - owned_app.icon_background = "#ffffff" - owned_app.enable_site = False - owned_app.enable_api = False - owned_app.api_rpm = 0 - owned_app.api_rph = 0 - owned_app.is_demo = False - owned_app.is_public = False - owned_app.is_universal = False - owned_app.max_active_requests = None - owned_app.maintainer = "other-account" - owned_app.use_icon_as_answer_icon = False - sqlite_session.add(owned_app) - sqlite_session.commit() - monkeypatch.setattr(common_wraps.db, "session", sqlite_session) - monkeypatch.setattr(common_wraps.RBACService.CheckAccess, "check", MagicMock(return_value=False)) - service_mock = MagicMock(return_value=service_result) - monkeypatch.setattr(ops_trace_module.OpsService, service_method_name, service_mock) + service_method = _service_method(tracing_configs, method_name) + service_method.side_effect = service_error + query_or_payload = ( + ops_trace_module.TraceProviderQuery(tracing_provider=PROVIDER) + if method_name in {"get", "delete"} + else ops_trace_module.TraceConfigPayload(tracing_provider=PROVIDER, tracing_config={}) + ) - with app.test_request_context(path, method=method_name.upper(), json=payload): - with _patch_payload(payload): - with pytest.raises(Forbidden): - getattr(ops_trace_module.TraceAppConfigApi(), method_name)(app_id="app-123") + with app.test_request_context("/"): + with pytest.raises(expected_http_error) as exc_info: + _original(_CONTROLLER_METHODS[method_name])( + ops_trace_module.TraceAppConfigApi(), + query_or_payload, + _request_context(), + UUID(APP_ID), + ) - service_mock.assert_not_called() + assert exc_info.value.code == expected_status + assert exc_info.value.error_code == expected_code + assert exc_info.value.data == { + "code": expected_code, + "message": exc_info.value.description, + "status": expected_status, + } + + +@pytest.mark.parametrize("method_name", ["get", "post", "patch", "delete"]) +def test_trace_app_config_maps_untyped_value_errors_to_internal_error( + app: Flask, + tracing_configs: MagicMock, + method_name: str, +) -> None: + service_method = _service_method(tracing_configs, method_name) + service_method.side_effect = ValueError("internal detail") + query_or_payload = ( + ops_trace_module.TraceProviderQuery(tracing_provider=PROVIDER) + if method_name in {"get", "delete"} + else ops_trace_module.TraceConfigPayload(tracing_provider=PROVIDER, tracing_config={}) + ) + + with app.test_request_context("/"), pytest.raises(TracingConfigProcessingError) as exc_info: + _original(_CONTROLLER_METHODS[method_name])( + ops_trace_module.TraceAppConfigApi(), + query_or_payload, + _request_context(), + UUID(APP_ID), + ) + + assert exc_info.value.code == 500 + assert exc_info.value.error_code == "tracing_config_processing_failed" + assert exc_info.value.description == "The tracing configuration could not be processed." + assert exc_info.value.data == { + "code": "tracing_config_processing_failed", + "message": "The tracing configuration could not be processed.", + "status": 500, + } + + +@pytest.mark.parametrize("method_name", ["get", "post", "patch", "delete"]) +def test_trace_app_config_does_not_mask_unexpected_errors( + app: Flask, + tracing_configs: MagicMock, + method_name: str, +) -> None: + service_method = _service_method(tracing_configs, method_name) + unexpected_error = RuntimeError("unexpected") + service_method.side_effect = unexpected_error + query_or_payload = ( + ops_trace_module.TraceProviderQuery(tracing_provider=PROVIDER) + if method_name in {"get", "delete"} + else ops_trace_module.TraceConfigPayload(tracing_provider=PROVIDER, tracing_config={}) + ) + + with app.test_request_context("/"), pytest.raises(RuntimeError) as exc_info: + _original(_CONTROLLER_METHODS[method_name])( + ops_trace_module.TraceAppConfigApi(), + query_or_payload, + _request_context(), + UUID(APP_ID), + ) + + assert exc_info.value is unexpected_error diff --git a/api/tests/unit_tests/extensions/test_ext_application_services.py b/api/tests/unit_tests/extensions/test_ext_application_services.py index 1601d420982..2be0f0c358e 100644 --- a/api/tests/unit_tests/extensions/test_ext_application_services.py +++ b/api/tests/unit_tests/extensions/test_ext_application_services.py @@ -30,6 +30,7 @@ from repositories.account_oauth_repository import ( from repositories.account_repository import SQLAlchemyAccountRepository from repositories.app_site_command_repository import AppSiteCommandRepository from repositories.app_statistic_query_repository import AppStatisticQueryRepository +from repositories.app_tracing_config_repository import SQLAlchemyAppTracingConfigRepository from repositories.sqlalchemy_api_workflow_run_repository import DifyAPISQLAlchemyWorkflowRunRepository from repositories.workflow_app_log_query_repository import WorkflowAppLogQueryRepository from repositories.workflow_run_archive_repository import WorkflowRunArchiveBundleQueryRepository @@ -58,6 +59,8 @@ from services.account_oauth_adapters import ( RedisOAuthAccountClaimLock, ) from services.app_site_service import AppSiteService +from services.app_tracing_config_gateway import OpsTraceManagerGateway +from services.app_tracing_config_service import AppTracingConfigService from services.auth.data_source_api_key_auth_service import DataSourceApiKeyAuthService from services.billing_portal_service import BillingPortalService from services.billing_service import BillingService @@ -266,6 +269,22 @@ def test_build_application_services_wires_app_site_boundary( assert services.app_sites._sites._session_factory is sqlite_session_factory +def test_build_application_services_wires_app_tracing_config_boundary( + sqlite_session_factory: sessionmaker[Session], +) -> None: + services = ext_application_services.build_application_services( + database_client=sqlite_session_factory, + deployment_edition=DeploymentEdition.COMMUNITY, + initialization_password="", + redis=MagicMock(spec=RedisClientWrapper), + ) + + assert isinstance(services.app_tracing_configs, AppTracingConfigService) + assert isinstance(services.app_tracing_configs._configs, SQLAlchemyAppTracingConfigRepository) + assert services.app_tracing_configs._configs._session_factory is sqlite_session_factory + assert isinstance(services.app_tracing_configs._provider, OpsTraceManagerGateway) + + def test_build_application_services_wires_workflow_app_log_boundary( sqlite_session_factory: sessionmaker[Session], ) -> None: diff --git a/api/tests/unit_tests/repositories/test_app_tracing_config_repository.py b/api/tests/unit_tests/repositories/test_app_tracing_config_repository.py new file mode 100644 index 00000000000..f58efbbbfc2 --- /dev/null +++ b/api/tests/unit_tests/repositories/test_app_tracing_config_repository.py @@ -0,0 +1,145 @@ +from collections.abc import Callable + +import pytest +from sqlalchemy import select, text +from sqlalchemy.orm import Session, sessionmaker + +from models.model import App, AppMode, TraceAppConfig +from repositories.app_tracing_config_repository import SQLAlchemyAppTracingConfigRepository +from services.app_tracing_config_service import AppTracingConfigAppNotFoundError, AppTracingConfigRecord + +_APP_ID = "11111111-1111-1111-1111-111111111111" +_WORKSPACE_ID = "22222222-2222-2222-2222-222222222222" +_OTHER_WORKSPACE_ID = "33333333-3333-3333-3333-333333333333" +_PROVIDER = "langfuse" + + +def _persist_app(session: Session) -> None: + session.add( + App( + id=_APP_ID, + tenant_id=_WORKSPACE_ID, + name="Tracing App", + description="", + mode=AppMode.CHAT, + icon_type=None, + icon=None, + icon_background=None, + enable_site=True, + enable_api=True, + ) + ) + session.commit() + + +def _repository(session_factory: sessionmaker[Session]) -> SQLAlchemyAppTracingConfigRepository: + return SQLAlchemyAppTracingConfigRepository(session_factory=session_factory) + + +def test_config_lifecycle_is_persisted_by_owned_transactions( + sqlite_session: Session, + sqlite_session_factory: sessionmaker[Session], +) -> None: + _persist_app(sqlite_session) + repository = _repository(sqlite_session_factory) + + assert repository.get(workspace_id=_WORKSPACE_ID, app_id=_APP_ID, tracing_provider=_PROVIDER) is None + assert repository.create( + workspace_id=_WORKSPACE_ID, + app_id=_APP_ID, + tracing_provider=_PROVIDER, + tracing_config={"public_key": "original"}, + ) + assert not repository.create( + workspace_id=_WORKSPACE_ID, + app_id=_APP_ID, + tracing_provider=_PROVIDER, + tracing_config={"public_key": "duplicate"}, + ) + + record = repository.get(workspace_id=_WORKSPACE_ID, app_id=_APP_ID, tracing_provider=_PROVIDER) + assert isinstance(record, AppTracingConfigRecord) + assert record.app_id == _APP_ID + assert record.tracing_provider == _PROVIDER + assert record.tracing_config == {"public_key": "original"} + + assert repository.update( + workspace_id=_WORKSPACE_ID, + app_id=_APP_ID, + tracing_provider=_PROVIDER, + tracing_config={"public_key": "updated"}, + ) + with sqlite_session_factory() as session: + config = session.scalar(select(TraceAppConfig).where(TraceAppConfig.app_id == _APP_ID)) + assert config is not None + assert config.tracing_config == {"public_key": "updated"} + + assert repository.delete(workspace_id=_WORKSPACE_ID, app_id=_APP_ID, tracing_provider=_PROVIDER) + assert not repository.update( + workspace_id=_WORKSPACE_ID, + app_id=_APP_ID, + tracing_provider=_PROVIDER, + tracing_config={"public_key": "missing"}, + ) + assert not repository.delete(workspace_id=_WORKSPACE_ID, app_id=_APP_ID, tracing_provider=_PROVIDER) + with sqlite_session_factory() as session: + assert session.scalar(select(TraceAppConfig).where(TraceAppConfig.app_id == _APP_ID)) is None + + +@pytest.mark.parametrize("app_state", ["other-workspace", "non-normal"]) +def test_all_operations_reject_apps_outside_the_active_workspace_scope( + app_state: str, + sqlite_session: Session, + sqlite_session_factory: sessionmaker[Session], +) -> None: + _persist_app(sqlite_session) + workspace_id = _OTHER_WORKSPACE_ID + if app_state == "non-normal": + sqlite_session.execute(text("UPDATE apps SET status = 'disabled' WHERE id = :app_id"), {"app_id": _APP_ID}) + sqlite_session.commit() + workspace_id = _WORKSPACE_ID + + repository = _repository(sqlite_session_factory) + operations: tuple[Callable[[], object], ...] = ( + lambda: repository.get(workspace_id=workspace_id, app_id=_APP_ID, tracing_provider=_PROVIDER), + lambda: repository.create( + workspace_id=workspace_id, + app_id=_APP_ID, + tracing_provider=_PROVIDER, + tracing_config={}, + ), + lambda: repository.update( + workspace_id=workspace_id, + app_id=_APP_ID, + tracing_provider=_PROVIDER, + tracing_config={}, + ), + lambda: repository.delete(workspace_id=workspace_id, app_id=_APP_ID, tracing_provider=_PROVIDER), + ) + + for operation in operations: + with pytest.raises(AppTracingConfigAppNotFoundError): + operation() + + +def test_record_mapping_does_not_expose_the_orm_model_or_its_config_dict( + sqlite_session: Session, +) -> None: + _persist_app(sqlite_session) + config = TraceAppConfig( + app_id=_APP_ID, + tracing_provider=_PROVIDER, + tracing_config={"public_key": "original"}, + ) + sqlite_session.add(config) + sqlite_session.commit() + + record = SQLAlchemyAppTracingConfigRepository._to_record(config) + + assert isinstance(record, AppTracingConfigRecord) + assert record is not config + assert record.tracing_config == config.tracing_config + assert record.tracing_config is not config.tracing_config + assert record.tracing_config is not None + record.tracing_config["public_key"] = "changed" + assert config.tracing_config == {"public_key": "original"} diff --git a/api/tests/unit_tests/services/test_app_tracing_config_gateway.py b/api/tests/unit_tests/services/test_app_tracing_config_gateway.py new file mode 100644 index 00000000000..7a5d7f9e75e --- /dev/null +++ b/api/tests/unit_tests/services/test_app_tracing_config_gateway.py @@ -0,0 +1,360 @@ +from unittest.mock import patch + +import pytest +from pydantic import ValidationInfo, field_validator + +from core.ops.entities.config_entity import BaseTracingConfig +from core.ops.ops_trace_manager import TracingProviderConfigEntry +from services import app_tracing_config_gateway as gateway_module +from services.app_tracing_config_gateway import OpsTraceManagerGateway +from services.app_tracing_config_service import ( + AppTracingConfigInvalidConfigurationError, + AppTracingConfigInvalidProviderError, + AppTracingConfigProcessingError, + AppTracingConfigVerificationFailedError, +) + + +class _ProviderConfig(BaseTracingConfig): + endpoint: str = "https://default.example.com" + project: str = "default-project" + + @field_validator("endpoint", "project", mode="before") + @classmethod + def replace_empty_with_default(cls, value: object, info: ValidationInfo) -> object: + if value != "": + return value + if info.field_name == "endpoint": + return "https://default.example.com" + return "default-project" + + +def _provider_entry(*, other_keys: list[str] | None = None) -> TracingProviderConfigEntry: + return { + "config_class": _ProviderConfig, + "secret_keys": ["api_key"], + "other_keys": other_keys or [], + "trace_instance": object, + } + + +def test_validate_provider_rejects_unknown_provider(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setattr(gateway_module, "provider_config_map", {}) + + with pytest.raises(AppTracingConfigInvalidProviderError, match="Invalid tracing provider: unknown"): + OpsTraceManagerGateway().validate_provider("unknown") + + +def test_prepare_new_config_applies_defaults_validates_and_encrypts(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setattr( + gateway_module, + "provider_config_map", + {"arize": _provider_entry(other_keys=["endpoint", "project"])}, + ) + submitted = {"api_key": "plain", "endpoint": "", "project": ""} + + with patch.object(gateway_module, "OpsTraceManager") as manager: + manager.check_trace_config_is_effective.return_value = True + manager.get_trace_config_project_url.return_value = "https://project.example.com" + manager.encrypt_tracing_config.return_value = {"api_key": "encrypted"} + + result = OpsTraceManagerGateway().prepare_new_config( + workspace_id="workspace-1", + tracing_provider="arize", + tracing_config=submitted, + ) + + normalized = { + "api_key": "plain", + "endpoint": "https://default.example.com", + "project": "default-project", + } + assert submitted == {"api_key": "plain", "endpoint": "", "project": ""} + assert result == {"api_key": "encrypted", "project_url": "https://project.example.com"} + manager.check_trace_config_is_effective.assert_called_once_with(normalized, "arize") + manager.encrypt_tracing_config.assert_called_once_with("workspace-1", "arize", normalized) + + +def test_prepare_new_config_reports_failed_verification_before_encryption( + monkeypatch: pytest.MonkeyPatch, +) -> None: + monkeypatch.setattr(gateway_module, "provider_config_map", {"arize": _provider_entry()}) + + with patch.object(gateway_module, "OpsTraceManager") as manager: + manager.check_trace_config_is_effective.return_value = False + + with pytest.raises(AppTracingConfigVerificationFailedError): + OpsTraceManagerGateway().prepare_new_config( + workspace_id="workspace-1", + tracing_provider="arize", + tracing_config={"api_key": "plain"}, + ) + + manager.encrypt_tracing_config.assert_not_called() + + +def test_prepare_new_config_keeps_success_when_project_url_lookup_fails( + monkeypatch: pytest.MonkeyPatch, +) -> None: + monkeypatch.setattr(gateway_module, "provider_config_map", {"arize": _provider_entry()}) + + with patch.object(gateway_module, "OpsTraceManager") as manager: + manager.check_trace_config_is_effective.return_value = True + manager.get_trace_config_project_url.side_effect = RuntimeError("provider unavailable") + manager.encrypt_tracing_config.return_value = {"api_key": "encrypted"} + + result = OpsTraceManagerGateway().prepare_new_config( + workspace_id="workspace-1", + tracing_provider="arize", + tracing_config={"api_key": "plain"}, + ) + + assert result == {"api_key": "encrypted"} + + +def test_prepare_new_langfuse_config_builds_project_url(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setattr(gateway_module, "provider_config_map", {"langfuse": _provider_entry()}) + + with patch.object(gateway_module, "OpsTraceManager") as manager: + manager.check_trace_config_is_effective.return_value = True + manager.get_trace_config_project_key.return_value = "project-key" + manager.encrypt_tracing_config.return_value = {"secret_key": "encrypted"} + + result = OpsTraceManagerGateway().prepare_new_config( + workspace_id="workspace-1", + tracing_provider="langfuse", + tracing_config={"host": "https://langfuse.example.com"}, + ) + + assert result["project_url"] == "https://langfuse.example.com/project/project-key" + + +def test_prepare_new_config_reports_provider_check_exception_as_failed_verification( + monkeypatch: pytest.MonkeyPatch, +) -> None: + monkeypatch.setattr(gateway_module, "provider_config_map", {"arize": _provider_entry()}) + + with patch.object(gateway_module, "OpsTraceManager") as manager: + manager.check_trace_config_is_effective.side_effect = ValueError("verification failed") + + with pytest.raises(AppTracingConfigVerificationFailedError) as caught: + OpsTraceManagerGateway().prepare_new_config( + workspace_id="workspace-1", + tracing_provider="arize", + tracing_config={}, + ) + + assert isinstance(caught.value.__cause__, ValueError) + + +def test_prepare_new_config_propagates_unexpected_verification_failure( + monkeypatch: pytest.MonkeyPatch, +) -> None: + monkeypatch.setattr(gateway_module, "provider_config_map", {"arize": _provider_entry()}) + failure = RuntimeError("unexpected provider bug") + + with patch.object(gateway_module, "OpsTraceManager") as manager: + manager.check_trace_config_is_effective.side_effect = failure + + with pytest.raises(RuntimeError) as caught: + OpsTraceManagerGateway().prepare_new_config( + workspace_id="workspace-1", + tracing_provider="arize", + tracing_config={}, + ) + + assert caught.value is failure + manager.encrypt_tracing_config.assert_not_called() + + +def test_prepare_new_config_rejects_invalid_schema_before_verification( + monkeypatch: pytest.MonkeyPatch, +) -> None: + monkeypatch.setattr(gateway_module, "provider_config_map", {"arize": _provider_entry()}) + + with patch.object(gateway_module, "OpsTraceManager") as manager: + with pytest.raises(AppTracingConfigInvalidConfigurationError): + OpsTraceManagerGateway().prepare_new_config( + workspace_id="workspace-1", + tracing_provider="arize", + tracing_config={"endpoint": {"invalid": "value"}}, + ) + + manager.check_trace_config_is_effective.assert_not_called() + manager.encrypt_tracing_config.assert_not_called() + + +def test_prepare_new_config_reports_encryption_failure_as_processing_error(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setattr(gateway_module, "provider_config_map", {"arize": _provider_entry()}) + failure = RuntimeError("key provider unavailable") + + with patch.object(gateway_module, "OpsTraceManager") as manager: + manager.check_trace_config_is_effective.return_value = True + manager.get_trace_config_project_url.return_value = None + manager.encrypt_tracing_config.side_effect = failure + + with pytest.raises(AppTracingConfigProcessingError) as caught: + OpsTraceManagerGateway().prepare_new_config( + workspace_id="workspace-1", + tracing_provider="arize", + tracing_config={}, + ) + + assert caught.value.__cause__ is failure + + +def test_prepare_updated_config_preserves_masked_secret_from_current_config( + monkeypatch: pytest.MonkeyPatch, +) -> None: + monkeypatch.setattr(gateway_module, "provider_config_map", {"arize": _provider_entry()}) + submitted = {"api_key": "******", "project": "new-project"} + current = {"api_key": "old-encrypted", "project": "old-project"} + encrypted = {"api_key": "old-encrypted", "project": "new-project"} + decrypted = {"api_key": "old-plain", "project": "new-project"} + + with patch.object(gateway_module, "OpsTraceManager") as manager: + manager.encrypt_tracing_config.return_value = encrypted + manager.decrypt_tracing_config.return_value = decrypted + manager.check_trace_config_is_effective.return_value = True + + result = OpsTraceManagerGateway().prepare_updated_config( + workspace_id="workspace-1", + tracing_provider="arize", + tracing_config=submitted, + current_tracing_config=current, + ) + + assert result == encrypted + assert submitted == {"api_key": "******", "project": "new-project"} + manager.encrypt_tracing_config.assert_called_once_with("workspace-1", "arize", submitted, current) + manager.decrypt_tracing_config.assert_called_once_with("workspace-1", "arize", encrypted) + manager.check_trace_config_is_effective.assert_called_once_with(decrypted, "arize") + + +def test_prepare_updated_config_validates_schema_before_encryption(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setattr(gateway_module, "provider_config_map", {"arize": _provider_entry()}) + + with patch.object(gateway_module, "OpsTraceManager") as manager: + with pytest.raises(AppTracingConfigInvalidConfigurationError): + OpsTraceManagerGateway().prepare_updated_config( + workspace_id="workspace-1", + tracing_provider="arize", + tracing_config={"endpoint": {"invalid": "value"}}, + current_tracing_config={"api_key": "old-encrypted"}, + ) + + manager.encrypt_tracing_config.assert_not_called() + manager.decrypt_tracing_config.assert_not_called() + manager.check_trace_config_is_effective.assert_not_called() + + +def test_prepare_updated_config_reports_decryption_failure_as_processing_error( + monkeypatch: pytest.MonkeyPatch, +) -> None: + monkeypatch.setattr(gateway_module, "provider_config_map", {"arize": _provider_entry()}) + failure = RuntimeError("stored credential cannot be decrypted") + + with patch.object(gateway_module, "OpsTraceManager") as manager: + manager.encrypt_tracing_config.return_value = {"api_key": "encrypted"} + manager.decrypt_tracing_config.side_effect = failure + + with pytest.raises(AppTracingConfigProcessingError) as caught: + OpsTraceManagerGateway().prepare_updated_config( + workspace_id="workspace-1", + tracing_provider="arize", + tracing_config={"project": "new-project"}, + current_tracing_config={"api_key": "old-encrypted"}, + ) + + assert caught.value.__cause__ is failure + manager.check_trace_config_is_effective.assert_not_called() + + +@pytest.mark.parametrize( + ("provider", "fallback_url"), + [ + ("arize", "https://app.arize.com/"), + ("phoenix", "https://app.phoenix.arize.com/projects/"), + ("langsmith", "https://smith.langchain.com/"), + ("opik", "https://www.comet.com/opik/"), + ("weave", "https://wandb.ai/"), + ("aliyun", "https://arms.console.aliyun.com/"), + ("tencent", "https://console.cloud.tencent.com/apm"), + ("mlflow", "http://localhost:5000/"), + ("databricks", "https://www.databricks.com/"), + ], +) +def test_present_config_uses_provider_fallback_when_project_lookup_fails( + provider: str, + fallback_url: str, +) -> None: + with patch.object(gateway_module, "OpsTraceManager") as manager: + decrypted_config: dict[str, object] = {} + presented_config: dict[str, object] = {} + manager.decrypt_tracing_config.return_value = decrypted_config + manager.obfuscated_decrypt_token.return_value = presented_config + manager.get_trace_config_project_url.side_effect = RuntimeError("provider unavailable") + + result = OpsTraceManagerGateway().present_config( + workspace_id="workspace-1", + tracing_provider=provider, + tracing_config={"encrypted": "config"}, + ) + + assert result == {"project_url": fallback_url} + + +def test_present_langfuse_config_builds_project_url() -> None: + with patch.object(gateway_module, "OpsTraceManager") as manager: + manager.decrypt_tracing_config.return_value = {"host": "https://langfuse.example.com"} + manager.obfuscated_decrypt_token.return_value = {"host": "https://langfuse.example.com"} + manager.get_trace_config_project_key.return_value = "project-key" + + result = OpsTraceManagerGateway().present_config( + workspace_id="workspace-1", + tracing_provider="langfuse", + tracing_config={"encrypted": "config"}, + ) + + assert result["project_url"] == "https://langfuse.example.com/project/project-key" + + +def test_present_langfuse_config_falls_back_to_host() -> None: + with patch.object(gateway_module, "OpsTraceManager") as manager: + manager.decrypt_tracing_config.return_value = {"host": "https://langfuse.example.com"} + manager.obfuscated_decrypt_token.return_value = {"host": "https://langfuse.example.com"} + manager.get_trace_config_project_key.side_effect = RuntimeError("provider unavailable") + + result = OpsTraceManagerGateway().present_config( + workspace_id="workspace-1", + tracing_provider="langfuse", + tracing_config={"encrypted": "config"}, + ) + + assert result["project_url"] == "https://langfuse.example.com/" + + +def test_present_config_rejects_missing_stored_config() -> None: + with pytest.raises(AppTracingConfigProcessingError, match="processing failed"): + OpsTraceManagerGateway().present_config( + workspace_id="workspace-1", + tracing_provider="arize", + tracing_config=None, + ) + + +def test_present_config_reports_decryption_failure_as_processing_error() -> None: + failure = RuntimeError("stored credential cannot be decrypted") + + with patch.object(gateway_module, "OpsTraceManager") as manager: + manager.decrypt_tracing_config.side_effect = failure + + with pytest.raises(AppTracingConfigProcessingError) as caught: + OpsTraceManagerGateway().present_config( + workspace_id="workspace-1", + tracing_provider="arize", + tracing_config={"encrypted": "config"}, + ) + + assert caught.value.__cause__ is failure + manager.obfuscated_decrypt_token.assert_not_called() diff --git a/api/tests/unit_tests/services/test_app_tracing_config_service.py b/api/tests/unit_tests/services/test_app_tracing_config_service.py new file mode 100644 index 00000000000..208b5d4a972 --- /dev/null +++ b/api/tests/unit_tests/services/test_app_tracing_config_service.py @@ -0,0 +1,229 @@ +from datetime import UTC, datetime +from unittest.mock import MagicMock + +import pytest + +from machinery.context import RequestContext +from services.app_tracing_config_service import ( + AppTracingConfigAlreadyExistsError, + AppTracingConfigNotFoundError, + AppTracingConfigRecord, + AppTracingConfigService, +) + + +def _context() -> RequestContext: + return RequestContext("request-1", None, "account-1", "workspace-1") + + +def _record(*, tracing_config: dict[str, object] | None = None) -> AppTracingConfigRecord: + timestamp = datetime(2026, 1, 1, tzinfo=UTC) + return AppTracingConfigRecord( + id="config-1", + app_id="app-1", + tracing_provider="arize", + tracing_config=tracing_config, + is_active=True, + created_at=timestamp, + updated_at=timestamp, + ) + + +def test_get_scopes_lookup_to_workspace_and_presents_config() -> None: + stored = _record(tracing_config={"api_key": "encrypted"}) + configs = MagicMock() + configs.get.return_value = stored + provider = MagicMock() + provider.present_config.return_value = {"api_key": "******", "project_url": "https://example.com"} + service = AppTracingConfigService(configs=configs, provider=provider) + + result = service.get(_context(), "app-1", "arize") + + assert result == _record(tracing_config={"api_key": "******", "project_url": "https://example.com"}) + assert stored.tracing_config == {"api_key": "encrypted"} + provider.validate_provider.assert_called_once_with("arize") + configs.get.assert_called_once_with( + workspace_id="workspace-1", + app_id="app-1", + tracing_provider="arize", + ) + provider.present_config.assert_called_once_with( + workspace_id="workspace-1", + tracing_provider="arize", + tracing_config={"api_key": "encrypted"}, + ) + + +def test_get_returns_none_without_calling_provider_for_missing_config() -> None: + configs = MagicMock() + configs.get.return_value = None + provider = MagicMock() + service = AppTracingConfigService(configs=configs, provider=provider) + + assert service.get(_context(), "app-1", "arize") is None + + provider.validate_provider.assert_called_once_with("arize") + provider.present_config.assert_not_called() + + +def test_get_validates_provider_before_store_lookup() -> None: + events: list[str] = [] + configs = MagicMock() + configs.get.side_effect = lambda **_: events.append("store.get") + provider = MagicMock() + provider.validate_provider.side_effect = lambda *_: events.append("provider.validate") + service = AppTracingConfigService(configs=configs, provider=provider) + + assert service.get(_context(), "app-1", "arize") is None + + assert events == ["provider.validate", "store.get"] + + +def test_create_calls_provider_between_separate_store_operations() -> None: + events: list[str] = [] + configs = MagicMock() + + def get_config(**_: object) -> None: + events.append("store.get:closed") + + def create_config(**_: object) -> bool: + events.append("store.create:closed") + return True + + configs.get.side_effect = get_config + configs.create.side_effect = create_config + provider = MagicMock() + + def prepare_config(**_: object) -> dict[str, str]: + events.append("provider.prepare") + return {"api_key": "encrypted"} + + provider.prepare_new_config.side_effect = prepare_config + service = AppTracingConfigService(configs=configs, provider=provider) + + service.create(_context(), "app-1", "arize", {"api_key": "plain"}) + + assert events == ["store.get:closed", "provider.prepare", "store.create:closed"] + provider.prepare_new_config.assert_called_once_with( + workspace_id="workspace-1", + tracing_provider="arize", + tracing_config={"api_key": "plain"}, + ) + configs.create.assert_called_once_with( + workspace_id="workspace-1", + app_id="app-1", + tracing_provider="arize", + tracing_config={"api_key": "encrypted"}, + ) + + +def test_create_reports_duplicate_before_provider_preparation() -> None: + configs = MagicMock() + configs.get.return_value = _record(tracing_config={"api_key": "encrypted"}) + provider = MagicMock() + provider.prepare_new_config.return_value = {"api_key": "new-encrypted"} + service = AppTracingConfigService(configs=configs, provider=provider) + + with pytest.raises(AppTracingConfigAlreadyExistsError, match="Trace config is exist"): + service.create(_context(), "app-1", "arize", {"api_key": "plain"}) + + provider.prepare_new_config.assert_not_called() + configs.create.assert_not_called() + + +def test_create_reports_duplicate_detected_during_write() -> None: + configs = MagicMock() + configs.get.return_value = None + configs.create.return_value = False + provider = MagicMock() + provider.prepare_new_config.return_value = {"api_key": "encrypted"} + service = AppTracingConfigService(configs=configs, provider=provider) + + with pytest.raises(AppTracingConfigAlreadyExistsError): + service.create(_context(), "app-1", "arize", {"api_key": "plain"}) + + +def test_update_validates_reads_prepares_and_writes_in_order() -> None: + events: list[str] = [] + current = _record(tracing_config={"api_key": "old-encrypted"}) + configs = MagicMock() + configs.get.side_effect = lambda **_: events.append("store.get:closed") or current + configs.update.side_effect = lambda **_: events.append("store.update:closed") or True + provider = MagicMock() + provider.validate_provider.side_effect = lambda *_: events.append("provider.validate") + provider.prepare_updated_config.side_effect = lambda **_: ( + events.append("provider.prepare") or {"api_key": "new-encrypted"} + ) + service = AppTracingConfigService(configs=configs, provider=provider) + + service.update(_context(), "app-1", "arize", {"api_key": "******"}) + + assert events == ["store.get:closed", "provider.validate", "provider.prepare", "store.update:closed"] + provider.prepare_updated_config.assert_called_once_with( + workspace_id="workspace-1", + tracing_provider="arize", + tracing_config={"api_key": "******"}, + current_tracing_config={"api_key": "old-encrypted"}, + ) + configs.update.assert_called_once_with( + workspace_id="workspace-1", + app_id="app-1", + tracing_provider="arize", + tracing_config={"api_key": "new-encrypted"}, + ) + + +def test_update_reports_config_missing_before_provider_preparation() -> None: + configs = MagicMock() + configs.get.return_value = None + provider = MagicMock() + service = AppTracingConfigService(configs=configs, provider=provider) + + with pytest.raises(AppTracingConfigNotFoundError, match="Trace config not exist"): + service.update(_context(), "app-1", "arize", {}) + + provider.validate_provider.assert_called_once_with("arize") + provider.prepare_updated_config.assert_not_called() + configs.update.assert_not_called() + + +def test_update_reports_config_removed_before_write() -> None: + configs = MagicMock() + configs.get.return_value = _record(tracing_config={"api_key": "old-encrypted"}) + configs.update.return_value = False + provider = MagicMock() + provider.prepare_updated_config.return_value = {"api_key": "new-encrypted"} + service = AppTracingConfigService(configs=configs, provider=provider) + + with pytest.raises(AppTracingConfigNotFoundError): + service.update(_context(), "app-1", "arize", {"api_key": "******"}) + + +def test_delete_scopes_to_workspace_and_reports_missing_config() -> None: + configs = MagicMock() + configs.delete.return_value = False + provider = MagicMock() + service = AppTracingConfigService(configs=configs, provider=provider) + + with pytest.raises(AppTracingConfigNotFoundError): + service.delete(_context(), "app-1", "arize") + + configs.delete.assert_called_once_with( + workspace_id="workspace-1", + app_id="app-1", + tracing_provider="arize", + ) + provider.validate_provider.assert_called_once_with("arize") + + +def test_delete_validates_provider_before_store_delete() -> None: + events: list[str] = [] + configs = MagicMock() + configs.delete.side_effect = lambda **_: events.append("store.delete") or True + provider = MagicMock() + provider.validate_provider.side_effect = lambda *_: events.append("provider.validate") + service = AppTracingConfigService(configs=configs, provider=provider) + + service.delete(_context(), "app-1", "arize") + + assert events == ["provider.validate", "store.delete"] diff --git a/packages/contracts/generated/api/console/apps/types.gen.ts b/packages/contracts/generated/api/console/apps/types.gen.ts index 3539ca8d6ea..1098bb2809e 100644 --- a/packages/contracts/generated/api/console/apps/types.gen.ts +++ b/packages/contracts/generated/api/console/apps/types.gen.ts @@ -5003,6 +5003,8 @@ export type DeleteAppsByAppIdTraceConfigData = { export type DeleteAppsByAppIdTraceConfigErrors = { 400: unknown 403: unknown + 404: unknown + 500: unknown } export type DeleteAppsByAppIdTraceConfigResponses = { @@ -5025,6 +5027,8 @@ export type GetAppsByAppIdTraceConfigData = { export type GetAppsByAppIdTraceConfigErrors = { 400: unknown + 404: unknown + 500: unknown } export type GetAppsByAppIdTraceConfigResponses = { @@ -5046,6 +5050,8 @@ export type PatchAppsByAppIdTraceConfigData = { export type PatchAppsByAppIdTraceConfigErrors = { 400: unknown 403: unknown + 404: unknown + 500: unknown } export type PatchAppsByAppIdTraceConfigResponses = { @@ -5067,6 +5073,9 @@ export type PostAppsByAppIdTraceConfigData = { export type PostAppsByAppIdTraceConfigErrors = { 400: unknown 403: unknown + 404: unknown + 409: unknown + 500: unknown } export type PostAppsByAppIdTraceConfigResponses = {