Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update opentelemetry dependencies #487

Merged
merged 3 commits into from Sep 29, 2022

Conversation

dashpole
Copy link
Contributor

@dashpole dashpole commented Sep 27, 2022

This includes updating to the newly rewritten metrics API + SDK.

It also updates to go 1.18, since the new API+SDK use generics

}
return e, nil
}

// InstallNewPipeline instantiates a NewExportPipeline and registers it globally.
func InstallNewPipeline(opts []Option, popts ...controller.Option) (*controller.Controller, error) {
Copy link
Contributor Author

@dashpole dashpole Sep 27, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I moved InstallNewPipeline and NewExportPipeline to cloudmonitoring.go, and deprecated it, since that is what upstream otel-go did with their equivalent functions.


monitoring "cloud.google.com/go/monitoring/apiv3/v2"
"golang.org/x/oauth2/google"
)

const (
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For now, remove interval-related options, since exporters don't configure the interval anymore.

me, err := newMetricExporter(&o)
// NewRawExporter creates a new Exporter thats implements metric.Exporter.
// Deprecated: Use New() instead.
func NewRawExporter(opts ...Option) (metric.Exporter, error) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NewRawExporter is renamed to New to match OTLP exporters' patterns.

var tss []*monitoringpb.TimeSeries
errs := []error{}
switch a := m.Data.(type) {
case metricdata.Gauge[int64]:
Copy link
Contributor Author

@dashpole dashpole Sep 27, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a lot of this code is repeated, but unfortunately it is hard to simplify because of the use of generics. We have a metric descriptor cache we want to use globally (so metricExporter can't be a generic type), but functions on types are not allowed to have generic arguments, so gaugeToTimeSeries (and others) can't be functions on metricExporter to be generic.
We can revisit it later to try and simplify if we want

})
if aggError != nil {
t.Errorf("%v", aggError)
out := me.recordToMpb(inputMetrics, inputAttributes, inputLibrary)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is now directly invoked with metricdata, rather than through the otel API path. MeterProvider does not provide a ForEach function anymore (instead has the exporter/reader interfaces), so we can't easily do that anymore. Same for the utf8 version of this test below.

@dashpole dashpole marked this pull request as ready for review September 27, 2022 14:27
exporter/metric/README.md Outdated Show resolved Hide resolved
exporter/metric/go.mod Outdated Show resolved Hide resolved
@dashpole dashpole merged commit af18bf5 into GoogleCloudPlatform:main Sep 29, 2022
@dashpole dashpole deleted the update_metrics branch September 29, 2022 15:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants