[brun0-matheus]

Bug: b/577973994

Overview

Instruments the code to be able to easily emit metrics related to the duration of each fuzz_task step. The metrics are emitted both to Cloud Monitoring (using only a combination of fuzzer name and job, to keep the cardinality low) and Big Query.

Changes

• Create TimeStatTracker, which is a reusable context manager that measures the time spent in its code context, saving it to a local dictionary (used to send the measurements to FuzzTaskOutput protobuf later on) and emitting it to a supplied Cloud Monitoring metric.
• Add fuzz_task_steps_duration to FuzzTaskOutput protobuf, which is a map of string to interval duration, which allows to easily extend the code to support additional steps.
• Add the has_field method in Metric, in order to make sure the metric supplied to TimeStatTracker has the correct fields.
• Create the FUZZ_TASK_STEPS_DURATION metric in Cloud Monitoring. It contains the same labels as FUZZING_SESSION_DURATION, with the addition of the step and success labels.
• Add the fuzz_task_steps_duration field in the JobRun schema (BigQuery), which contains the duration of all steps. Currently it only contains the bad_build_check duration.
• Simplify the load_bigquery_stats test. Now it uses the schema defined in fuzzer_stats.py, avoiding duplicated code.

Tests

Additional unit tests created. I ran tests in the dev environment with the bad_build check implemented.
Evidence that the CloudMonitoring metric is being tracked: https://screenshot.corp.google.com/2u8b1d3psrv68.
Evidence that the BigQuery metric is being tracked: https://screenshot-v2.corp.google.com/73759kq0eje1o.

[ViniciustCosta]

Let's do the bad_build_check metric in a separate PR and only add the time tracker structure and plumbing needed for the metrics in this one, so it's easier to review.

[ViniciustCosta]

Are there cases when the same step runs more than once (loops, retries)? Right now it warns and overwrites. Should durations be summed instead?

[brun0-matheus]

Yes, there are. In this case I opted to make a tracker for each loop (as in PR #5569). Maybe summing it up is a simpler solution.

As a matter of fact, I opted for the first solution (make a tracker for each loop) in order to get the stats for TestcaseRun, but later on (PR #5571) I had to do the same for blackbox fuzzing, so now I don't think it's the best solution anymore.

[brun0-matheus]

Done, now the time measurements are accumulated.

[ViniciustCosta]

Hmmm with this accumulation, stats holds the total per session, but the metric still gets one sample per call in Cloud Monitorign.

For steps inside loops, BQ (session total) and CM (per-iteration distribution) will mean different things. We could send the total once per step at the end.

In any case, please document it in the metric description.

[brun0-matheus]

Ah, yes, that's intended, I forgot to add a comment explaining this. Sending the total once the steps end is a little bit difficult to do because of the following reasons:
1 - you need to define an extra method to mark the step as finished. This can add to confusion if you track the same step after you marked it as finished.
2 - you can't easily define success. If you had 5 steps, 4 were successful and 1 wasn't, what do you do?

Also, by sending the measurements in each step you gain granularity, so I don't see any problem in doing this.

[brun0-matheus]

I updated the metric description to include this scenario.

[ViniciustCosta]

Makes sense to me! As we discussed offline, we might need to revisit that since some steps can loop over more than 100 iterations.

1. Not sure if that's a problem, we can guarantee consistency in the code itself.
2. Yeah, that's a good point, not sure the most correct approach here.

[brun0-matheus]

Let's do the bad_build_check metric in a separate PR and only add the time tracker structure and plumbing needed for the metrics in this one, so it's easier to review.

Done, now the bad build check PR is #5574.

[ViniciustCosta]

Won't this break the JobRun load since STEPS_DURATION_FIELDS is empty for now?

Maybe we should leve the BQ schema field to be add it in the first real step.

[brun0-matheus]

I will rerun the tests in the dev environment once we're done with reviewing, so let's see if this breaks.

[ViniciustCosta]

Hmmm with this accumulation, stats holds the total per session, but the metric still gets one sample per call in Cloud Monitorign.

For steps inside loops, BQ (session total) and CM (per-iteration distribution) will mean different things. We could send the total once per step at the end.

In any case, please document it in the metric description.