grafeas / voucher

🎟 Voucher creates attestations for Binary Authorization
Apache License 2.0
73 stars 20 forks source link

Datadog direct metrics #37

Closed thepwagner closed 3 years ago

thepwagner commented 3 years ago

Metrics collection is tightly coupled to statsd, which is a great decision in environments that can host an aggregator like dogstatsd or statsd_exporter nearby. Serverless environments might lack this facility.

This PR introduces metrics.DatadogClient as an alternative implementation of metrics.Client that submits metrics directly to Datadog via the metrics submission API. Moving from a local UDP aggregator to remote TCP is a notable but accepted performance regression in this iteration, and the reason this functionality is opt-in.

google-cla[bot] commented 3 years ago

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹī¸ Googlers: Go here for more info.

dani-santos-code commented 3 years ago

@googlebot I consent

thepwagner commented 3 years ago

@ooq this is another ~feature sized PR that we'd like to confirm before merging.

If this lands, are you open to us tagging and releasing as v2.6.0?

ooq commented 3 years ago

Tagging a new release makes sense! I'm for it. @thepwagner

thepwagner commented 3 years ago

My only comment is that can you update the documentation to reflect this change? Particularly on the configuration differences between the local statsd mode and direct datadog mode?

Good shout! Given the existing documentation doesn't mention statsd at all and I've introduced a bit of documentation debt by adding SOPS with only a sample configuration, I'll address documentation in another PR, also before v2.6.0.

edit: this became https://github.com/grafeas/voucher/pull/40

thepwagner commented 3 years ago

The implementation in https://github.com/grafeas/voucher/pull/37/commits/03478c200ebad1591eb4471190aa81061401cbb2 tested great in our staging environment 👍 .

But the lack of aggregation meant metrics went sideways when we promoted to production 👎 . I've added aggregation and async submission to do a better job of mocking dogstatsd in-process.