Skip to content

Commit 8b856cb

Browse files
committed
Azure: more improvements on logs
- Use lazy mode for matting with logger - Use a standard for logging messages - Performance improvements on some DEBUG logs Refers to SPSTRAT-583 Signed-off-by: Jonathan Gangi <[email protected]>
1 parent 50e7393 commit 8b856cb

File tree

3 files changed

+47
-33
lines changed

3 files changed

+47
-33
lines changed

cloudpub/ms_azure/service.py

Lines changed: 28 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,10 @@ def _configure(self, data: Dict[str, Any]) -> ConfigureStatus:
103103
Returns:
104104
The job ID to track its status alongside the initial status.
105105
"""
106-
log.debug("Received the following data to create/modify: %s" % json.dumps(data, indent=2))
106+
if log.isEnabledFor(logging.DEBUG):
107+
log.debug(
108+
"Received the following data to create/modify: %s", json.dumps(data, indent=2)
109+
)
107110
resp = self.session.post(path="configure", json=data)
108111
self._raise_for_status(response=resp)
109112
rsp_data = resp.json()
@@ -121,17 +124,19 @@ def _query_job_details(self, job_id: str) -> ConfigureStatus:
121124
Returns:
122125
The updated job status.
123126
"""
124-
log.debug(f"Query job details for \"{job_id}\"")
127+
log.debug("Query job details for \"%s\"", job_id)
125128
resp = self.session.get(path=f"configure/{job_id}/status")
126129

127130
# We don't want to fail if there's a server error thus we make a fake
128131
# response for it so the query job details will be retried.
129132
if resp.status_code >= 500:
130133
log.warning(
131134
(
132-
f"Got HTTP {resp.status_code} from server when querying job {job_id} status."
133-
" Considering the job_status as \"pending\"."
134-
)
135+
"Got HTTP %s from server when querying job %s status."
136+
" Considering the job_status as \"pending\".",
137+
),
138+
resp.status_code,
139+
job_id,
135140
)
136141
return ConfigureStatus.from_json(
137142
{
@@ -177,7 +182,7 @@ def _wait_for_job_completion(self, job_id: str) -> ConfigureStatus:
177182
error_message = f"Job {job_id} failed: \n{job_details.errors}"
178183
self._raise_error(InvalidStateError, error_message)
179184
elif job_details.job_result == "succeeded":
180-
log.debug(f"Job {job_id} succeeded")
185+
log.debug("Job %s succeeded", job_id)
181186
return job_details
182187

183188
def configure(self, resource: AzureResource) -> ConfigureStatus:
@@ -194,7 +199,8 @@ def configure(self, resource: AzureResource) -> ConfigureStatus:
194199
"$schema": self.CONFIGURE_SCHEMA.format(AZURE_API_VERSION=self.AZURE_API_VERSION),
195200
"resources": [resource.to_json()],
196201
}
197-
log.debug("Data to configure: %s", json.dumps(data, indent=2))
202+
if log.isEnabledFor(logging.DEBUG):
203+
log.debug("Data to configure: %s", json.dumps(data, indent=2))
198204
res = self._configure(data=data)
199205
return self._wait_for_job_completion(job_id=res.job_id)
200206

@@ -601,16 +607,18 @@ def publish(self, metadata: AzurePublishingMetadata) -> None:
601607
plan_name = metadata.destination.split("/")[-1]
602608
product, plan = self.get_product_plan_by_name(product_name, plan_name)
603609
log.info(
604-
"Preparing to associate the image \"%s\" with the plan \"%s\" from product \"%s\""
605-
% (metadata.image_path, plan_name, product_name)
610+
"Preparing to associate the image \"%s\" with the plan \"%s\" from product \"%s\"",
611+
metadata.image_path,
612+
plan_name,
613+
product_name,
606614
)
607615

608616
# 2. Retrieve the VM Technical configuration for the given plan
609-
log.info("Retrieving the technical config for \"%s\"." % metadata.destination)
617+
log.info("Retrieving the technical config for \"%s\".", metadata.destination)
610618
tech_config = self.get_plan_tech_config(product, plan)
611619

612620
# 3. Prepare the Disk Version
613-
log.info("Creating the VMImageResource with SAS for image: \"%s\"" % metadata.image_path)
621+
log.info("Creating the VMImageResource with SAS for image: \"%s\"", metadata.image_path)
614622
sas = OSDiskURI(uri=metadata.image_path)
615623
source = VMImageSource(source_type="sasUri", os_disk=sas.to_json(), data_disks=[])
616624

@@ -640,8 +648,10 @@ def publish(self, metadata: AzurePublishingMetadata) -> None:
640648
# Check the images of the selected DiskVersion if it exists
641649
if disk_version:
642650
log.info(
643-
"DiskVersion \"%s\" exists in \"%s\" for the image \"%s\"."
644-
% (disk_version.version_number, metadata.destination, metadata.image_path)
651+
"DiskVersion \"%s\" exists in \"%s\" for the image \"%s\".",
652+
disk_version.version_number,
653+
metadata.destination,
654+
metadata.image_path,
645655
)
646656
disk_version = set_new_sas_disk_version(disk_version, metadata, source)
647657

@@ -651,20 +661,21 @@ def publish(self, metadata: AzurePublishingMetadata) -> None:
651661
tech_config.disk_versions.append(disk_version)
652662
else:
653663
log.info(
654-
"The destination \"%s\" already contains the SAS URI: \"%s\"."
655-
% (metadata.destination, metadata.image_path)
664+
"The destination \"%s\" already contains the SAS URI: \"%s\".",
665+
metadata.destination,
666+
metadata.image_path,
656667
)
657668

658669
# 4. With the updated disk_version we should adjust the SKUs and submit the changes
659670
if disk_version:
660-
log.info("Updating SKUs for \"%s\"." % metadata.destination)
671+
log.info("Updating SKUs for \"%s\".", metadata.destination)
661672
tech_config.skus = update_skus(
662673
disk_versions=tech_config.disk_versions,
663674
generation=metadata.generation,
664675
plan_name=plan_name,
665676
old_skus=tech_config.skus,
666677
)
667-
log.info("Updating the technical configuration for \"%s\"." % metadata.destination)
678+
log.info("Updating the technical configuration for \"%s\".", metadata.destination)
668679
self.configure(resource=tech_config)
669680

670681
# 5. Proceed to publishing if it was requested.

cloudpub/ms_azure/session.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ def __init__(self, json: Dict[str, str]):
2424
"""
2525
self.expires_on = datetime.fromtimestamp(int(json["expires_on"]))
2626
self.access_token = json["access_token"]
27-
log.debug(f"Obtained token with expiration date on {self.expires_on}")
27+
log.debug("Obtained token with expiration date on %s", self.expires_on)
2828

2929
def is_expired(self) -> bool:
3030
"""Return True if the token is expired and False otherwise."""
@@ -108,7 +108,7 @@ def _validate_auth_keys(auth_keys: Dict[str, str]) -> Dict[str, str]:
108108
"AZURE_API_SECRET",
109109
]
110110
for key in mandatory_keys:
111-
log.debug(f"Validating mandatory key \"{key}\"")
111+
log.debug("Validating mandatory key \"%s\"", key)
112112
if key not in auth_keys.keys() or not auth_keys.get(key):
113113
err_msg = f'The key/value for "{key}" must be set.'
114114
log.error(err_msg)
@@ -156,7 +156,7 @@ def _request(
156156
params = {}
157157
params.update(self._mandatory_params)
158158

159-
log.debug(f"Sending a {method} request to {path}")
159+
log.debug("Sending a %s request to %s", method, path)
160160
formatted_url = self._prefix_url.format(**self.auth_keys)
161161
url = join_url(formatted_url, path)
162162
return self.session.request(method, url=url, params=params, headers=headers, **kwargs)

cloudpub/ms_azure/utils.py

Lines changed: 16 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -150,22 +150,23 @@ def is_sas_eq(sas1: str, sas2: str, base_only=False) -> bool:
150150

151151
# Base URL differs
152152
if base_sas1 != base_sas2:
153-
log.debug("Got different base SAS: %s - Expected: %s" % (base_sas1, base_sas2))
153+
log.debug("Got different base SAS: %s - Expected: %s", base_sas1, base_sas2)
154154
return False
155155

156156
if not base_only:
157157
# Parameters lengh differs
158158
if len(params_sas1) != len(params_sas2):
159159
log.debug(
160-
"Got different lengh of SAS parameters: len(%s) - Expected len(%s)"
161-
% (params_sas1, params_sas2)
160+
"Got different lengh of SAS parameters: len(%s) - Expected len(%s)",
161+
params_sas1,
162+
params_sas2,
162163
)
163164
return False
164165

165166
# Parameters values differs
166167
for k, v in params_sas1.items():
167168
if v != params_sas2.get(k, None):
168-
log.debug("The SAS parameter %s doesn't match %s." % (v, params_sas2.get(k, None)))
169+
log.debug("The SAS parameter %s doesn't match %s.", v, params_sas2.get(k, None))
169170
return False
170171

171172
# Equivalent SAS
@@ -203,8 +204,8 @@ def is_azure_job_not_complete(job_details: ConfigureStatus) -> bool:
203204
Returns:
204205
bool: False if job completed, True otherwise
205206
"""
206-
log.debug(f"Checking if the job \"{job_details.job_id}\" is still running")
207-
log.debug(f"job {job_details.job_id} is in {job_details.job_status} state")
207+
log.debug("Checking if the job \"%s\" is still running", job_details.job_id)
208+
log.debug("job %s is in %s state", job_details.job_id, job_details.job_status)
208209
if job_details.job_status != "completed":
209210
return True
210211
return False
@@ -559,14 +560,15 @@ def set_new_sas_disk_version(
559560
log.info("Setting up a new SAS disk version for \"%s\"", metadata.image_path)
560561
# If we already have a VMImageDefinition let's use it
561562
if disk_version.vm_images:
562-
log.debug("The DiskVersion \"%s\" contains inner images." % disk_version.version_number)
563+
log.debug("The DiskVersion \"%s\" contains inner images.", disk_version.version_number)
563564
img, img_legacy = vm_images_by_generation(disk_version, metadata.architecture)
564565

565566
# Now we replace the SAS URI for the vm_images
566567
log.info(
567568
"Adjusting the VMImages from existing DiskVersion \"%s\""
568-
"to fit the new image with SAS \"%s\"."
569-
% (disk_version.version_number, metadata.image_path)
569+
"to fit the new image with SAS \"%s\".",
570+
disk_version.version_number,
571+
metadata.image_path,
570572
)
571573
disk_version.vm_images = prepare_vm_images(
572574
metadata=metadata,
@@ -578,11 +580,12 @@ def set_new_sas_disk_version(
578580
# If no VMImages, we need to create them from scratch
579581
else:
580582
log.debug(
581-
"The DiskVersion \"%s\" does not contain inner images." % disk_version.version_number
583+
"The DiskVersion \"%s\" does not contain inner images.", disk_version.version_number
582584
)
583585
log.info(
584-
"Setting the new image \"%s\" on DiskVersion \"%s\"."
585-
% (metadata.image_path, disk_version.version_number)
586+
"Setting the new image \"%s\" on DiskVersion \"%s\".",
587+
metadata.image_path,
588+
disk_version.version_number,
586589
)
587590
disk_version.vm_images = create_vm_image_definitions(metadata, source)
588591

@@ -592,4 +595,4 @@ def set_new_sas_disk_version(
592595
def logdiff(diff: DeepDiff) -> None:
593596
"""Log the offer diff if it exists."""
594597
if diff:
595-
log.warning(f"Found the following offer diff before publishing:\n{diff.pretty()}")
598+
log.warning("Found the following offer diff before publishing:\n%s", diff.pretty())

0 commit comments

Comments
 (0)