Skip to content
This repository has been archived by the owner on Jul 31, 2023. It is now read-only.

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

Open
michaelli321 opened this issue Jul 23, 2020 · 1 comment
Open

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

michaelli321 opened this issue Jul 23, 2020 · 1 comment
Labels

Comments

@michaelli321
Copy link

michaelli321 commented Jul 23, 2020

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
Copy link
Collaborator

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.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

2 participants