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

Remove interface for exporting ViewData #193

Open
songy23 opened this issue Aug 30, 2019 · 6 comments
Open

Remove interface for exporting ViewData #193

songy23 opened this issue Aug 30, 2019 · 6 comments

Comments

@songy23
Copy link
Contributor

songy23 commented Aug 30, 2019

A placeholder for rolling forward #188.

@bogdandrutu
Copy link
Contributor

Can we install the metrics exporter instead of the viewdata for the users, so we keep the public API?

At least I would like to avoid duplicate code to translate from viewdata to SD metrics and metrics data to Sd

@songy23
Copy link
Contributor Author

songy23 commented Aug 30, 2019

The major problem is for now lots of users still do

view.RegisterExporter(stackdriverExporter)

That requires Stackdriver exporter to implement the interface ExportView(viewData *Data) (https://godoc.org/go.opencensus.io/stats/view#Exporter). If we just remove this interface from Stackdriver exporter, the above code will break. We can translate View.Data to Metric under the hood though I don't see that simplifies things a lot (still need to add the mapping from View.Data to Metric).

@bogdandrutu
Copy link
Contributor

The mapping is already implemented in the opencensus we can re-use that.

@songy23
Copy link
Contributor Author

songy23 commented Aug 30, 2019

We have a mapping from the internal representation of ViewData (called viewInternal) to Metric: https://github.com/census-instrumentation/opencensus-go/blob/29aa3cabbf25be9f1c3c6d78cecfbe0c3e20cf5a/stats/view/view_to_metric.go#L128-L149. (This method and viewInternal are not exposed btw). However we don't currently have a method to directly convert ViewData to Metric that we can reuse here.

In addition I think the performance of ExportView will get worse if we do that, because it added an extra intermediate step (ViewData -> SDMetric vs. ViewData -> Metric -> SDMetric).

@yegle
Copy link
Contributor

yegle commented Sep 28, 2020

Oh my...

I've spent more than a week to investigate why we have an excessive export issue in our code base and I just realized we called both view.RegisterExporter(stackdriverExporter) and exporter.StartMetricsExporter() somehow.

If we can't remove the view.Exporter interface, can we at least add a warning log when both view.RegisterExporter and exporter.StartMetricsExporter are used?

@yegle
Copy link
Contributor

yegle commented Sep 29, 2020

And FWIW, the official guide on writing a metric exporter still uses view.Exporter interface: https://opencensus.io/exporters/custom-exporter/go/metrics/

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

No branches or pull requests

3 participants