Make route metric names configurable

[maier]

Had request for the ability to alter the route metric names coming out of the clusters with nomad/consul testing. e.g.

package main

import (
    "fmt"
    "net/url"
    "strings"
)

func clean(s string) string {
    if s == "" {
        return "_"
    }
    s = strings.Replace(s, ".", "_", -1)
    s = strings.Replace(s, ":", "_", -1)
    return strings.ToLower(s)
}

func TargetName(template string, service string, host string, path string, targetURL *url.URL) string {
    name := template
    name = strings.Replace(name, "$service$", clean(service), -1)
    name = strings.Replace(name, "$host$", clean(host), -1)
    name = strings.Replace(name, "$path$", clean(path), -1)
    name = strings.Replace(name, "$target$", clean(targetURL.Host), -1)

    return name
}

func main() {

    // given a route rule of: route add hashiapp hashiapp.com/ http://10.1.2.3:12345/
    service := "hashiapp"
    host := "hashiapp.com"
    path := "/"
    target, err := url.Parse("http://10.1.2.3:12345")
    if err != nil {
        panic(err)
    }

    /* default template
     *
     * metrics.routemetricnametemplate = $service$.$host$.$path$.$target$
     *
     */

    template := "$service$.$host$.$path$.$target$"
    fmt.Printf("Default route metric name: %s\n", TargetName(template, service, host, path, target))

    template = "$service$.$host$.$path$"
    fmt.Printf("Custom route metric name : %s\n", TargetName(template, service, host, path, target))

    template = "$service$.$host$.$path$.latency_ns"
    fmt.Printf("Custom route metric name : %s\n", TargetName(template, service, host, path, target))

}

⁖ go run test.go
Default route metric name: hashiapp.hashiapp_com./.10_1_2_3_12345
Custom route metric name : hashiapp.hashiapp_com./
Custom route metric name : hashiapp.hashiapp_com./.latency_ns

[magiconair]

That sounds like a good idea. However, I would like to use a different templating mechanism also because I need something similar for the access log configuration from #80. I think we should just use text/template and be done with it.

Then this could look like

{{clean .Service}}.{{clean .Host}}.{{clean .Path}}.{{clean targetURL.Host}}

[aaronhurt]

This could potentially solve another issue for us that we are currently working around in text/template language using consul-template and HAProxy. We have some legacy services that want to be exposed like /app.something.foo/ and consul doesn't support dots in service names so we register them with dashes and inside our template do this ...

{{if in .Tags "proxy-dash2dots"}}{{.Name | replaceAll "-" "."}}{{else}}{{.Name}}{{end}}

Having support for templates and template options in fabio would be a great feature.

[magiconair]

@leprechau Where would this be used?

Lets try not to morph this PR into a multi-issue discussion. If there is another use-case for templates then lets open a separate ticket and collect thoughts there. Could you do that @leprechau ?

[maier]

Since template.Execute() can return an error, what are your thoughts on what TargetName should return? An error or have a backup default action if the template execution fails? Or, are you thinking about putting together a more generic set of template handling helper functions?

package main

import (
    "bytes"
    "fmt"
    "log"
    "net/url"
    "strings"
    "text/template"
)

var routeMetricNameTemplate *template.Template

type routeMetricNameAttributes struct {
    Service, Host, Path string
    TargetURL           *url.URL
}

func clean(s string) string {
    if s == "" {
        return "_"
    }
    s = strings.Replace(s, ".", "_", -1)
    s = strings.Replace(s, ":", "_", -1)
    return strings.ToLower(s)
}

// TargetName blah
func TargetName(service string, host string, path string, targetURL *url.URL) (string, error) {
    var metricName bytes.Buffer
    data := &routeMetricNameAttributes{service, host, path, targetURL}
    err := routeMetricNameTemplate.Execute(&metricName, data)
    if err != nil {
        return "", err
    }

    return metricName.String(), nil
}

func main() {
    funcMap := template.FuncMap{
        "clean": clean,
    }
    nametemplate := "{{clean .Service}}.{{clean .Host}}.{{clean .Path}}.{{clean .TargetURL.Host}}"
    routeMetricNameTemplate = template.Must(template.New("routeMetricName").Funcs(funcMap).Parse(nametemplate))
    service := "hashiapp"
    host := "hashiapp.com"
    path := "/"
    target, err := url.Parse("http://10.1.2.3:12345")
    if err != nil {
        log.Fatalln(err)
    }

    metricName, err := TargetName(service, host, path, target)
    if err != nil {
        log.Fatalln(err)
    }

    fmt.Println(metricName)

    nametemplate = "{{clean .Service}}.{{clean .Host}}.{{clean .Path}}.latency_ns"
    routeMetricNameTemplate = template.Must(template.New("routeMetricNameCustom").Funcs(funcMap).Parse(nametemplate))

    metricName, err = TargetName(service, host, path, target)
    if err != nil {
        log.Fatalln(err)
    }

    fmt.Println(metricName)

}

[magiconair]

An empty string since I can check the templates during startup.

[magiconair]

(first think, then type): yes, Execute can return an error as well. It should still return an empty string and an error which I need to log somewhere. Otherwise, your metrics get polluted.

[maier]

cool, yeah, agree on the pollution, so TargetName(...) (name string, error err).

[aaronhurt]

@magiconair Yes, sorry I'll get my thoughts together and submit another PR for that use case. It really just occurred to me when you mentioned text/template in this discussion.

[magiconair]

@leprechau no need to apologize. See what you can come up with.

[maier]

Updated to use text/template. Hoping it might be close enough to make it easier for you to shuffle it around the way you'd prefer. The main things I wasn't sure about are handling of the return from TargetName in route.go and where I put the template verification.

[magiconair]

I think this looks good enough. I'll probably add a log line for the error you've skipped in route.go since my guess is that people will use this for strange things. It should at least log. And we need to come up with a better name than routemetricname :)

Thanks for the work. Really appreciate it.

[magiconair]

@maier I've taken your changed some smaller parts since my tests were failing. Also changed routemetricname to just names. Pls have a look whether there is a glaring omission otherwise I'll merge it tomorrow.

[maier]

Yeah, that's good, cleaner.

[magiconair]

Merged to master.