ricoberger/script_exporter

Script Exporter max_timeout not taking effect if query or header timeouts not specified

cah-rita-kassar opened this issue · 3 comments

Configuring the maximum script timeout in Script Exporter's config.yaml file is not taking effect unless query or header timeouts are being specified alongside. Scripts are executing successfully even when their duration exceeds the max_timeout set.

I created a test_script that sleeps for 5 seconds before executing an echo command.

ping 192.0.2.0 -n 1 -w 5000 >nul
ECHO Script executed successfully 

Then, in Script Exporter's config.yaml file, the script's max_timeout was set to 1 second.

scripts:
  - name: test_script
    command: .\test_script.bat
    timeout:
      max_timeout: 1.0
      enforced: true 

The script was expected to be killed. Instead, it executed successfully.

image-2023-03-17-11-20-31-388

However, the max_timeout worked as expected when passed as a header in the GET request on Script Exporter's probe endpoint. As a result, the script was killed and did not return any value.
image-2023-03-17-13-14-37-832

ts=2023-03-17T11:11:19.895Z caller=metrics.go:77 level=error msg="Run script failed" err="exit status 1" 

The max_timeout also killed the test_script when the timeout was passed as a query in Script Exporter's probe endpoint.
image-2023-03-23-12-52-53-475

ts=2023-03-23T10:23:41.217Z caller=metrics.go:77 level=error msg="Run script failed" err="exit status 1"

I think I found out how this is happening: https://github.com/ricoberger/script_exporter/blob/v2.11.0/pkg/exporter/scripts.go#L121

The code clearly shows that this will be the expected behavior

But is it in the intended behavior or is it a bug?

Hi @cah-rita-kassar, thanks for reporting. I'm with you and think it makes more sense to return the configured max timeout when the query parameter and header is not present. I created #78 to change the behavior accordingly.

@siebenmann since you contributed this feature, what do you think about it?

I agree with the change. I probably missed that this required the header set when I submitted the initial change (I don't think I touched that code at all), and always tested in a situation where I was submitting the header (or a timeout URL parameter).

Thanks for the fast feedback 🙂