[Serve] log deployment config in controller logs

[KeeProMise]

Description

[Serve] log deployment config in controller logs

Related issues

#59167

Additional information

Optional: Add implementation details, API changes, usage examples, screenshots, etc.

[gemini-code-assist]

Code Review

This pull request introduces logging for deployment configurations in Ray Serve, which is a valuable addition for debugging. The new _log_deployment_configs function logs declarative, imperative, and merged configurations. My review focuses on improving the implementation of this new function for better maintainability and readability by reducing code duplication.

[gemini-code-assist]

The _log_deployment_configs function can be refactored for better readability and to reduce code duplication.

1. The logic for formatting imperative_configs and merged_configs is identical. This can be extracted into a nested helper function, _format_deployment_infos, to avoid repetition.
2. The creation of declarative_dict can be simplified using a dictionary comprehension. The hasattr(dep, "dict") check is also redundant because declarative_config.deployments is a list of DeploymentSchema pydantic models, which are guaranteed to have a .dict() method.

Here is the suggested refactoring:

def _log_deployment_configs(
    app_name: str,
    declarative_config: ServeApplicationSchema,
    imperative_configs: Dict[str, DeploymentInfo],
    merged_configs: Dict[str, DeploymentInfo],
) -> None:
    """Log the declarative, imperative, and merged deployment configs.
    
    This function logs the deployment configuration after each update to help
    with debugging and understanding how declarative (from YAML/config) and
    imperative (from code) configurations are merged.
    
    Args:
        app_name: Name of the application.
        declarative_config: The declarative config from the YAML/config file.
        imperative_configs: The imperative configs from the application code.
        merged_configs: The final merged configs after applying overrides.
    """
    try:
        def _format_deployment_infos(infos: Dict[str, DeploymentInfo]) -> Dict[str, Dict]:
            """Helper to format DeploymentInfo objects for logging."""
            return {
                dep_name: {
                    "deployment_config": dep_info.deployment_config.dict(),
                    "replica_config": {
                        "ray_actor_options": dep_info.replica_config.ray_actor_options,
                        "placement_group_bundles": dep_info.replica_config.placement_group_bundles,
                        "placement_group_strategy": dep_info.replica_config.placement_group_strategy,
                        "max_replicas_per_node": dep_info.replica_config.max_replicas_per_node,
                    },
                    "route_prefix": dep_info.route_prefix,
                    "version": dep_info.version,
                }
                for dep_name, dep_info in infos.items()
            }

        # Format declarative config
        declarative_dict = {
            dep.name: dep.dict(exclude_unset=True)
            for dep in declarative_config.deployments
        } if declarative_config.deployments else {}
        
        # Format imperative and merged configs
        imperative_dict = _format_deployment_infos(imperative_configs)
        merged_dict = _format_deployment_infos(merged_configs)
        
        # Log the configurations
        logger.info(
            f"Application '{app_name}' deployment configs after update:\n"
            f"Declarative config (from YAML/config): {json.dumps(declarative_dict, indent=2, default=str)}\n"
            f"Imperative config (from code): {json.dumps(imperative_dict, indent=2, default=str)}\n"
            f"Merged config (final): {json.dumps(merged_dict, indent=2, default=str)}"
        )
    except Exception as e:
        # Don't fail the deployment if logging fails
        logger.warning(
            f"Failed to log deployment configs for application '{app_name}': {e}"
        )

[abrarsheikh]

These changes seem overly complex for a simple task like logging config. Is there a architectural reason why logging just the deployment config is difficult?

[KeeProMise]

These changes seem overly complex for a simple task like logging config. Is there a architectural reason why logging just the deployment config is difficult?

Thanks for the feedback. Indeed it looks complex, but this is caused by architectural limitations.
Architectural reasons:
ReplicaConfig contains non-JSON-serializable byte fields:
serialized_deployment_def: bytes - serialized deployment code
serialized_init_args: bytes - serialized initialization arguments
serialized_init_kwargs: bytes - serialized initialization keyword arguments
These fields are used for transmitting code in Ray clusters and cannot be directly serialized to JSON for logging.
Why DeploymentInfo cannot be serialized directly:
If calling dict() or similar methods directly on DeploymentInfo, these byte fields in ReplicaConfig would cause JSON serialization to fail, or produce large amounts of meaningless binary data.
Current approach:
We only extract and log serializable configuration fields:
deployment_config (Pydantic model, directly serializable)
Serializable fields in replica_config (ray_actor_options, placement_group_bundles, etc.)
route_prefix and version