Skip to content
This repository has been archived by the owner on Jun 14, 2023. It is now read-only.

Refactor main.go, to use a config struct instead of global vars #18

Closed
colakong opened this issue May 20, 2020 · 2 comments · Fixed by #26
Closed

Refactor main.go, to use a config struct instead of global vars #18

colakong opened this issue May 20, 2020 · 2 comments · Fixed by #26

Comments

@colakong
Copy link
Collaborator

This will

  • help make handleWebhook easier to test
  • unblock development of some other features
@otakup0pe
Copy link
Contributor

We should probably support falling back to the env vars at least for a period of time. This would make the transition to new config format easier.

@colakong
Copy link
Collaborator Author

For some context on this issue and some of the following ones, this issue is meant to implement something like this:

type Config struct {
    listenAddr string
    verbose bool
    processDuration prometheus.Histogram
    processesCurrent prometheus.Gauge
    errCounter *prometheus.CounterVec
    rnr *runner
}

func (cfg *Config) handleWebhook(w http.ResponseWriter, req *http.Request) {
    cfg.rnr.run(...)
}

func main() {
    c := Config{
        ...
    }

    http.HandleFunc("/", c.handleWebhook)
}

We're passing the same function (now a method) to http.HandleFunc, but we can customize its behaviour with the Config struct. This is meant to make it easier to use handleWebhook in a test (#19)

Then in issue #20, we can populate values in the Config struct from a config file. In #21, we add support for config file sections, that specify executing different commands based on matching alarm labels. The same AMX_ env vars are still provided for those commands.

Issue #24 talks about adding functionality for providing alarm data to a command via STDIN. We can make it so that the AMX_ env vars are provided to a command, regardless of if STDIN was selected.

colakong added a commit that referenced this issue May 26, 2020
* resolves #18
* handleWebhook actually records prometheus metrics (available via the `/metrics` endpoint) for itself now
* Also add test cases for handleWebhook, handleHealth. resolves #19
    * Checks http responses
    * Check metrics observed by handleWebhook
* HTTP server can be stopped independently of program, so we have the option of running instances of it in test cases.
    * Test cases use a random available TCP port to bind to, HTTP if server is run
* Add signal-handling, to gracefully shut down HTTP server when program receives SIGINT (give existing requests 4 seconds to finish)
* Connect command stdout, stderr directly to log writer
colakong added a commit that referenced this issue Jun 4, 2020
* Switch to using config struct for program configuration

* resolves #18
* handleWebhook actually records prometheus metrics (available via the `/metrics` endpoint) for itself now
* Also add test cases for handleWebhook, handleHealth. resolves #19
    * Checks http responses
    * Check metrics observed by handleWebhook
* HTTP server can be stopped independently of program, so we have the option of running instances of it in test cases.
    * Test cases use a random available TCP port to bind to, HTTP if server is run
* Add signal-handling, to gracefully shut down HTTP server when program receives SIGINT (give existing requests 4 seconds to finish)
* Connect command stdout, stderr directly to log writer

* Don't attempt to execute the command, if we weren't able to decode prometheus alarm data properly

* Have the fake request for handleHealth match the path being used to route requests to it in real http server. It's a noop, but makes the test look more consistent~
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants