gcp/observability: Add compressed metrics to observability module and synchronize View data with exporter #6105
Conversation
gcp/observability/opencensus.go
Outdated
| @@ -35,6 +35,19 @@ import ( | |||
| var ( | |||
| // It's a variable instead of const to speed up testing | |||
| defaultMetricsReportingInterval = time.Second * 30 | |||
| o11yMetrics = []*view.View{ | |||
There was a problem hiding this comment.
I'd rather not put o11y into our code base, personally. It's an abbreviation that many people may not be familiar with.
Should this be defaultViews to be even more descriptive?
There was a problem hiding this comment.
Ah, ok. Kevin said this comment about wordage in my packet as well. Switched.
There was a problem hiding this comment.
I switched it, but I don't know if I like "default" semantically. These are the views defined by Yash in his document. I don't feel strongly though so went ahead and switched.
gcp/observability/opencensus.go
Outdated
| @@ -130,11 +140,15 @@ func stopOpenCensus() { | |||
| if exporter != nil { | |||
| internal.ClearGlobalDialOptions() | |||
| internal.ClearGlobalServerOptions() | |||
| // This Unregister call guarantees the data recorded gets sent to exporter, | |||
| // synchronising the view package and exporter. | |||
There was a problem hiding this comment.
As with the comment below, I assume this is also safe to do even if the views weren't already registered (i.e. metrics are disabled)?
There was a problem hiding this comment.
Luckily, yes. Like below, added a docstring.
There was a problem hiding this comment.
https://github.com/census-instrumentation/opencensus-go/blob/master/stats/view/worker_commands.go#L92 here is the check which makes it safe.
This PR adds compressed metrics to observability module, and also unregisters the views on module closure, synchronizing any recorded data with any exporters registered. It also fixes a naming bug in stats/opencensus. This was not caught in unit tests, as the unit tests in stats/opencensus verified keyed on name. However, the compression/uncompressed bytes measurements are technically correct. After this is merged I will need to get rid of go.mod import directive in gcp/observability and stats/opencensus as this fixes bug in stats/opencensus, which will create new commit hash in that package.
RELEASE NOTES: