Skip to content

Commit 75e1b1a

Browse files
authored
Instrumenting build retrieval and unpacking times (#4339)
### Motivation We currently lack metrics for build retrieval and unpacking times. This PR adds that, with granularity by fuzz target and job type. There are two different implementations for build downloading/unpacking: - In the Build class, from which RegularBuild, SplitTargetBuild, FuchsiaBuild and SymbolizedBuild inherit the downloading/unpacking behavior - In the CustomBuild class, which implements its own logic There are two possible cases for downloading/unpacking: clusterfuzz either downloads the whole build and unpacks it locally, or unpacks it remotely. This is the case for all build types except CustomBuild, which does not perform remote unpacking. For build retrieval over http, we do not track download time. For all the other cases, it suffices to keep track of start/finish time for download and unpacking. Finally, a _build_type is added to the constructor of the Build class, from which all other inherit. This is used to track the build type (debug or release), and is only mutated by SymbolizedBuild when attempting to fetch a debug build. Part of #4271
1 parent a27c6a9 commit 75e1b1a

File tree

3 files changed

+65
-0
lines changed

3 files changed

+65
-0
lines changed

src/clusterfuzz/_internal/build_management/build_manager.py

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
from clusterfuzz._internal.google_cloud_utils import blobs
3535
from clusterfuzz._internal.google_cloud_utils import storage
3636
from clusterfuzz._internal.metrics import logs
37+
from clusterfuzz._internal.metrics import monitoring_metrics
3738
from clusterfuzz._internal.platforms import android
3839
from clusterfuzz._internal.system import archive
3940
from clusterfuzz._internal.system import environment
@@ -332,6 +333,9 @@ def __init__(self,
332333
# This is used by users of the class to instruct the class which fuzz
333334
# target to unpack.
334335
self.fuzz_target = fuzz_target
336+
# Every fetched build is a release one, except when SymbolizedBuild
337+
# explicitly downloads a debug build
338+
self._build_type = 'release'
335339

336340
def _reset_cwd(self):
337341
"""Reset current working directory. Needed to clean up build
@@ -431,7 +435,16 @@ def _download_and_open_build_archive(self, base_build_dir: str,
431435

432436
logs.info(f'Downloading build from {build_url} to {build_local_archive}.')
433437
try:
438+
start_time = time.time()
434439
storage.copy_file_from(build_url, build_local_archive)
440+
build_download_duration = time.time() - start_time
441+
monitoring_metrics.JOB_BUILD_RETRIEVAL_TIME.add(
442+
build_download_duration, {
443+
'job': os.getenv('JOB_TYPE'),
444+
'platform': environment.platform(),
445+
'step': 'download',
446+
'build_type': self._build_type,
447+
})
435448
except Exception as e:
436449
logs.error(f'Unable to download build from {build_url}: {e}')
437450
raise
@@ -475,6 +488,7 @@ def _open_build_archive(self, base_build_dir: str, build_dir: str,
475488
if not can_unzip_over_http:
476489
return self._download_and_open_build_archive(base_build_dir, build_dir,
477490
build_url)
491+
# We do not emmit a metric for build download time, if using http
478492
logs.info("Opening an archive over HTTP, skipping archive download.")
479493
assert http_build_url
480494
return build_archive.open_uri(http_build_url)
@@ -506,6 +520,7 @@ def _unpack_build(self,
506520
try:
507521
with self._open_build_archive(base_build_dir, build_dir, build_url,
508522
http_build_url, unpack_everything) as build:
523+
unpack_start_time = time.time()
509524
unpack_everything = environment.get_value(
510525
'UNPACK_ALL_FUZZ_TARGETS_AND_FILES')
511526

@@ -537,6 +552,16 @@ def _unpack_build(self,
537552
fuzz_target=fuzz_target_to_unpack,
538553
trusted=trusted)
539554

555+
unpack_elapsed_time = time.time() - unpack_start_time
556+
557+
monitoring_metrics.JOB_BUILD_RETRIEVAL_TIME.add(
558+
unpack_elapsed_time, {
559+
'job': os.getenv('JOB_TYPE'),
560+
'platform': environment.platform(),
561+
'step': 'unpack',
562+
'build_type': self._build_type,
563+
})
564+
540565
except Exception as e:
541566
logs.error(f'Unable to unpack build archive {build_url}: {e}')
542567
return False
@@ -550,6 +575,7 @@ def _unpack_build(self,
550575
utils.write_data_to_file('', partial_build_file_path)
551576

552577
elapsed_time = time.time() - start_time
578+
553579
elapsed_mins = elapsed_time / 60.
554580
log_func = logs.warning if elapsed_time > UNPACK_TIME_LIMIT else logs.info
555581
log_func(f'Build took {elapsed_mins:0.02f} minutes to unpack.')
@@ -798,11 +824,14 @@ def _unpack_builds(self):
798824
return False
799825

800826
if self.release_build_url:
827+
# Expect self._build_type to be set in the constructor for Build
828+
assert self._build_type == 'release'
801829
if not self._unpack_build(self.base_build_dir, self.release_build_dir,
802830
self.release_build_url):
803831
return False
804832

805833
if self.debug_build_url:
834+
self._build_type = 'debug'
806835
if not self._unpack_build(self.base_build_dir, self.debug_build_dir,
807836
self.debug_build_url):
808837
return False
@@ -862,6 +891,9 @@ def _unpack_custom_build(self):
862891
self.custom_binary_filename)
863892
custom_builds_bucket = local_config.ProjectConfig().get(
864893
'custom_builds.bucket')
894+
895+
download_start_time = time.time()
896+
865897
if custom_builds_bucket:
866898
directory = os.path.dirname(build_local_archive)
867899
if not os.path.exists(directory):
@@ -872,6 +904,15 @@ def _unpack_custom_build(self):
872904
build_local_archive):
873905
return False
874906

907+
build_download_time = time.time() - download_start_time
908+
monitoring_metrics.JOB_BUILD_RETRIEVAL_TIME.add(
909+
build_download_time, {
910+
'job': os.getenv('JOB_TYPE'),
911+
'platform': environment.platform(),
912+
'step': 'download',
913+
'build_type': self._build_type,
914+
})
915+
875916
# If custom binary is an archive, then unpack it.
876917
if archive.is_archive(self.custom_binary_filename):
877918
try:
@@ -888,7 +929,17 @@ def _unpack_custom_build(self):
888929
logs.log_fatal_and_exit('Could not make space for build.')
889930

890931
try:
932+
# Unpack belongs to the BuildArchive class
933+
unpack_start_time = time.time()
891934
build.unpack(self.build_dir, trusted=True)
935+
build_unpack_time = time.time() - unpack_start_time
936+
monitoring_metrics.JOB_BUILD_RETRIEVAL_TIME.add(
937+
build_unpack_time, {
938+
'job': os.getenv('JOB_TYPE'),
939+
'platform': environment.platform(),
940+
'step': 'unpack',
941+
'build_type': self._build_type,
942+
})
892943
except:
893944
build.close()
894945
logs.error('Unable to unpack build archive %s.' % build_local_archive)

src/clusterfuzz/_internal/metrics/monitoring_metrics.py

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,19 @@
3232
monitor.BooleanField('bad_build'),
3333
])
3434

35+
JOB_BUILD_RETRIEVAL_TIME = monitor.CumulativeDistributionMetric(
36+
'task/build_retrieval_time',
37+
bucketer=monitor.FixedWidthBucketer(width=0.05, num_finite_buckets=20),
38+
description=('Distribution of fuzz task\'s build retrieval times. '
39+
'(grouped by fuzzer/job)'),
40+
field_spec=[
41+
monitor.StringField('job'),
42+
monitor.StringField('step'),
43+
monitor.StringField('platform'),
44+
monitor.StringField('build_type'),
45+
],
46+
)
47+
3548
FUZZER_KNOWN_CRASH_COUNT = monitor.CounterMetric(
3649
'task/fuzz/fuzzer/known_crash_count',
3750
description=('Count of fuzz task\'s known crash count '

src/clusterfuzz/_internal/tests/core/build_management/build_manager_test.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1155,6 +1155,7 @@ def test_setup(self):
11551155
def test_delete(self):
11561156
"""Test deleting this build."""
11571157
os.environ['JOB_NAME'] = 'job_custom'
1158+
self.mock.time.return_value = 1000.0
11581159
build = build_manager.setup_custom_binary()
11591160
self.assertTrue(os.path.isdir('/builds/job_custom/custom'))
11601161
build.delete()

0 commit comments

Comments
 (0)