search

census-instrumentation / opencensus-go

A stats collection and distributed tracing framework
http://opencensus.io
Apache License 2.0
2.05k stars 326 forks source link

Allow creating additional View universes. #1196

Closed evankanderson closed 4 years ago

evankanderson commented 4 years ago

Fixes #1195

I wanted to be able to export statistics for multiple Resources, but I discovered that all the stats aggregation was bundled into a single "universe" in the stats/view defaultWorker. This PR moves all the singleton state into a Worker interface, and maintains the simple singleton interface for the common case, but allows creating additional Workers in complex cases.

evankanderson commented 4 years ago

CC @anniefu

evankanderson commented 4 years ago

I added an additional interface for extracting the stats.Option settings for passing to the view.Record() method. This seemed the simplest way to extract the logic in stats.RecordWithOptions, which currently consumes the context and performs some short-circuiting logic.

evankanderson commented 4 years ago

(Let me know if you want me to squash the commits or if there are other changes I need to make.)

rghetia commented 4 years ago

(Let me know if you want me to squash the commits or if there are other changes I need to make.)

@evankanderson Leave it as is. When I merge, I will squash it.

@songy23 would have time to review this? I would like to have one more reviewer.

evankanderson commented 4 years ago

I have code like that, too.

Worse than that, I have code which auto-detects which type of resource it is, and has a fixed schema for each Resource of which tags should be extracted and removed from the tag map.

I suppose that I could still go through tags for aggregation and add a "RegisterType" method into my exports which would allow dynamically registering the schemas, but misdirecting all the aggregation through multiple layers of Tags and view aggregations feels wrong to me.

rghetia commented 4 years ago

The high level idea makes sense to me. The only alternative I can think of is to hard-code resources as metric labels, export to Collector, then have some sort of metric processor on Collector to reconstruct resources and strip those labels. For example, in addition to normal metric labels, add labels ["container_name": "some container", "pod_name": "pod"], then on the Collector remove those labels and attach a new k8s resource with these values.

We have internal use cases like this, but I'm don't think that's ideal since it requires lots of hard code and also mixes resources and metrics in a confusing way.

I agree that it would be confusing. Even though OC will be deprecated once Otel is released, this PR probably makes more sense than the alternate solution. It is also inline with concept of Meter in Otel. Do you agree?