From 8ca9f693d54fb93072543f55babfb799e40aa525 Mon Sep 17 00:00:00 2001 From: Ian Woodard <17186604+IanWoodard@users.noreply.github.com> Date: Tue, 27 May 2025 10:13:02 -0500 Subject: [PATCH 1/3] ref(up): Adding more context to up --- devservices/commands/up.py | 150 ++++++++++++++++++++++++------------- 1 file changed, 98 insertions(+), 52 deletions(-) diff --git a/devservices/commands/up.py b/devservices/commands/up.py index 19655917..fe709263 100644 --- a/devservices/commands/up.py +++ b/devservices/commands/up.py @@ -8,6 +8,8 @@ from argparse import Namespace from sentry_sdk import capture_exception +from sentry_sdk import set_context +from sentry_sdk import start_span from devservices.constants import CONFIG_FILE_NAME from devservices.constants import DEPENDENCY_CONFIG_VERSION @@ -92,47 +94,58 @@ def up(args: Namespace, existing_status: Status | None = None) -> None: state = State() with Status( - lambda: console.warning(f"Starting '{service.name}' in mode: '{mode}'") - if existing_status is None - else existing_status.warning(f"Starting '{service.name}' in mode: '{mode}'"), - lambda: console.success(f"{service.name} started") - if existing_status is None - else existing_status.success(f"{service.name} started"), + lambda: ( + console.warning(f"Starting '{service.name}' in mode: '{mode}'") + if existing_status is None + else existing_status.warning(f"Starting '{service.name}' in mode: '{mode}'") + ), + lambda: ( + console.success(f"{service.name} started") + if existing_status is None + else existing_status.success(f"{service.name} started") + ), ) as status: - services_with_local_runtime = state.get_services_by_runtime( - ServiceRuntime.LOCAL - ) local_runtime_dependency_names = set() - for service_with_local_runtime in services_with_local_runtime: - if ( - mode in modes - and service_with_local_runtime != service.name - and service_with_local_runtime in modes[mode] - ): - local_runtime_dependency_names.add(service_with_local_runtime) - if exclude_local: - status.warning( - f"Skipping '{service_with_local_runtime}' as it is set to run locally" - ) - try: - status.info("Retrieving dependencies") - remote_dependencies = install_and_verify_dependencies( - service, force_update_dependencies=True, modes=[mode] + with start_span( + op="service.dependencies", name="Check local runtime dependencies" + ) as span: + services_with_local_runtime = state.get_services_by_runtime( + ServiceRuntime.LOCAL ) - except DependencyError as de: - capture_exception(de) - status.failure( - f"{str(de)}. If this error persists, try running `devservices purge`" + for service_with_local_runtime in services_with_local_runtime: + if ( + mode in modes + and service_with_local_runtime != service.name + and service_with_local_runtime in modes[mode] + ): + local_runtime_dependency_names.add(service_with_local_runtime) + if exclude_local: + status.warning( + f"Skipping '{service_with_local_runtime}' as it is set to run locally" + ) + span.set_data("service_name", service.name) + span.set_data("mode", mode) + span.set_data( + "local_runtime_dependency_count", len(local_runtime_dependency_names) ) - exit(1) - except ModeDoesNotExistError as mde: - status.failure(str(mde)) - exit(1) - try: - _create_devservices_network() - except subprocess.CalledProcessError: - # Network already exists, ignore the error - pass + span.set_data( + "local_runtime_dependency_names", local_runtime_dependency_names + ) + + context_name = f"local_runtime_dependencies.{service.name}" + set_context( + context_name, + { + "service_name": service.name, + "mode": mode, + "exclude_local": exclude_local, + "count": len(local_runtime_dependency_names), + "names": list(local_runtime_dependency_names), + }, + ) + + remote_dependencies = _install_service_dependencies(service, mode, status) + _create_devservices_network() # Add the service to the starting services table state.update_service_entry(service.name, mode, StateTables.STARTING_SERVICES) mode_dependencies = modes[mode] @@ -168,17 +181,59 @@ def up(args: Namespace, existing_status: Status | None = None) -> None: status, ) status.warning(f"Continuing with service '{service.name}'") - try: - _up(service, [mode], remote_dependencies, mode_dependencies, status) - except DockerComposeError as dce: - capture_exception(dce, level="info") - status.failure(f"Failed to start {service.name}: {dce.stderr}") - exit(1) + with start_span(op="service.startup", name="Start service") as span: + span.set_data("service_name", service.name) + span.set_data("mode", mode) + try: + _up(service, [mode], remote_dependencies, mode_dependencies, status) + except DockerComposeError as dce: + capture_exception(dce, level="info") + status.failure(f"Failed to start {service.name}: {dce.stderr}") + exit(1) # TODO: We should factor in healthchecks here before marking service as running state.remove_service_entry(service.name, StateTables.STARTING_SERVICES) state.update_service_entry(service.name, mode, StateTables.STARTED_SERVICES) +def _install_service_dependencies( + service: Service, mode: str, status: Status +) -> set[InstalledRemoteDependency]: + with start_span(op="service.dependencies", name="Install dependencies") as span: + status.info("Retrieving dependencies") + span.set_data("service_name", service.name) + span.set_data("mode", mode) + try: + remote_dependencies = install_and_verify_dependencies( + service, force_update_dependencies=True, modes=[mode] + ) + span.set_data("remote_dependency_count", len(remote_dependencies)) + return remote_dependencies + except DependencyError as de: + capture_exception(de) + status.failure( + f"{str(de)}. If this error persists, try running `devservices purge`" + ) + exit(1) + except ModeDoesNotExistError as mde: + status.failure(str(mde)) + exit(1) + + +def _create_devservices_network() -> None: + with start_span(op="service.network", name="Create devservices network") as span: + try: + subprocess.run( + ["docker", "network", "create", "devservices"], + stdout=subprocess.DEVNULL, + stderr=subprocess.DEVNULL, + check=True, + ) + span.set_data("network_created", True) + except subprocess.CalledProcessError: + # Network already exists, ignore the error + span.set_data("network_created", False) + + def _pull_dependency_images( cmd: DockerComposeCommand, current_env: dict[str, str], status: Status ) -> None: @@ -285,12 +340,3 @@ def _up( except ContainerHealthcheckFailedError as e: status.failure(str(e)) exit(1) - - -def _create_devservices_network() -> None: - subprocess.run( - ["docker", "network", "create", "devservices"], - stdout=subprocess.DEVNULL, - stderr=subprocess.DEVNULL, - check=True, - ) From d0514a550c88912dc9e1d4ca6416db4f60df52fd Mon Sep 17 00:00:00 2001 From: Ian Woodard <17186604+IanWoodard@users.noreply.github.com> Date: Wed, 28 May 2025 20:21:20 -0500 Subject: [PATCH 2/3] Addressing review feedback --- devservices/commands/up.py | 1 + 1 file changed, 1 insertion(+) diff --git a/devservices/commands/up.py b/devservices/commands/up.py index fe709263..206d230f 100644 --- a/devservices/commands/up.py +++ b/devservices/commands/up.py @@ -184,6 +184,7 @@ def up(args: Namespace, existing_status: Status | None = None) -> None: with start_span(op="service.startup", name="Start service") as span: span.set_data("service_name", service.name) span.set_data("mode", mode) + span.set_data("exclude_local", exclude_local) try: _up(service, [mode], remote_dependencies, mode_dependencies, status) except DockerComposeError as dce: From 65e9f90aec4eef2978b6ca1c3968f946229a7d43 Mon Sep 17 00:00:00 2001 From: Ian Woodard <17186604+IanWoodard@users.noreply.github.com> Date: Wed, 28 May 2025 21:02:06 -0500 Subject: [PATCH 3/3] Improving span coverage, improving naming --- devservices/commands/up.py | 33 +++++++++++++++++++++++---------- 1 file changed, 23 insertions(+), 10 deletions(-) diff --git a/devservices/commands/up.py b/devservices/commands/up.py index 206d230f..68116ec6 100644 --- a/devservices/commands/up.py +++ b/devservices/commands/up.py @@ -107,7 +107,7 @@ def up(args: Namespace, existing_status: Status | None = None) -> None: ) as status: local_runtime_dependency_names = set() with start_span( - op="service.dependencies", name="Check local runtime dependencies" + op="service.dependencies.check", name="Check local runtime dependencies" ) as span: services_with_local_runtime = state.get_services_by_runtime( ServiceRuntime.LOCAL @@ -181,7 +181,9 @@ def up(args: Namespace, existing_status: Status | None = None) -> None: status, ) status.warning(f"Continuing with service '{service.name}'") - with start_span(op="service.startup", name="Start service") as span: + with start_span( + op="service.containers.start", name="Start service containers" + ) as span: span.set_data("service_name", service.name) span.set_data("mode", mode) span.set_data("exclude_local", exclude_local) @@ -199,7 +201,9 @@ def up(args: Namespace, existing_status: Status | None = None) -> None: def _install_service_dependencies( service: Service, mode: str, status: Status ) -> set[InstalledRemoteDependency]: - with start_span(op="service.dependencies", name="Install dependencies") as span: + with start_span( + op="service.dependencies.install", name="Install dependencies" + ) as span: status.info("Retrieving dependencies") span.set_data("service_name", service.name) span.set_data("mode", mode) @@ -221,7 +225,9 @@ def _install_service_dependencies( def _create_devservices_network() -> None: - with start_span(op="service.network", name="Create devservices network") as span: + with start_span( + op="service.network.create", name="Create devservices network" + ) as span: try: subprocess.run( ["docker", "network", "create", "devservices"], @@ -238,17 +244,24 @@ def _create_devservices_network() -> None: def _pull_dependency_images( cmd: DockerComposeCommand, current_env: dict[str, str], status: Status ) -> None: - run_cmd(cmd.full_command, current_env, retries=4) - for dependency in cmd.services: - status.info(f"Pulled image for {dependency}") + with start_span(op="service.images.pull", name="Pull dependency images") as span: + span.set_data("command", cmd.full_command) + run_cmd(cmd.full_command, current_env, retries=4) + for dependency in cmd.services: + status.info(f"Pulled image for {dependency}") def _bring_up_dependency( cmd: DockerComposeCommand, current_env: dict[str, str], status: Status ) -> None: - for dependency in cmd.services: - status.info(f"Starting {dependency}") - run_cmd(cmd.full_command, current_env) + with start_span( + op="service.containers.up", name="Bring up dependency containers" + ) as span: + span.set_data("command", cmd.full_command) + span.set_data("services", cmd.services) + for dependency in cmd.services: + status.info(f"Starting {dependency}") + run_cmd(cmd.full_command, current_env) def _up(