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

flush interval reader when stopping metric reader #282

Merged
merged 2 commits into from
Feb 3, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ goimports:

.PHONY: staticcheck
staticcheck:
$(STATICCHECK) ./...
$(STATICCHECK) -checks "inherit,-SA1019" ./...
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added this to bypass SA1019 because the following failure after dependency upgrade:

internal/testpb/test.pb.go:42:9: xxx_messageInfo_FooRequest.Unmarshal is deprecated: Do not use; this method existed for intenal-use only.  (SA1019)
internal/testpb/test.pb.go:45:9: xxx_messageInfo_FooRequest.Marshal is deprecated: Do not use; this method existed for intenal-use only.  (SA1019)
internal/testpb/test.pb.go:48:2: xxx_messageInfo_FooRequest.Merge is deprecated: Do not use; this method existed for intenal-use only.  (SA1019)
internal/testpb/test.pb.go:51:9: xxx_messageInfo_FooRequest.Size is deprecated: Do not use; this method existed for intenal-use only.  (SA1019)
internal/testpb/test.pb.go:54:2: xxx_messageInfo_FooRequest.DiscardUnknown is deprecated: Do not use; this method existed for intenal-use only.  (SA1019)
internal/testpb/test.pb.go:57:32: proto.InternalMessageInfo is deprecated: Do not use; this type existed for intenal-use only.  (SA1019)
internal/testpb/test.pb.go:87:9: xxx_messageInfo_FooResponse.Unmarshal is deprecated: Do not use; this method existed for intenal-use only.  (SA1019)
internal/testpb/test.pb.go:90:9: xxx_messageInfo_FooResponse.Marshal is deprecated: Do not use; this method existed for intenal-use only.  (SA1019)
internal/testpb/test.pb.go:93:2: xxx_messageInfo_FooResponse.Merge is deprecated: Do not use; this method existed for intenal-use only.  (SA1019)
internal/testpb/test.pb.go:96:9: xxx_messageInfo_FooResponse.Size is deprecated: Do not use; this method existed for intenal-use only.  (SA1019)
internal/testpb/test.pb.go:99:2: xxx_messageInfo_FooResponse.DiscardUnknown is deprecated: Do not use; this method existed for intenal-use only.  (SA1019)
internal/testpb/test.pb.go:102:33: proto.InternalMessageInfo is deprecated: Do not use; this type existed for intenal-use only.  (SA1019)
internal/testpb/test.pb.go:105:2: proto.RegisterType is deprecated: Use protoregistry.GlobalTypes.RegisterMessage instead.  (SA1019)
internal/testpb/test.pb.go:106:2: proto.RegisterType is deprecated: Use protoregistry.GlobalTypes.RegisterMessage instead.  (SA1019)
internal/testpb/test.pb.go:109:15: proto.RegisterFile is deprecated: Use protoregistry.GlobalFiles.RegisterFile instead.  (SA1019)
metrics.go:27:2: package github.com/golang/protobuf/proto is deprecated: Use the "google.golang.org/protobuf/proto" package instead.  (SA1019)
metrics_batcher.go:26:2: package cloud.google.com/go/monitoring/apiv3 is deprecated: Please use cloud.google.com/go/monitoring/apiv3/v2.  (SA1019)
metrics_batcher_test.go:23:2: package cloud.google.com/go/monitoring/apiv3 is deprecated: Please use cloud.google.com/go/monitoring/apiv3/v2.  (SA1019)
metrics_proto_api_test.go:27:2: package github.com/golang/protobuf/proto is deprecated: Use the "google.golang.org/protobuf/proto" package instead.  (SA1019)
metrics_test.go:23:2: package github.com/golang/protobuf/proto is deprecated: Use the "google.golang.org/protobuf/proto" package instead.  (SA1019)
metrics_utils_test.go:24:2: package github.com/golang/protobuf/proto is deprecated: Use the "google.golang.org/protobuf/proto" package instead.  (SA1019)
stats.go:34:2: package cloud.google.com/go/monitoring/apiv3 is deprecated: Please use cloud.google.com/go/monitoring/apiv3/v2.  (SA1019)
stats_test.go:23:2: package cloud.google.com/go/monitoring/apiv3 is deprecated: Please use cloud.google.com/go/monitoring/apiv3/v2.  (SA1019)
trace.go:25:2: package github.com/golang/protobuf/proto is deprecated: Use the "google.golang.org/protobuf/proto" package instead.  (SA1019)
trace_proto_test.go:27:2: package github.com/golang/protobuf/proto is deprecated: Use the "google.golang.org/protobuf/proto" package instead.  (SA1019)

Please let me know if this should be an issue or be fix as part of the PR.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd prefer not disabling the deprecated notices. I might consider disabling specific files with //lint directives if the upgrade is problematic for some reason, but only if it isn't possible to use the new package easily.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think I have fixed the staticcheck errors. I had to make quite a bit of changes. I did test it by deploying it into Knative cluster just to be sure both metrics and traces are being exported correctly. Please let me know what you think. Thanks again!


.PHONY: install-tools
install-tools:
Expand Down
35 changes: 18 additions & 17 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -3,21 +3,22 @@ module contrib.go.opencensus.io/exporter/stackdriver
go 1.12

require (
cloud.google.com/go v0.45.1
github.com/aws/aws-sdk-go v1.23.20
github.com/census-instrumentation/opencensus-proto v0.2.1
github.com/golang/protobuf v1.3.2
github.com/google/go-cmp v0.3.1
github.com/jstemmer/go-junit-report v0.0.0-20190106144839-af01ea7f8024
go.opencensus.io v0.22.4
golang.org/x/lint v0.0.0-20190409202823-959b441ac422
golang.org/x/net v0.0.0-20190923162816-aa69164e4478
golang.org/x/oauth2 v0.0.0-20190604053449-0f29369cfe45
golang.org/x/sync v0.0.0-20190911185100-cd5d95a43a6e // indirect
golang.org/x/tools v0.0.0-20191010075000-0337d82405ff
google.golang.org/api v0.10.0
google.golang.org/appengine v1.6.2 // indirect
google.golang.org/genproto v0.0.0-20190911173649-1774047e7e51
google.golang.org/grpc v1.23.1
honnef.co/go/tools v0.0.1-2019.2.3
cloud.google.com/go v0.75.0
github.com/aws/aws-sdk-go v1.37.0
github.com/census-instrumentation/opencensus-proto v0.3.0
github.com/golang/protobuf v1.4.3
github.com/google/go-cmp v0.5.4
github.com/jstemmer/go-junit-report v0.9.1
go.opencensus.io v0.22.6
golang.org/x/lint v0.0.0-20201208152925-83fdc39ff7b5
golang.org/x/net v0.0.0-20210119194325-5f4716e94777
golang.org/x/oauth2 v0.0.0-20210126194326-f9ce19ea3013
golang.org/x/sys v0.0.0-20210124154548-22da62e12c0c // indirect
golang.org/x/text v0.3.5 // indirect
golang.org/x/tools v0.0.0-20210108195828-e2f9c7f1fc8e
google.golang.org/api v0.37.0
google.golang.org/genproto v0.0.0-20210126160654-44e461bb6506
google.golang.org/grpc v1.35.0
google.golang.org/protobuf v1.25.0
honnef.co/go/tools v0.0.1-2020.1.4
)
336 changes: 322 additions & 14 deletions go.sum

Large diffs are not rendered by default.

10 changes: 3 additions & 7 deletions metrics_proto_api_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,14 +23,12 @@ import (
"testing"

"github.com/google/go-cmp/cmp"
"github.com/google/go-cmp/cmp/cmpopts"

"github.com/golang/protobuf/proto"
"github.com/golang/protobuf/ptypes/empty"
"github.com/golang/protobuf/ptypes/timestamp"
"google.golang.org/api/option"
"google.golang.org/protobuf/testing/protocmp"

labelpb "google.golang.org/genproto/googleapis/api/label"
googlemetricpb "google.golang.org/genproto/googleapis/api/metric"
monitoredrespb "google.golang.org/genproto/googleapis/api/monitoredres"
monitoringpb "google.golang.org/genproto/googleapis/monitoring/v3"
Expand Down Expand Up @@ -593,7 +591,7 @@ func requireTimeSeriesRequestEqual(t *testing.T, got, want []*monitoringpb.Creat
}
for i, g := range got {
w := want[i]
diff = cmp.Diff(g, w, cmpopts.IgnoreFields(timestamp.Timestamp{}, "XXX_sizecache"))
diff = cmp.Diff(g, w, protocmp.Transform())
if diff != "" {
return diff, i
}
Expand All @@ -610,9 +608,7 @@ func requireMetricDescriptorRequestEqual(t *testing.T, got, want []*monitoringpb
}
for i, g := range got {
w := want[i]
diff = cmp.Diff(g, w,
cmpopts.IgnoreFields(labelpb.LabelDescriptor{}, "XXX_sizecache"),
cmpopts.IgnoreFields(googlemetricpb.MetricDescriptor{}, "XXX_sizecache"))
diff = cmp.Diff(g, w, protocmp.Transform())
if diff != "" {
return diff, i
}
Expand Down
3 changes: 2 additions & 1 deletion metrics_proto_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import (
monitoredrespb "google.golang.org/genproto/googleapis/api/monitoredres"
monitoringpb "google.golang.org/genproto/googleapis/monitoring/v3"
"google.golang.org/grpc"
"google.golang.org/protobuf/testing/protocmp"

metricspb "github.com/census-instrumentation/opencensus-proto/gen-go/metrics/v1"
"github.com/golang/protobuf/ptypes/wrappers"
Expand Down Expand Up @@ -995,7 +996,7 @@ func TestConvertSummaryMetrics(t *testing.T) {
se = new(statsExporter)
}
got := se.convertSummaryMetrics(tt.in)
if !cmp.Equal(got, tt.want) {
if !cmp.Equal(got, tt.want, protocmp.Transform()) {
t.Fatalf("conversion failed:\n got=%v\n want=%v\n", got, tt.want)
}
}
Expand Down
11 changes: 6 additions & 5 deletions metrics_utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import (
googlemetricpb "google.golang.org/genproto/googleapis/api/metric"
monitoredrespb "google.golang.org/genproto/googleapis/api/monitoredres"
monitoringpb "google.golang.org/genproto/googleapis/monitoring/v3"
"google.golang.org/protobuf/testing/protocmp"
)

func cmpResource(got, want *monitoredrespb.MonitoredResource) string {
Expand All @@ -47,21 +48,21 @@ func requireTimeSeriesRequestEqual(t *testing.T, got, want []*monitoringpb.Creat
}

func cmpTSReqs(got, want []*monitoringpb.CreateTimeSeriesRequest) string {
return cmp.Diff(got, want, cmpopts.IgnoreUnexported(monitoringpb.CreateTimeSeriesRequest{}), cmpopts.IgnoreTypes(googlemetricpb.MetricDescriptor_METRIC_KIND_UNSPECIFIED, googlemetricpb.MetricDescriptor_VALUE_TYPE_UNSPECIFIED))
return cmp.Diff(got, want, protocmp.Transform(), protocmp.IgnoreEnums(googlemetricpb.MetricDescriptor_METRIC_KIND_UNSPECIFIED, googlemetricpb.MetricDescriptor_VALUE_TYPE_UNSPECIFIED))
}

func cmpMD(got, want *googlemetricpb.MetricDescriptor) string {
return cmp.Diff(got, want, cmpopts.IgnoreUnexported(googlemetricpb.MetricDescriptor{}))
return cmp.Diff(got, want, protocmp.Transform())
}

func cmpMDReq(got, want *monitoringpb.CreateMetricDescriptorRequest) string {
return cmp.Diff(got, want, cmpopts.IgnoreUnexported(monitoringpb.CreateMetricDescriptorRequest{}))
return cmp.Diff(got, want, protocmp.Transform())
}

func cmpMDReqs(got, want []*monitoringpb.CreateMetricDescriptorRequest) string {
return cmp.Diff(got, want, cmpopts.IgnoreUnexported(monitoringpb.CreateMetricDescriptorRequest{}))
return cmp.Diff(got, want, protocmp.Transform())
}

func cmpPoint(got, want *monitoringpb.Point) string {
return cmp.Diff(got, want, cmpopts.IgnoreUnexported(monitoringpb.Point{}))
return cmp.Diff(got, want, protocmp.Transform())
}
3 changes: 2 additions & 1 deletion resource_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (
"go.opencensus.io/resource"
"go.opencensus.io/resource/resourcekeys"
monitoredrespb "google.golang.org/genproto/googleapis/api/monitoredres"
"google.golang.org/protobuf/testing/protocmp"
)

func TestDefaultMapResource(t *testing.T) {
Expand Down Expand Up @@ -476,7 +477,7 @@ func TestDefaultMapResource(t *testing.T) {
}

got := DefaultMapResource(c.input)
if diff := cmp.Diff(got, c.want); diff != "" {
if diff := cmp.Diff(got, c.want, protocmp.Transform()); diff != "" {
t.Errorf("Values differ -got +want: %s", diff)
}
})
Expand Down
9 changes: 4 additions & 5 deletions stats.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,6 @@ import (
"google.golang.org/api/support/bundler"
distributionpb "google.golang.org/genproto/googleapis/api/distribution"
labelpb "google.golang.org/genproto/googleapis/api/label"
"google.golang.org/genproto/googleapis/api/metric"
googlemetricpb "google.golang.org/genproto/googleapis/api/metric"
metricpb "google.golang.org/genproto/googleapis/api/metric"
monitoredrespb "google.golang.org/genproto/googleapis/api/monitoredres"
monitoringpb "google.golang.org/genproto/googleapis/monitoring/v3"
Expand Down Expand Up @@ -147,6 +145,7 @@ func (e *statsExporter) startMetricsReader() error {
func (e *statsExporter) stopMetricsReader() {
if e.ir != nil {
e.ir.Stop()
e.ir.Flush()
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 the actual change

}
}

Expand Down Expand Up @@ -433,7 +432,7 @@ func (e *statsExporter) combineTimeSeriesToCreateTimeSeriesRequest(ts []*monitor
// metricSignature creates a unique signature consisting of a
// metric's type and its lexicographically sorted label values
// See https://github.com/census-ecosystem/opencensus-go-exporter-stackdriver/issues/120
func metricSignature(metric *googlemetricpb.Metric) string {
func metricSignature(metric *metricpb.Metric) string {
labels := metric.GetLabels()
labelValues := make([]string, 0, len(labels))

Expand Down Expand Up @@ -593,7 +592,7 @@ func newLabelDescriptors(defaults map[string]labelValue, keys []tag.Key) []*labe
return labelDescriptors
}

func (e *statsExporter) createMetricDescriptor(ctx context.Context, md *metric.MetricDescriptor) error {
func (e *statsExporter) createMetricDescriptor(ctx context.Context, md *metricpb.MetricDescriptor) error {
ctx, cancel := newContextWithTimeout(ctx, e.o.Timeout)
defer cancel()
cmrdesc := &monitoringpb.CreateMetricDescriptorRequest{
Expand All @@ -604,7 +603,7 @@ func (e *statsExporter) createMetricDescriptor(ctx context.Context, md *metric.M
return err
}

var createMetricDescriptor = func(ctx context.Context, c *monitoring.MetricClient, mdr *monitoringpb.CreateMetricDescriptorRequest) (*metric.MetricDescriptor, error) {
var createMetricDescriptor = func(ctx context.Context, c *monitoring.MetricClient, mdr *monitoringpb.CreateMetricDescriptorRequest) (*metricpb.MetricDescriptor, error) {
return c.CreateMetricDescriptor(ctx, mdr)
}

Expand Down
18 changes: 9 additions & 9 deletions stats_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,11 @@ import (
"go.opencensus.io/tag"
"google.golang.org/api/option"
"google.golang.org/genproto/googleapis/api/distribution"
"google.golang.org/genproto/googleapis/api/metric"
metricpb "google.golang.org/genproto/googleapis/api/metric"
monitoredrespb "google.golang.org/genproto/googleapis/api/monitoredres"
monitoringpb "google.golang.org/genproto/googleapis/monitoring/v3"
"google.golang.org/grpc"
"google.golang.org/protobuf/testing/protocmp"
)

var authOptions = []option.ClientOption{option.WithGRPCConn(&grpc.ClientConn{})}
Expand Down Expand Up @@ -487,7 +487,7 @@ func TestExporter_makeReq(t *testing.T) {
if len(tt.want) == 0 {
return
}
if diff := cmp.Diff(resps, tt.want); diff != "" {
if diff := cmp.Diff(resps, tt.want, protocmp.Transform()); diff != "" {
t.Errorf("Values differ -got +want: %s", diff)
}
})
Expand Down Expand Up @@ -622,7 +622,7 @@ func TestExporter_createMetricDescriptorFromView(t *testing.T) {
}

var createCalls int
createMetricDescriptor = func(ctx context.Context, c *monitoring.MetricClient, mdr *monitoringpb.CreateMetricDescriptorRequest) (*metric.MetricDescriptor, error) {
createMetricDescriptor = func(ctx context.Context, c *monitoring.MetricClient, mdr *monitoringpb.CreateMetricDescriptorRequest) (*metricpb.MetricDescriptor, error) {
createCalls++
if got, want := mdr.MetricDescriptor.Name, "projects/test_project/metricDescriptors/custom.googleapis.com/opencensus/test_view_sum"; got != want {
t.Errorf("MetricDescriptor.Name = %q; want %q", got, want)
Expand All @@ -645,7 +645,7 @@ func TestExporter_createMetricDescriptorFromView(t *testing.T) {
if got, want := mdr.MetricDescriptor.Unit, stats.UnitMilliseconds; got != want {
t.Errorf("MetricDescriptor.Unit = %q; want %q", got, want)
}
return &metric.MetricDescriptor{
return &metricpb.MetricDescriptor{
DisplayName: "OpenCensus/test_view_sum",
Description: "view_description",
Unit: stats.UnitMilliseconds,
Expand Down Expand Up @@ -699,7 +699,7 @@ func TestExporter_createMetricDescriptorFromView_CountAggregation(t *testing.T)
o: Options{ProjectID: "test_project"},
}

createMetricDescriptor = func(ctx context.Context, c *monitoring.MetricClient, mdr *monitoringpb.CreateMetricDescriptorRequest) (*metric.MetricDescriptor, error) {
createMetricDescriptor = func(ctx context.Context, c *monitoring.MetricClient, mdr *monitoringpb.CreateMetricDescriptorRequest) (*metricpb.MetricDescriptor, error) {
if got, want := mdr.MetricDescriptor.Name, "projects/test_project/metricDescriptors/custom.googleapis.com/opencensus/test_view_count"; got != want {
t.Errorf("MetricDescriptor.Name = %q; want %q", got, want)
}
Expand All @@ -721,7 +721,7 @@ func TestExporter_createMetricDescriptorFromView_CountAggregation(t *testing.T)
if got, want := mdr.MetricDescriptor.Unit, stats.UnitDimensionless; got != want {
t.Errorf("MetricDescriptor.Unit = %q; want %q", got, want)
}
return &metric.MetricDescriptor{
return &metricpb.MetricDescriptor{
DisplayName: "OpenCensus/test_view_sum",
Description: "view_description",
Unit: stats.UnitDimensionless,
Expand Down Expand Up @@ -1079,7 +1079,7 @@ func TestExporter_makeReq_withCustomMonitoredResource(t *testing.T) {
if len(tt.want) == 0 {
return
}
if diff := cmp.Diff(resps, tt.want); diff != "" {
if diff := cmp.Diff(resps, tt.want, protocmp.Transform()); diff != "" {
t.Errorf("Requests differ, -got +want: %s", diff)
}
})
Expand All @@ -1096,14 +1096,14 @@ func TestExporter_customContext(t *testing.T) {
}()

var timedOut = 0
createMetricDescriptor = func(ctx context.Context, c *monitoring.MetricClient, mdr *monitoringpb.CreateMetricDescriptorRequest) (*metric.MetricDescriptor, error) {
createMetricDescriptor = func(ctx context.Context, c *monitoring.MetricClient, mdr *monitoringpb.CreateMetricDescriptorRequest) (*metricpb.MetricDescriptor, error) {
select {
case <-time.After(1 * time.Second):
fmt.Println("createMetricDescriptor did not time out")
case <-ctx.Done():
timedOut++
}
return &metric.MetricDescriptor{}, nil
return &metricpb.MetricDescriptor{}, nil
}
createTimeSeries = func(ctx context.Context, c *monitoring.MetricClient, ts *monitoringpb.CreateTimeSeriesRequest) error {
select {
Expand Down