Add config option collect_timeout

[sudeephb]

Add a config option --collect-timeout which can be used to change the timeout used by the collectors while making subprocess calls.

[Pjack]

I suggest to provide only 1 configuration to user , maybe named "scrape_timeout".
We can align the timeout value between grafana-agent and commandline tool internal.

[Pjack]

Do we need to change the scrape_timeout as well? And I suggest to use the same name with grafana-agent.

[sudeephb]

@Pjack I have changed the name of the option.

[facundofc]

Even though a detail, I strongly disagree with the name change. Scraping is what prometheus does to the exporter, and it will lend to confusion using the same name here. Just imagine having a conversation about this exporter and scrape timeout comes up.

I'd suggest collect_timeout, as that is the right name for this concept: the exporter collects the data when prometheus scrapes the target.

[Pjack]

Even though a detail, I strongly disagree with the name change. Scraping is what prometheus does to the exporter, and it will lend to confusion using the same name here. Just imagine having a conversation about this exporter and scrape timeout comes up.

I'd suggest collect_timeout, as that is the right name for this concept: the exporter collects the data when prometheus scrapes the target.

scrape_timeout is configurable from exporter when we relate with grafana-agent. We also need to change it. That's why I suggest to use the same name. But collect_timeout or timeout , some common name are good to me too. I hope we only expose one configuration to user and we align the timeout within grafana-agent and subprocess internally.

[dashmage]

LGTM 🚀 Just left one small, non-blocking comment to get some info.

It does feel a bit wasteful that we pass the full Config object to each of the collector classes even though they just need to use thecollect_timeout parameter to send it to their Command super class. But I'm guessing it's not that much overhead and it's easier to manage all these config options in one place.

[sudeephb]

It does feel a bit wasteful that we pass the full Config object to each of the collector classes even though they just need to use thecollect_timeout parameter to send it to their Command super class.

If we only pass the collect_timeout option, we'd need to provide more arguments if we need to add extra config in the future, so I think managing all of them inside the config object is okay. Additionally, the config won't be big, it's just 5-6 options, so it's not a huge amount of data.

[jneo8]

Even though a detail, I strongly disagree with the name change. Scraping is what prometheus does to the exporter, and it will lend to confusion using the same name here. Just imagine having a conversation about this exporter and scrape timeout comes up.

I'd suggest collect_timeout, as that is the right name for this concept: the exporter collects the data when prometheus scrapes the target.

+1 for this. We should keep the exporter simple and leave the relate configuration with grafana-agent in the charm.