Conversation
|
|
|
|
||
| // New returns a new datadog reporter | ||
| func New(apiKey string, opts ...Option) (*Reporter, error) { | ||
| endpoint := "https://app.datadoghq.com/api/v1/series?api_key=" + apiKey |
There was a problem hiding this comment.
Want to make this URL a const at the top of this file?
Also might want use URL encoding to encode anything in the api key that needs encoding, e.g.
var params url.Values
params.Add("api_key", apiKey)
endpoint := fmt.Sprintf(endpointURL + "?" + params.Encode())| m := poolMetric.Get().(*metric) | ||
| m.Name = name | ||
| m.Type = metricType | ||
| m.Points = [][]float64{ |
There was a problem hiding this comment.
If you're pooling these objects you may want to reuse the array on it. i.e.
if len(m.Points) == 1 && len(m.Points[0]) == 2 {
m.Points[0][0] = float64(time.Now().Unix())
m.Points[0][1] = value
} else {
m.Points = /* construct */
}| }, | ||
| } | ||
| for k, v := range tags { | ||
| m.Tags = append(m.Tags, k+":"+v) |
There was a problem hiding this comment.
If you're pooling the metric you'll have to reset the m.Tags slice at first.
e.g.
m.Tags = m.Tags[:0] // make sure slice has length zero (but retains capacity)
for k, v := range tags {
m.Tags = append(m.Tags, k+":"+v)
}| r.metrics[r.offset] = m | ||
| r.offset++ | ||
|
|
||
| if r.offset == r.bufSize { |
There was a problem hiding this comment.
Do you need offset? Can't you simply do a len(r.metrics)? It's better to have one source of truth if possible.
| } | ||
|
|
||
| // flush is a thread-unsafe flush. assumes caller has already obtained lock | ||
| func (r *Reporter) flush() { |
There was a problem hiding this comment.
Perhaps rename the method flushWithLock(...) to signal that it relies on the lock, makes the code a little easier to read.
| if err := r.post(data); err != nil { | ||
| fmt.Fprintln(os.Stderr, "failed to connect to host") | ||
| fmt.Fprintln(r.output, "failed to connect to host") | ||
| time.Sleep(time.Second * 15) |
There was a problem hiding this comment.
Perhaps make this backoff configurable?
| for attempts := 0; attempts < 3; attempts++ { | ||
| if err := r.post(data); err != nil { | ||
| fmt.Fprintln(os.Stderr, "failed to connect to host") | ||
| fmt.Fprintln(r.output, "failed to connect to host") |
There was a problem hiding this comment.
Perhaps make where this notification goes configurable? Users may have their own logging library they wish to use, etc.
| m.Points = nil | ||
| m.Type = "" | ||
| m.Host = "" | ||
| m.Tags = nil |
There was a problem hiding this comment.
If you want to pool this, probably better to m.Tags = m.Tags[:0]
|
|
||
| func (m *metric) Reset() { | ||
| m.Name = "" | ||
| m.Points = nil |
There was a problem hiding this comment.
If you want to pool this you probably don't want to reset this slice to nil.
| const ( | ||
| // DefaultBufferSize contains the number of metrics the datadog reporter | ||
| // will capture before forcing a flush | ||
| DefaultBufferSize = 512 |
There was a problem hiding this comment.
Perhaps a larger default is ideal? 4096 or similar, which would generate probably 100kb roughly of payload I guess.
|
Hey savaki, thanks for the contribution. After reviewing the change you may want to also look into flushing based on some timer in addition to your size based submission. Otherwise the last batch of metrics is unlikely to ever be submitted. Could you make your reporter implement |
|
Is there any update? |
|
Any update on this? |
|
However unlikely it's, I'm just going to ask anyway... Any update on this? |
6 participants
initial support for datadog service. to test it with a live account, run