Improve logging to include more identifiable information for default volume storage plugin

Author: vishesh92

engine/schema/src/main/java/com/cloud/storage/SnapshotVO.java

@@ -283,7 +283,7 @@ public Class<?> getEntityType() {
 
     @Override
     public String toString() {
-        return String.format("Snapshot %s", new ToStringBuilder(this, ToStringStyle.JSON_STYLE).append("uuid", getUuid()).append("name", getName())
+        return String.format("Snapshot %s", new ToStringBuilder(this, ToStringStyle.JSON_STYLE).append("id", getId()).append("uuid", getUuid()).append("name", getName())
                 .append("volumeId", getVolumeId()).toString());
     }
 }

engine/schema/src/main/java/com/cloud/storage/VolumeVO.java

@@ -511,10 +511,10 @@ public void setUpdated(Date updated) {
 
     @Override
     public String toString() {
-        return String.format("StoragePool %s",
+        return String.format("Volume %s",
                 ReflectionToStringBuilderUtils.reflectOnlySelectedFields(
                         this, "id", "name",
-                        "uuid", "volumeType", "instanceId", "path"));
+                        "uuid", "volumeType", "instanceId"));
     }
 
     @Override

engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/ImageStoreVO.java

@@ -33,6 +33,7 @@
 import com.cloud.storage.ScopeType;
 import com.cloud.utils.UriUtils;
 import com.cloud.utils.db.GenericDao;
+import org.apache.cloudstack.utils.reflectiontostringbuilderutils.ReflectionToStringBuilderUtils;
 
 @Entity
 @Table(name = "image_store")
@@ -215,4 +216,11 @@ public Long getUsedBytes() {
     public void setUsedBytes(Long usedBytes) {
         this.usedBytes = usedBytes;
     }
+
+    @Override
+    public String toString() {
+        return String.format("ImageStoreVO %s",
+                ReflectionToStringBuilderUtils.reflectOnlySelectedFields(
+                        this, "id", "name", "uuid"));
+    }
 }

engine/storage/image/src/main/java/org/apache/cloudstack/storage/image/store/ImageStoreImpl.java

@@ -228,4 +228,9 @@ public Void createDataDiskTemplateAsync(TemplateInfo dataDiskTemplate, String pa
         return driver.createDataDiskTemplateAsync(dataDiskTemplate, path, diskId, bootable, fileSize, callback);
     }
 
+    @Override
+    public String toString() {
+        return imageDataStoreVO.toString();
+    }
+
 }

engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/snapshot/SnapshotObject.java

@@ -41,8 +41,6 @@
 import org.apache.cloudstack.storage.datastore.db.SnapshotDataStoreDao;
 import org.apache.cloudstack.storage.datastore.db.SnapshotDataStoreVO;
 import org.apache.cloudstack.storage.to.SnapshotObjectTO;
-import org.apache.commons.lang3.builder.ToStringBuilder;
-import org.apache.commons.lang3.builder.ToStringStyle;
 import org.apache.logging.log4j.Logger;
 import org.apache.logging.log4j.LogManager;
 
@@ -471,7 +469,9 @@ public Class<?> getEntityType() {
 
     @Override
     public String toString() {
-        return String.format("Snapshot %s", new ToStringBuilder(this, ToStringStyle.JSON_STYLE).append("uuid", getUuid()).append("name", getName())
-                .append("volumeId", getVolumeId()).append("path", getPath()).toString());
+        return "VolumeObject{" +
+                "snapshotVO=" + getSnapshotVO() +
+                ", dataStore=" + getDataStore() +
+                '}';
     }
 }

engine/storage/volume/src/main/java/org/apache/cloudstack/storage/datastore/PrimaryDataStoreImpl.java

@@ -469,7 +469,7 @@ public StoragePoolType getParentPoolType() {
 
     @Override
     public String toString() {
-        return ReflectionToStringBuilderUtils.reflectOnlySelectedFields(this,
+        return ReflectionToStringBuilderUtils.reflectOnlySelectedFields(pdsv,
                 "id", "name", "uuid");
     }
 }

engine/storage/volume/src/main/java/org/apache/cloudstack/storage/volume/VolumeObject.java

@@ -944,4 +944,12 @@ public boolean isDeleteProtection() {
     public boolean isFollowRedirects() {
         return followRedirects;
     }
+
+    @Override
+    public String toString() {
+        return "VolumeObject{" +
+                "volumeVO=" + volumeVO +
+                ", dataStore=" + dataStore +
+                '}';
+    }
 }

plugins/storage/volume/default/src/main/java/org/apache/cloudstack/storage/datastore/driver/CloudStackPrimaryDataStoreDriverImpl.java

@@ -256,15 +256,20 @@ public void deleteAsync(DataStore dataStore, DataObject data, AsyncCompletionCal
                 }
             }
         } catch (Exception ex) {
-            logger.debug("Unable to destroy volume" + data.getId(), ex);
+            logger.debug(String.format(
+                    "Unable to destroy volume [id: %d, uuid: %s]",
+                    data.getId(), data.getUuid()), ex);
             result.setResult(ex.toString());
         }
         callback.complete(result);
     }
 
     @Override
     public void copyAsync(DataObject srcdata, DataObject destData, AsyncCompletionCallback<CopyCommandResult> callback) {
-        logger.debug(String.format("Copying volume %s(%s) to %s(%s)", srcdata.getId(), srcdata.getType(), destData.getId(), destData.getType()));
+        logger.debug(
+                "Copying volume [id: {}, uuid: {}, type:{}] to [id: {} uuid: {}, type: {}]",
+                srcdata.getId(), srcdata.getUuid(), srcdata.getType(),
+                destData.getId(), destData.getUuid(), destData.getType());
         boolean encryptionRequired = anyVolumeRequiresEncryption(srcdata, destData);
         DataStore store = destData.getDataStore();
         if (store.getRole() == DataStoreRole.Primary) {
@@ -381,7 +386,7 @@ public void takeSnapshot(SnapshotInfo snapshot, AsyncCompletionCallback<CreateCm
             callback.complete(result);
             return;
         } catch (Exception e) {
-            logger.debug("Failed to take snapshot: " + snapshot.getId(), e);
+            logger.debug(String.format("Failed to take snapshot: %s", snapshot), e);
             result = new CreateCmdResult(null, null);
             result.setResult(e.toString());
         }
@@ -416,7 +421,7 @@ public void revertSnapshot(SnapshotInfo snapshot, SnapshotInfo snapshotOnPrimary
                 }
             }
         } catch (Exception ex) {
-            logger.debug("Unable to revert snapshot " + snapshot.getId(), ex);
+            logger.debug(String.format("Unable to revert snapshot %s", snapshot), ex);
             result.setResult(ex.toString());
         }
         callback.complete(result);
@@ -476,7 +481,7 @@ private void updateVolumePathDetails(VolumeObject vol, ResizeVolumeAnswer answer
             if (storagePoolVO != null) {
                 volumeVO.setPoolId(storagePoolVO.getId());
             } else {
-                logger.warn(String.format("Unable to find datastore %s while updating the new datastore of the volume %d", datastoreUUID, vol.getId()));
+                logger.warn("Unable to find datastore {} while updating the new datastore of the volume {}", datastoreUUID, vol);
             }
         }
 

plugins/storage/volume/default/src/main/java/org/apache/cloudstack/storage/datastore/lifecycle/CloudStackPrimaryDataStoreLifeCycleImpl.java

@@ -335,16 +335,17 @@ private void validateVcenterDetails(Long zoneId, Long podId, Long clusterId, Str
                 if (answer != null) {
                     throw new InvalidParameterValueException("Provided vCenter server details does not match with the existing vCenter in zone id: " + zoneId);
                 } else {
-                    String msg = "Can not validate vCenter through host " + h.getId() + " due to ValidateVcenterDetailsCommand returns null";
+                    String msg = "Can not validate vCenter through host " + h + " due to ValidateVcenterDetailsCommand returns null";
                     logger.warn(msg);
                 }
             }
         }
         throw new CloudRuntimeException("Could not validate vCenter details through any of the hosts with in zone: " + zoneId + ", pod: " + podId + ", cluster: " + clusterId);
     }
 
-    protected boolean createStoragePool(long hostId, StoragePool pool) {
-        logger.debug("creating pool " + pool.getName() + " on  host " + hostId);
+    protected boolean createStoragePool(HostVO host, StoragePool pool) {
+        long hostId = host.getId();
+        logger.debug("creating pool {} on  host {}", pool.getName(), host);
 
         if (pool.getPoolType() != StoragePoolType.NetworkFilesystem && pool.getPoolType() != StoragePoolType.Filesystem &&
                 pool.getPoolType() != StoragePoolType.IscsiLUN && pool.getPoolType() != StoragePoolType.Iscsi && pool.getPoolType() != StoragePoolType.VMFS &&
@@ -363,10 +364,10 @@ protected boolean createStoragePool(long hostId, StoragePool pool) {
             primaryDataStoreDao.expunge(pool.getId());
             String msg = "";
             if (answer != null) {
-                msg = "Can not create storage pool through host " + hostId + " due to " + answer.getDetails();
+                msg = "Can not create storage pool through host " + host + " due to " + answer.getDetails();
                 logger.warn(msg);
             } else {
-                msg = "Can not create storage pool through host " + hostId + " due to CreateStoragePoolCommand returns null";
+                msg = "Can not create storage pool through host " + host + " due to CreateStoragePoolCommand returns null";
                 logger.warn(msg);
             }
             throw new CloudRuntimeException(msg);
@@ -392,7 +393,7 @@ public boolean attachCluster(DataStore store, ClusterScope scope) {
 
         boolean success = false;
         for (HostVO h : allHosts) {
-            success = createStoragePool(h.getId(), primarystore);
+            success = createStoragePool(h, primarystore);
             if (success) {
                 break;
             }