HTTP status monitoring

Question

I am fairly new to programming. I am writing a script which gets URLs and parameters through a config and makes http requests to check status, finally push a json string to Open-falcon server.

I am not sure if the logic and writing is reasonable. Could I get some guidance from someone experienced with Go. Really appreciate that. Thanks!

Code:

package main

import (
    "bytes"
    "encoding/json"
    "fmt"
    "io/ioutil"
    "net/http"
    "os"
    "strings"
    "time"

    "github.com/tkanos/gonfig"
)

type Config struct {
    Params []struct {
        Metric         string
        URL            string
        RequestMethod  string
        Data           string
        ExpectedString string
        Timeout        int8
        Step           int8
    }
}

const path = "config/config.json"

var c Config

func parseConfig(path string) Config {
    err := gonfig.GetConf(path, &c)
    if err != nil {
        panic(err)
    }
    return c
}

type Data struct {
    Metric string
    Step   int8
    Value  int8
}

var (
    l []Data
    m Data
)

func makeRequest(c Config) []Data {
    for _, i := range c.Params {
        m.Metric = i.Metric
        m.Step = i.Step
        if i.RequestMethod == "get" {
            client := &http.Client{Timeout: time.Duration(i.Timeout) * time.Second}
            resp, err := client.Get(i.URL)
            if err != nil {
                panic(err)
            }
            defer resp.Body.Close()
            if i.ExpectedString == "" {
                if resp.StatusCode >= 200 && resp.StatusCode <= 299 {
                    m.Value = 0
                } else {
                    m.Value = 1
                }
            } else {
                body, err := ioutil.ReadAll(resp.Body)
                if err != nil {
                    panic(err)
                }
                if strings.Contains(string(body), i.ExpectedString) {
                    m.Value = 0
                } else {
                    m.Value = 1
                }
            }
        } else {
            client := http.Client{Timeout: time.Duration(i.Timeout) * time.Second}
            resp, err := client.Post(i.URL, "application/json", strings.NewReader(i.Data))
            if err != nil {
                panic(err)
            }
            defer resp.Body.Close()
            if i.ExpectedString == "" {
                if resp.StatusCode >= 200 && resp.StatusCode <= 299 {
                    m.Value = 0
                } else {
                    m.Value = 1
                }
            } else {
                body, err := ioutil.ReadAll(resp.Body)
                if err != nil {
                    panic(err)
                }
                if strings.Contains(string(body), i.ExpectedString) {
                    m.Value = 0
                } else {
                    m.Value = 1
                }
            }

        }
        l = append(l, m)
    }

    fmt.Println(l)
    return l
}

type item struct {
    Endpoint    string `json:"endpoint"`
    Metric      string `json:"metric"`
    Timestamp   int64  `json:"timestamp"`
    Step        int8   `json:"step"`
    Value       int8   `json:"value"`
    CounterType string `json:"counterType"`
    Tags        string `json:"tags"`
}

type message struct {
    Item []item `json:"item"`
}

func prepare(d message) {
    apiurl := "http://XXX.XXX.XXX.XXX:1988/v1/push"
    jsonStr, _ := json.Marshal(d.Item)
    req, err := http.NewRequest("POST", apiurl, bytes.NewBuffer([]byte(jsonStr)))

    req.Header.Set("Content-Type", "application/json")
    client := &http.Client{}
    resp, err := client.Do(req)
    if err != nil {
        panic(err)
    }

    defer resp.Body.Close()
}

func pushToFalcon(l []Data) {
    for _, i := range l {
        var v item
        hostName, _ := os.Hostname()
        v.Endpoint = hostName
        v.Timestamp = time.Now().Unix()
        v.Step = i.Step
        v.CounterType = "GAUGE"
        v.Tags = "service=intranet_service"
        v.Metric = i.Metric
        v.Value = i.Value
        var o message
        o.Item = append(o.Item, v)
        prepare(o)
    }

}

func main() {
    parseConfig(path)
    makeRequest(c)
    pushToFalcon(l)
}

Config:

 { 
     "Params" : [
        { 
            "Metric" : "Test",
            "URL" :    "http://www.google.com",
            "RequestMethod" : "get",
            "Data" : "",
            "ExpectedString" : "",
            "Timeout" : 10,
            "Step" : 60
        },
        {
            "Metric" : "Test2",
            "URL" :    "https://www.baidu.com",
            "RequestMethod" : "post",
            "Data" : "datapostes",
            "ExpectedString" : "haha",
            "Timeout" : 10,
            "Step" : 60
    }
]

}

Dave C · Accepted Answer · 2019-07-31 13:21:25Z

Not a full review, but several issues pop up at a quick scan:

Avoid panic, it's meant for reporting a bug in the program (e.g. dividing by zero, failing a bounds check, dereferencing a nil pointer, etc) and not for user errors (e.g. a failure to complete an HTTP request).

Panics also produce stack traces by default. Users should (almost) never get shown stack traces but instead a simple error report (e.g. if in unix you do ls /non-existant-dir you don't get a stack trace of the internals of the ls command but a report that the directory /non-existant-dir does not exist).
defer should almost never be used within a loop. The deferred function doesn't run until the enclosing function returns/exits. Here you should be calling resp.Body.Close on each loop iteration before the next one.
Avoid using global variables. Here there is no reason that l and m cannot be local to makeRequest.
From the net/http documentation:

Clients and Transports are safe for concurrent use by multiple goroutines and for efficiency should only be created once and re-used.

You create a new http.Client on each loop iteration. At minimum you could create just one outside of the loop and then set client.Timeout in each iteration.
You repeat a lot of code in each branch of the if i.RequestMethod == "get". I'd instead conditionally create a nil or non-nil body and use a single req, err := http.NewRequest(i.RequestMethod, i.URL, body) and client.Do(req).
Never ignore errors like this:

jsonStr, _ := json.Marshal(d.Item)

Either handle/report/return the error as regular or if the surrounding function doesn't have (and shouldn't have) an error return and if the code should never fail (e.g. the code is constructing a value that should always be marshallable) then a panic for err != nil is reasonable (i.e. if someone should change the code later that json.Marshal suddenly fails they'll get a panic telling them where they messed up rather than strange failures later on).
Instead of bytes.NewBuffer([]byte(jsonStr)) do strings.NewReader(jsonStr)

In general, try and avoid or limit []byte <-> string conversions as they require allocations.
Config should probably be passed around as a pointer, *Config. Either that or it's Params field should be a slice of pointers ([]*struct). As-is if there the slice was 100 items each time it's passed 100×5 strings need to be copied (which only copies a pointer to the string, but it's still O(n)).

Stack Exchange Network

HTTP status monitoring

Code:

Config:

1 Answer 1

You must log in to answer this question.

Hot Network Questions

HTTP status monitoring

Code:

Config:

1 Answer 1

You must log in to answer this question.

Related

Hot Network Questions