feat: allow configuration of idle instance timeout

[TheRayquaza]

Configurable idle instance timeout (SPIN_HTTP_IDLE_INSTANCE_TIMEOUT_SECS):

The HTTP trigger keeps a component instance alive for this many seconds after handling a request.
Previously hardcoded to 1 s.
Increase this on pods with bursty or low-frequency traffic to avoid repeated cold starts.

[kate-goldenring]

is this useful if instance reuse is not turned on? I think we should expand this to take advantage of all instance reuse fields spinframework/spin#3405. This may just be more env vars but there may be some validation cases to check amongst how the vars relate to each other. We could then add configuration for this in the SpinApp type in SpinKube to make this simpler to configure.

[TheRayquaza]

Hey @kate-goldenring thanks for taking the time to guide me on this, I will just add the env as it is already the case for other variables 👌

[kate-goldenring]

@TheRayquaza I am not sure i fully follow. Are you saying we can get the instance timeout config from env? Looks like we are explicitly setting the timeout, so i'd assume you are still not unblocked.

[TheRayquaza]

actually yes 👍 I think I am still blocked even after setting the SPIN_HTTP_IDLE_INSTANCE_TIMEOUT_SECS, it's never passed down to the CLI args because it's harcoded.

I think the best approach is by not passing any idle instance timeout parameter at all and letting clap read from env, what do you think @kate-goldenring ?

[kate-goldenring]

I think the best approach is by not passing any idle instance timeout parameter at all and letting clap read from env, what do you think @kate-goldenring ?

@TheRayquaza the shim does not do the same trigger initialization path as Spin CLI which resolves CliArgs via clap when the trigger child process is executed. Instead, here we explicitly create the CliArgs struct for each trigger. So we will need to in the least add equivalent env var resolution

[TheRayquaza]

@TheRayquaza the shim does not do the same trigger initialization path as Spin CLI which resolves CliArgs via clap when the trigger child process is executed. Instead, here we explicitly create the CliArgs struct for each trigger. So we will need to in the least add equivalent env var resolution

ok mb I just assumed clap was reading from env variable anyway

Since it must explicit passed, I think the change is already doing the work ?

[kate-goldenring]

Since it must explicit passed, I think the change is already doing the work ?

Yes, I think you're on track. Can you capture all HTTP trigger CLI args environment variables in a separate function to scope it (falling back to defaults if they are not set)?

[TheRayquaza]

Yes, I think you're on track. Can you capture all HTTP trigger CLI args environment variables in a separate function to scope it (falling back to defaults if they are not set)?

@kate-goldenring hey again, so now I have added this build_http_cli_args function to parse the arguments using http_trigger_env_config (with default values handled) down to the spin_trigger_http::CliArgs struct

[kate-goldenring]

@TheRayquaza thank you for factoring out the CliArgs creation into a separate struct. Can you avoid using clap and instead directly create the structure, directly checking for environment variable values?

[kate-goldenring]

@TheRayquaza can we avoid taking a dependency on clap? I'd rather explicitly create the CliArgs struct