Issue #151: Add support for Circonus metrics

[maier]

Adding support for sending metrics to Circonus.

[magiconair]

Don't use double quotes unless you want to send them. Also, if this is the default then you need to set this in config/default.go. Same for metrics.circonus.apiurl

[maier]

Added a default for CirconusAPIApp to defaults. I reworded the description of the API URL parameter, I'd prefer to keep that default localized in the library so that if it changes there isn't a waterfall of PRs to update.

[magiconair]

Could you also update the comment for the 2nd commit to Issue #147: Add support to send metrics to Circonus. I think if you force push the branch to your repo git push -f then this won't create additional commits in the PR. GH should just replace the current PR.

[magiconair]

Hmm, if you set a default in config/default.go then this needs to be reflected here. Why did you remove it?

should be metrics.circonus.apiapp = fabio

[magiconair]

I thinks this looks ok enough. I'll merge this manually since the config/load_test.go needs updating and there are one or two typos. Thanks for your patience with the code review.

[magiconair]

@maier you seem to have forgotten to add metrics.circonus.apikey to the fabio.properties. Could you provide me with the snippet and then I'll add it. Thx

[magiconair]

@maier or is this ok?

# metrics.circonus.apikey configures the API key to use when
# submitting metrics to Circonus. See: https://login.circonus.com/user/tokens
# This is optional when ${metrics.target} is set to "circonus".
#
# The default is
#
# metrics.circonus.apikey =

[maier]

# metrics.circonus.apikey configures the API token key to use when
# submitting metrics to Circonus. See: https://login.circonus.com/user/tokens
# This is required when ${metrics.target} is set to "circonus".
#
# The default is
#
# metrics.circonus.apikey =

[maier]

that's the one setting which is required, the rest are optional.

[magiconair]

Got it. I'm still curious what TestAll is testing and how you determine that the test has passed. Could you explain, please?

[maier]

testAll actually does the job, it connects to the api and performs all of the steps. verification is two-fold, if the code fails and then checking manually in the UI for the metric coming through. the code will fail in finding/creating the check, then i go over to the ui and verify that a) the fake metric was created and b) a submission came through for it. it's gated by the env vars so that it doesn't thrash the checks.

[magiconair]

ok, that's what I thought. This is an integration test which requires manual inspection. I'll leave it in for now but need to think about whether I would want to mock the API or check this another way. I don't want to test the driver.

As a side note: I would also like to merge the change from #125 which adds a counter to the metrics registry.

 // Counter defines a metric for counting events.
 type Counter interface {
     // Inc increases the counter value by 'n'.
     Inc(n int64)
 }

How would I add this to the circonus registry?

[maier]

type cgmCounter struct {
    metrics *cgm.CirconusMetrics
    name    string
}

func (m *cgmRegistry) GetCounter(name string) Counter {
    metricName := fmt.Sprintf("%s`%s", m.prefix, name)
    return &cgmCounter{m.metrics, metricName}
}

func (c cgmCounter) Inc(n int64) {
    c.metrics.IncrementByValue(c.name, uint64(n))
}

[magiconair]

OK, I just noticed that the registry methods return pointers (&cgmTimer) but the methods on the structs have value receivers. How does this even work? I guess in the current implementation it doesn't make a difference whether it is either/or but I think it should be consistent. I'll change the timer/counter methods to have pointer receivers as well (e.g. func (t *cgmTimer) UpdateSince(...)) unless you know a good reason why I shouldn't.

[magiconair]

Did a manual merge of this PR to master.