Skip to content

Commit

Permalink
enable testing the SDK exporter using collector integration tests (#503)
Browse files Browse the repository at this point in the history
  • Loading branch information
dashpole authored Oct 10, 2022
1 parent 41e85d4 commit ca90414
Show file tree
Hide file tree
Showing 7 changed files with 54 additions and 25 deletions.
10 changes: 7 additions & 3 deletions exporter/collector/integrationtest/metrics_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -133,14 +133,16 @@ func TestSDKMetrics(t *testing.T) {
require.NoError(t, err)
go testServer.Serve()
defer testServer.Shutdown()
testServerExporter, err := metric.New(
opts := append([]metric.Option{
metric.WithProjectID("fakeprojectid"),
metric.WithMonitoringClientOptions(
apioption.WithEndpoint(testServer.Endpoint),
apioption.WithoutAuthentication(),
apioption.WithGRPCDialOption(grpc.WithTransportCredentials(insecure.NewCredentials())),
),
)},
test.MetricSDKExporterOptions...,
)
testServerExporter, err := metric.New(opts...)
require.NoError(t, err)

for _, m := range testcases.ConvertResourceMetrics(metrics) {
Expand Down Expand Up @@ -171,7 +173,9 @@ func TestSDKMetrics(t *testing.T) {
return expectFixture.CreateServiceTimeSeriesRequests[i].Name < expectFixture.CreateServiceTimeSeriesRequests[j].Name
})

require.NoError(t, err)
// Do not test self-observability metrics with SDK exporters
expectFixture.SelfObservabilityMetrics = nil

fixture := &protos.MetricExpectFixture{
CreateTimeSeriesRequests: testServer.CreateTimeSeriesRequests(),
CreateMetricDescriptorRequests: testServer.CreateMetricDescriptorRequests(),
Expand Down
5 changes: 4 additions & 1 deletion exporter/collector/integrationtest/testcases/testcase.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import (
monitoringpb "google.golang.org/genproto/googleapis/monitoring/v3"

"github.com/GoogleCloudPlatform/opentelemetry-operations-go/exporter/collector/internal/integrationtest/protos"
"github.com/GoogleCloudPlatform/opentelemetry-operations-go/exporter/metric"

"github.com/stretchr/testify/require"
"go.opentelemetry.io/collector/pdata/pcommon"
Expand All @@ -53,7 +54,7 @@ var (
const SecondProjectEnv = "SECOND_PROJECT_ID"

type TestCase struct {
// Configure will be called to modify the default configuration for this test case. Optional.
// ConfigureCollector will be called to modify the default configuration for this test case. Optional.
ConfigureCollector func(cfg *collector.Config)
// Name of the test case
Name string
Expand All @@ -63,6 +64,8 @@ type TestCase struct {
// ExpectFixturePath is the path to the JSON encoded MetricExpectFixture
// (see fixtures.proto) that contains request messages the exporter is expected to send.
ExpectFixturePath string
// When testing the SDK metrics exporter (not collector), this is the options to use. Optional.
MetricSDKExporterOptions []metric.Option
// Skip, if true, skips this test case
Skip bool
// SkipForSDK, if true, skips this test case when testing the SDK
Expand Down
49 changes: 36 additions & 13 deletions exporter/collector/integrationtest/testcases/testcases_metrics.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,16 +15,21 @@
package testcases

import (
"strings"

"go.opentelemetry.io/otel/attribute"
"go.opentelemetry.io/otel/sdk/metric/metricdata"

"github.com/GoogleCloudPlatform/opentelemetry-operations-go/exporter/collector"
"github.com/GoogleCloudPlatform/opentelemetry-operations-go/exporter/collector/googlemanagedprometheus"
"github.com/GoogleCloudPlatform/opentelemetry-operations-go/exporter/metric"
)

var MetricsTestCases = []TestCase{
{
Name: "Basic Counter",
OTLPInputFixturePath: "testdata/fixtures/metrics/basic_counter_metrics.json",
ExpectFixturePath: "testdata/fixtures/metrics/basic_counter_metrics_expect.json",
SkipForSDK: true,
},
{
Name: "Basic Counter with not found return code",
Expand All @@ -33,14 +38,13 @@ var MetricsTestCases = []TestCase{
ConfigureCollector: func(cfg *collector.Config) {
cfg.ProjectID = "notfoundproject"
},
ExpectErr: true,
SkipForSDK: true,
MetricSDKExporterOptions: []metric.Option{metric.WithProjectID("notfoundproject")},
ExpectErr: true,
},
{
Name: "Basic Prometheus metrics",
OTLPInputFixturePath: "testdata/fixtures/metrics/basic_prometheus_metrics.json",
ExpectFixturePath: "testdata/fixtures/metrics/basic_prometheus_metrics_expect.json",
SkipForSDK: true,
},
{
Name: "Modified prefix unknown domain",
Expand All @@ -49,7 +53,9 @@ var MetricsTestCases = []TestCase{
ConfigureCollector: func(cfg *collector.Config) {
cfg.MetricConfig.Prefix = "custom.googleapis.com/foobar.org"
},
SkipForSDK: true,
MetricSDKExporterOptions: []metric.Option{metric.WithMetricDescriptorTypeFormatter(func(m metricdata.Metrics) string {
return "custom.googleapis.com/foobar.org/" + m.Name
})},
},
{
Name: "Modified prefix workload.googleapis.com",
Expand All @@ -58,31 +64,30 @@ var MetricsTestCases = []TestCase{
ConfigureCollector: func(cfg *collector.Config) {
cfg.MetricConfig.Prefix = "workload.googleapis.com"
},
SkipForSDK: true,
},
{
Name: "Delta Counter",
OTLPInputFixturePath: "testdata/fixtures/metrics/delta_counter_metrics.json",
ExpectFixturePath: "testdata/fixtures/metrics/delta_counter_metrics_expect.json",
SkipForSDK: true,
},
{
Name: "Non-monotonic Counter",
OTLPInputFixturePath: "testdata/fixtures/metrics/nonmonotonic_counter_metrics.json",
ExpectFixturePath: "testdata/fixtures/metrics/nonmonotonic_counter_metrics_expect.json",
SkipForSDK: true,
},
{
Name: "Summary",
OTLPInputFixturePath: "testdata/fixtures/metrics/summary_metrics.json",
ExpectFixturePath: "testdata/fixtures/metrics/summary_metrics_expect.json",
SkipForSDK: true,
// Summary metrics are not possible with the SDK.
SkipForSDK: true,
},
{
Name: "Batching",
OTLPInputFixturePath: "testdata/fixtures/metrics/batching_metrics.json",
ExpectFixturePath: "testdata/fixtures/metrics/batching_metrics_expect.json",
SkipForSDK: true,
// Summary metrics are not possible with the SDK.
SkipForSDK: true,
},
{
Name: "Ops Agent Self-Reported metrics",
Expand All @@ -93,6 +98,7 @@ var MetricsTestCases = []TestCase{
cfg.MetricConfig.SkipCreateMetricDescriptor = true
cfg.MetricConfig.ServiceResourceLabels = false
},
// We don't support disabling metric descriptor creation for the SDK exporter
SkipForSDK: true,
},
{
Expand All @@ -103,6 +109,7 @@ var MetricsTestCases = []TestCase{
// Metric descriptors should not be created under agent.googleapis.com
cfg.MetricConfig.SkipCreateMetricDescriptor = true
},
// We don't support disabling metric descriptor creation for the SDK exporter
SkipForSDK: true,
},
{
Expand All @@ -114,6 +121,7 @@ var MetricsTestCases = []TestCase{
cfg.MetricConfig.SkipCreateMetricDescriptor = true
cfg.MetricConfig.ServiceResourceLabels = false
},
// We don't support disabling metric descriptor creation for the SDK exporter
SkipForSDK: true,
},
{
Expand All @@ -129,6 +137,7 @@ var MetricsTestCases = []TestCase{
cfg.MetricConfig.ServiceResourceLabels = false
cfg.MetricConfig.EnableSumOfSquaredDeviation = true
},
// prometheus_target is not supported by the SDK
SkipForSDK: true,
},
{
Expand All @@ -138,6 +147,7 @@ var MetricsTestCases = []TestCase{
ConfigureCollector: func(cfg *collector.Config) {
cfg.MetricConfig.CreateServiceTimeSeries = true
},
// SDK exporter does not support CreateServiceTimeSeries
SkipForSDK: true,
},
{
Expand All @@ -148,13 +158,16 @@ var MetricsTestCases = []TestCase{
cfg.MetricConfig.CreateServiceTimeSeries = true
cfg.MetricConfig.ServiceResourceLabels = false
},
// SDK exporter does not support CreateServiceTimeSeries
SkipForSDK: true,
},
{
Name: "Exponential Histogram",
OTLPInputFixturePath: "testdata/fixtures/metrics/exponential_histogram_metrics.json",
ExpectFixturePath: "testdata/fixtures/metrics/exponential_histogram_metrics_expect.json",
SkipForSDK: true,
// Blocked on upstream support for exponential histograms:
// https://github.com/open-telemetry/opentelemetry-go/issues/2966
SkipForSDK: true,
},
{
Name: "CreateServiceTimeSeries",
Expand All @@ -163,6 +176,7 @@ var MetricsTestCases = []TestCase{
ConfigureCollector: func(cfg *collector.Config) {
cfg.MetricConfig.CreateServiceTimeSeries = true
},
// SDK exporter does not support CreateServiceTimeSeries
SkipForSDK: true,
},
{
Expand All @@ -174,13 +188,22 @@ var MetricsTestCases = []TestCase{
{Prefix: "telemetry.sdk."},
}
},
SkipForSDK: true,
MetricSDKExporterOptions: []metric.Option{
metric.WithFilteredResourceAttributes(func(kv attribute.KeyValue) bool {
// Include the default set of promoted resource attributes
if metric.DefaultResourceAttributesFilter(kv) {
return true
}
return strings.HasPrefix(string(kv.Key), "telemetry.sdk.")
}),
},
},
{
Name: "Multi-project metrics",
OTLPInputFixturePath: "testdata/fixtures/metrics/metrics_multi_project.json",
ExpectFixturePath: "testdata/fixtures/metrics/metrics_multi_project_expected.json",
SkipForSDK: true,
// Multi-project exporting is not supported in the SDK exporter
SkipForSDK: true,
},
// TODO: Add integration tests for workload.googleapis.com metrics from the ops agent
}
2 changes: 1 addition & 1 deletion exporter/metric/cloudmonitoring.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ import (
func New(opts ...Option) (metric.Exporter, error) {
o := options{
context: context.Background(),
resourceAttributeFilter: defaultResourceAttributesFilter,
resourceAttributeFilter: DefaultResourceAttributesFilter,
}
for _, opt := range opts {
opt(&o)
Expand Down
3 changes: 1 addition & 2 deletions exporter/metric/metric.go
Original file line number Diff line number Diff line change
Expand Up @@ -480,8 +480,7 @@ func gaugeToTimeSeries[N int64 | float64](point metricdata.DataPoint[N], metrics
ValueType: valueType,
Points: []*monitoringpb.Point{{
Interval: &monitoringpb.TimeInterval{
StartTime: timestamp,
EndTime: timestamp,
EndTime: timestamp,
},
Value: value,
}},
Expand Down
6 changes: 3 additions & 3 deletions exporter/metric/metric_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -275,12 +275,12 @@ func TestExtraLabelsFromResource(t *testing.T) {
}{
{
desc: "empty resource",
resourceAttributeFilter: defaultResourceAttributesFilter,
resourceAttributeFilter: DefaultResourceAttributesFilter,
expected: attribute.EmptySet(),
},
{
desc: "service labels added",
resourceAttributeFilter: defaultResourceAttributesFilter,
resourceAttributeFilter: DefaultResourceAttributesFilter,
input: resource.NewSchemaless(
semconv.ServiceNameKey.String("myservicename"),
semconv.ServiceNamespaceKey.String("myservicenamespace"),
Expand All @@ -290,7 +290,7 @@ func TestExtraLabelsFromResource(t *testing.T) {
},
{
desc: "non-service labels ignored",
resourceAttributeFilter: defaultResourceAttributesFilter,
resourceAttributeFilter: DefaultResourceAttributesFilter,
input: resource.NewSchemaless(
semconv.ServiceNameKey.String("myservicename"),
semconv.ServiceNamespaceKey.String("myservicenamespace"),
Expand Down
4 changes: 2 additions & 2 deletions exporter/metric/option.go
Original file line number Diff line number Diff line change
Expand Up @@ -103,9 +103,9 @@ func WithFilteredResourceAttributes(filter attribute.Filter) func(o *options) {
}
}

// defaultResourceAttributesFilter is the default filter applied to resource
// DefaultResourceAttributesFilter is the default filter applied to resource
// attributes.
func defaultResourceAttributesFilter(kv attribute.KeyValue) bool {
func DefaultResourceAttributesFilter(kv attribute.KeyValue) bool {
return kv.Key == semconv.ServiceNameKey ||
kv.Key == semconv.ServiceNamespaceKey ||
kv.Key == semconv.ServiceInstanceIDKey
Expand Down

0 comments on commit ca90414

Please sign in to comment.