feat: add parameters to disable metrics

[qusuyan]

Metrics can be disabled by adding METRICS_ENABLED=false when running task reexecute-cchain-range

Why this should be merged

It simplifies running reexecution benchmark when tests do not require collecting metrics.

How this works

Add an environment variable to task.yml that disables metric logging.

How this was tested

Reexecuted to block 200. Since I do not have metric collection set up, being able to run the benchmark indicates that the parameter is properly set. I also added a print statement for metricsEnabledArg to assert that the parameters are properly set while testing.

Need to be documented in RELEASES.md?

No

[aaronbuchwald]

This results in some subtle behavior that we should prefer to make explicit.

By setting the default to "", this variable is falsy and will not be appended in the benchmark script, which means the default of "" subtly results in defaulting to enabled, but that's not obvious at first glance.

Could we make this explicit by making the default true in both places rather

[RodrigoVillar]

I think that instead of having to set METRICS_ENABLED=false, we should just make metrics collection disabled by default with the option of enabling it via a flag. For example, we already do this with tmpnet and the load test where metrics collection needs to be explicitly enabled if developers want their metrics to be enabled.

[qusuyan]

I think that instead of having to set METRICS_ENABLED=false, we should just make metrics collection disabled by default with the option of enabling it via a flag. For example, we already do this with tmpnet and the load test where metrics collection needs to be explicitly enabled if developers want their metrics to be enabled.

You gonna fight @aaronbuchwald for a consensus lol

[aaronbuchwald]

I think that instead of having to set METRICS_ENABLED=false, we should just make metrics collection disabled by default with the option of enabling it via a flag. For example, we already do this with tmpnet and the load test where metrics collection needs to be explicitly enabled if developers want their metrics to be enabled.

You gonna fight @aaronbuchwald for a consensus lol

Either way we should use the same default in Taskfile and the golang version. I'd prefer that they are enabled by default and if we change the default, we should update the README to reflect that as well.

I think this is good to merge now as it adds the option and bubbles up the flag and current default through the Taskfile. If there's more feedback that it should default to false and the CI jobs should enable it, then we can make that a separate PR and make sure that it:

• Updates the README to reflect that metrics gathering is optionally enabled and requires prometheus vars to be set
• Updates the default in the golang definition, updates the taskfile to use default "" since this would now match the default of the golang version

[RodrigoVillar]

Regarding whether metrics should be disabled by default, happy to punt that discussion to a separate issue/PR.

[joshua-kim]

How useful this test is without these metrics? I'm wondering if we should just always run the metrics server either way (i.e do we even want this flag at all?), the metrics server is not publicly accessible since it runs in CI so I don't understand why we would care if it was running even if we weren't using it.

[aaronbuchwald]

Yes, this is direct feedback from @qusuyan and @demosdemon that they prefer this to at least be optional to make local testing without configuring collection easier ie. setting the prometheus credentials for collection.

Even without the detailed metrics, it still produces top-level metrics ie. mgas/s, re-executes a range of blocks to produce a new version of the current state, and can serve as a smoke test of whether or not the VM is able to correctly process the range of blocks.

[aaronbuchwald]

Ref #4104 (comment)

[maru-ava]

As per offline discussion, a follow-up PR will ensure metrics are disabled by default which will allow the default for the script to run locally when metrics haven't been configured.