Move Datadog to a plugin #261
Conversation
Gerald Rule: Copy Observability on Veneur and Unilog pull requestscc @stripe/observability |
| s.Statsd.Count("worker.checks_flushed_total", int64(len(checks)), nil, 1.0) | ||
|
|
||
| go s.metricSinks[0].FlushEventsChecks(span.Attach(ctx), events, checks) // we can do all of this separately | ||
| go s.flushTraces(span.Attach(ctx)) |
There was a problem hiding this comment.
feels like we could provide a method to cancel those goroutines somehow, passing in a context?
There was a problem hiding this comment.
You think it's ok to defer this work until it's no longer a hardcoded s.metricSinks[0]? My next move is to convert plugins into this interface and make this agnostic to backends. That feels like a good time to fix the cancellation.
|
I just opened #275 as a stop gap to fix the errors when you try to remove the datadog config.. I like this PR for the longer term fix. Feel free to close my PR if this PR is something we can start using in the next few weeks. |
|
I’d definitely prefer to merge this first. We likely would’ve already if I wasn’t OoO for a bit. |
cory-stripe
force-pushed
the
cory-more-dd-plugification
branch
2 times, most recently
from
96f64b5 to
d7d27ac
Compare
|
LGTM |
| metrics := c.Flush(10 * time.Second) | ||
| assert.Equal(t, 0.5, metrics[0].Value[0][1], "Metric value") | ||
| assert.Equal(t, float64(5), metrics[0].Value, "Metric value") |
There was a problem hiding this comment.
So to be clear: the reason this value has changed is that we're now letting the sampler provide the "true" value, and the per-interval rate calculation is being done within the plugin (ie, by Datadog)?
|
lgtm! |
cory-stripe
force-pushed
the
cory-more-dd-plugification
branch
from
5947001 to
6a1bc97
Compare
Summary
With the stage set, this moves Datadog to being a generic plugin… mostly.
Motivation
We want Datadog to be just another sink rather than the sink you must use. To that end we need to generalize metrics and make an interface for metric sinks. There are two major changes in this PR:
InterMetricrather thanDDMetricfrom samplers and as the struct that sinks receiveTest plan
Included tests! Will also verify with QA running, etc
Notes
Rollout/monitoring/revert plan
Will do some QA testing in a few weeks.