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

Metric types in point.go don't align with protobuf #1223

Open michaelli321 opened 4 years ago

michaelli321 commented 4 years ago

Please answer these questions before submitting a bug report.

What version of OpenCensus are you using?

0.22.4

What version of Go are you using?

1.14.2

What did you do?

If possible, provide a recipe for reproducing the error. 1) I created a custom exporter 2) I created a library to convert metricdata.Metric to its protobuf format 3) observe the exported protobuf metric type

What did you expect to see?

I expected to see the metric type returned from https://github.com/census-instrumentation/opencensus-go/blob/master/stats/view/view_to_metric.go#L39-L76

What did you see instead?

I saw an incorrect metric type

Additional context

Add any other context about the problem here.

The metric types defined in the Metrics API are as follows. https://github.com/census-instrumentation/opencensus-go/blob/master/metric/metricdata/point.go#L185-L193. However, in the OpenCensus metrics protobuf they are defined as https://github.com/census-instrumentation/opencensus-proto/blob/master/gen-go/metrics/v1/metrics.pb.go#L61-L89. Since the protobuf start on 0 as unspecified type it would make sense to start the enum metric types in point.go at 1 as well since metrics will panic during conversion from view to metric if it receives an unspecified type https://github.com/census-instrumentation/opencensus-go/blob/master/stats/view/view_to_metric.go#L39-L76

james-bebbington commented 4 years ago

While this change would be nice for simplicity, we are generally limiting any changes to OpenCensus to critical bug fixes given that OpenCensus is being wound down in favour of OpenTelemetry. I don't think anything is actually broken here. The OC agent exporter correctly maps SDK types to proto types afaict.