engine: report metrics to opentelemetry.tilt.dev, and give an example of how to report tilt cli commands #3749
Conversation
|
Most of this should be pretty straightforward boilerplate:
The interesting bits are about the opencensus views. The state of the art for reporting metrics these days is pretty consistent across libraries I looked at (prometheus and statsd and opencensus and opentelemetry and openmetrics all look pretty much the same). They ask you to "pre-index" how your metrics are going to look, and send a heartbeat to the server with a summary of the metrics as they're collected. I think the idea is that the metrics are pre-indexed a bit, so that if a server request fails, we can re-send them without the risk of double-counting them |
| } | ||
| } | ||
|
|
||
| func (c *Controller) diff(rStore store.RStore) model.MetricsSettings { |
There was a problem hiding this comment.
should rename or pull the diff logic up into this method
| } | ||
|
|
||
| func (c *Controller) OnChange(ctx context.Context, rStore store.RStore) { | ||
| newMetrics := c.diff(rStore) |
There was a problem hiding this comment.
this name made me think it more directly contained to actual metrics / values - how about "settings" or "metricsSettings"?
| c.metrics = newMetrics | ||
| view.SetReportingPeriod(newMetrics.ReportingPeriod) | ||
|
|
||
| if !newMetrics.Enabled { |
There was a problem hiding this comment.
| if !newMetrics.Enabled { | |
| if oldMetrics.Enabled && !newMetrics.Enabled { |
nit: so that we don't log "shutting down" when changing settings other than "enabled", or in case someone adds some non-idempotent code to this block thinking it's the "shut down" block that fails when it wasn't running in the first place
| f.mc.OnChange(f.ctx, f.st) | ||
| assert.NotSame(t, remote, f.exp.remote) | ||
|
|
||
| // Verify that disablingthe metrics settings nulls out the remote exporter. |
There was a problem hiding this comment.
| // Verify that disablingthe metrics settings nulls out the remote exporter. | |
| // Verify that disabling the metrics settings nulls out the remote exporter. |
… of how to report tilt cli commands
Hello @landism, @maiamcc,
Please review the following commits I made in branch nicks/ch9214:
dd45c78 (2020-09-03 12:21:54 -0400)
engine: report metrics to opentelemetry.tilt.dev, and give an example of how to report tilt cli commands
Code review reminders, by giving a LGTM you attest that: