-
Notifications
You must be signed in to change notification settings - Fork 554
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Instrumenting build retrieval and unpacking times #4339
Open
vitorguidi
wants to merge
16
commits into
master
Choose a base branch
from
feature/build-retrieval-metrics
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+60
−0
Open
Changes from 13 commits
Commits
Show all changes
16 commits
Select commit
Hold shift + click to select a range
d53806a
Adding job type field to build_manager.setup_build
vitorguidi e33c3f4
Fix missing job_type argument in mock_setup_build for corpus_pruning
vitorguidi 878b426
Passing job type to auxiliary functions that instantiate build classes
vitorguidi 24c8eb4
Revert "Passing job type to auxiliary functions that instantiate buil…
vitorguidi 96b52a6
Revert "Fix missing job_type argument in mock_setup_build for corpus_…
vitorguidi 51c6f57
Revert "Adding job type field to build_manager.setup_build"
vitorguidi 108090d
Adding build retrieval time metric definition
vitorguidi 19b6f1e
Fix lint
vitorguidi eb8ae0c
Adding build download time metric
vitorguidi 5fc6dc5
Add remaining metrics
vitorguidi 436b853
Fix lint
vitorguidi ce3e364
Add time mock so unit tests pass
vitorguidi 2ba39ba
Adding platform to build retrieval metric definition
vitorguidi d0561ab
Using job label to conform to the previous standard
vitorguidi abf4d27
Improving fuzz target comments
vitorguidi 54fc387
Fix lint
vitorguidi File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -34,6 +34,7 @@ | |
from clusterfuzz._internal.google_cloud_utils import blobs | ||
from clusterfuzz._internal.google_cloud_utils import storage | ||
from clusterfuzz._internal.metrics import logs | ||
from clusterfuzz._internal.metrics import monitoring_metrics | ||
from clusterfuzz._internal.platforms import android | ||
from clusterfuzz._internal.system import archive | ||
from clusterfuzz._internal.system import environment | ||
|
@@ -431,7 +432,16 @@ def _download_and_open_build_archive(self, base_build_dir: str, | |
|
||
logs.info(f'Downloading build from {build_url} to {build_local_archive}.') | ||
try: | ||
start_time = time.time() | ||
storage.copy_file_from(build_url, build_local_archive) | ||
build_download_duration = time.time() - start_time | ||
monitoring_metrics.JOB_BUILD_RETRIEVAL_TIME.add( | ||
build_download_duration, { | ||
'fuzz_target': self.fuzz_target, | ||
'job_type': os.getenv('JOB_TYPE'), | ||
'platform': environment.platform(), | ||
'step': 'download', | ||
}) | ||
except Exception as e: | ||
logs.error(f'Unable to download build from {build_url}: {e}') | ||
raise | ||
|
@@ -475,6 +485,7 @@ def _open_build_archive(self, base_build_dir: str, build_dir: str, | |
if not can_unzip_over_http: | ||
return self._download_and_open_build_archive(base_build_dir, build_dir, | ||
build_url) | ||
# We do not emmit a metric for build download time, if using http | ||
logs.info("Opening an archive over HTTP, skipping archive download.") | ||
assert http_build_url | ||
return build_archive.open_uri(http_build_url) | ||
|
@@ -550,6 +561,17 @@ def _unpack_build(self, | |
utils.write_data_to_file('', partial_build_file_path) | ||
|
||
elapsed_time = time.time() - start_time | ||
monitoring_metrics.JOB_BUILD_RETRIEVAL_TIME.add( | ||
elapsed_time, | ||
{ | ||
# The concept of a fuzz target does not apply | ||
# to blackbox fuzzers | ||
'fuzz_target': self.fuzz_target, | ||
'job_type': os.getenv('JOB_TYPE'), | ||
'platform': environment.platform(), | ||
'step': 'unpack', | ||
}) | ||
|
||
elapsed_mins = elapsed_time / 60. | ||
log_func = logs.warning if elapsed_time > UNPACK_TIME_LIMIT else logs.info | ||
log_func(f'Build took {elapsed_mins:0.02f} minutes to unpack.') | ||
|
@@ -862,6 +884,9 @@ def _unpack_custom_build(self): | |
self.custom_binary_filename) | ||
custom_builds_bucket = local_config.ProjectConfig().get( | ||
'custom_builds.bucket') | ||
|
||
download_start_time = time.time() | ||
|
||
if custom_builds_bucket: | ||
directory = os.path.dirname(build_local_archive) | ||
if not os.path.exists(directory): | ||
|
@@ -872,6 +897,18 @@ def _unpack_custom_build(self): | |
build_local_archive): | ||
return False | ||
|
||
build_download_time = time.time() - download_start_time | ||
monitoring_metrics.JOB_BUILD_RETRIEVAL_TIME.add( | ||
build_download_time, | ||
{ | ||
# The concept of a fuzz target does not apply | ||
# to blackbox fuzzers | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: it's more that custom builds don't support fuzz targets today, so a better comment would be: "Custom builds don't support fuzz targets." (We could probably fix this if needed -- I think we did support this in the past right @jonathanmetzman ?) |
||
'fuzz_target': 'N/A', | ||
'job_type': os.getenv('JOB_TYPE'), | ||
'platform': environment.platform(), | ||
'step': 'download', | ||
}) | ||
|
||
# If custom binary is an archive, then unpack it. | ||
if archive.is_archive(self.custom_binary_filename): | ||
try: | ||
|
@@ -888,7 +925,20 @@ def _unpack_custom_build(self): | |
logs.log_fatal_and_exit('Could not make space for build.') | ||
|
||
try: | ||
# Unpack belongs to the BuildArchive class | ||
unpack_start_time = time.time() | ||
build.unpack(self.build_dir, trusted=True) | ||
build_unpack_time = time.time() - unpack_start_time | ||
monitoring_metrics.JOB_BUILD_RETRIEVAL_TIME.add( | ||
build_unpack_time, | ||
{ | ||
# The concept of a fuzz target does not apply | ||
# to blackbox fuzzers | ||
'fuzz_target': 'N/A', | ||
'job_type': os.getenv('JOB_TYPE'), | ||
'platform': environment.platform(), | ||
'step': 'unpack', | ||
}) | ||
except: | ||
build.close() | ||
logs.error('Unable to unpack build archive %s.' % build_local_archive) | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this comment correct here? we are setting
fuzz_target
here.