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

fix: test failure cases on windows #1047

Merged
merged 10 commits into from
Dec 6, 2021
4 changes: 1 addition & 3 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -108,9 +108,7 @@ jobs:
strategy:
matrix:
go-version: [1.17, 1.16]
# TODO: support all of these.
#os: [ubuntu-latest, macos-latest, windows-latest]
os: [ubuntu-latest, macos-latest]
os: [ubuntu-latest, macos-latest, windows-latest]
# GitHub Actions does not support arm* architectures on default
# runners. It is possible to acomplish this with a self-hosted runner
# if we want to add this in the future:
Expand Down
11 changes: 11 additions & 0 deletions exporters/metric/cortex/utils/config_utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,17 @@ func initYAML(yamlBytes []byte, path string) (afero.Fs, error) {
// Create an in-memory file system.
fs := afero.NewMemMapFs()

// https://github.com/spf13/viper/blob/v1.8.1/viper.go#L480
// absPathify uses filepath.Clean, so here you also need to use filepath.Clean
if filepath.IsAbs(path) {
path = filepath.Clean(path)
} else {
p, err := filepath.Abs(path)
if err == nil {
path = filepath.Clean(p)
}
}

// Retrieve the directory path.
dirPath := filepath.Dir(path)

Expand Down
12 changes: 9 additions & 3 deletions exporters/metric/datadog/example_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,16 @@ import (

"github.com/DataDog/datadog-go/statsd"

"go.opentelemetry.io/contrib/exporters/metric/datadog"
"go.opentelemetry.io/otel/attribute"
"go.opentelemetry.io/otel/metric"
"go.opentelemetry.io/otel/metric/global"
controller "go.opentelemetry.io/otel/sdk/metric/controller/basic"
"go.opentelemetry.io/otel/sdk/metric/processor/basic"
"go.opentelemetry.io/otel/sdk/metric/selector/simple"
"go.opentelemetry.io/otel/sdk/resource"
semconv "go.opentelemetry.io/otel/semconv/v1.4.0"

"go.opentelemetry.io/contrib/exporters/metric/datadog"
)

type TestUDPServer struct {
Expand All @@ -57,12 +60,15 @@ func ExampleExporter() {
go func() {
defer exp.Close()
processor := basic.NewFactory(selector, exp)
cont := controller.New(processor, controller.WithExporter(exp), controller.WithCollectPeriod(time.Second*10))
cont := controller.New(processor, controller.WithExporter(exp), controller.WithCollectPeriod(time.Second*10),
controller.WithResource(resource.Default()),
controller.WithResource(resource.NewSchemaless(semconv.ServiceNameKey.String("ExampleExporter"))))
Copy link
Member

Choose a reason for hiding this comment

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

Should this use New() instead of NewSchemaless()? We have a schema from semconv.SchemaURL.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, @Aneurysm9

Okey! like this?

ctx := context.Background()
serviceName, err := resource.New(ctx, resource.WithAttributes(semconv.ServiceNameKey.String("ExampleExporter")))
if err != nil {
	panic(err)
}

cont := controller.New(processor, controller.WithExporter(exp), controller.WithCollectPeriod(time.Second*10),
	controller.WithResource(resource.Default()), controller.WithResource(serviceName))

ctx := context.Background()
err := cont.Start(ctx)
if err != nil {
panic(err)
}

defer func() { handleErr(cont.Stop(ctx)) }()
global.SetMeterProvider(cont)
meter := global.Meter("marwandist")
Expand Down Expand Up @@ -98,7 +104,7 @@ func ExampleExporter() {
}

// Output:
// myrecorder.max:100|g|#env:dev,l:1,service.name:unknown_service:datadog.test,telemetry.sdk.language:go,telemetry.sdk.name:opentelemetry,telemetry.sdk.version:1.2.0
// myrecorder.max:100|g|#env:dev,l:1,service.name:ExampleExporter,telemetry.sdk.language:go,telemetry.sdk.name:opentelemetry,telemetry.sdk.version:1.2.0
//
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
"context"
"os"
"reflect"
"runtime"
"testing"
"time"

Expand Down Expand Up @@ -67,6 +68,12 @@ func setEnvVars() {
_ = os.Setenv("AWS_REGION", "us-texas-1")
_ = os.Setenv("AWS_LAMBDA_FUNCTION_VERSION", "$LATEST")
_ = os.Setenv("_X_AMZN_TRACE_ID", "Root=1-5759e988-bd862e3fe1be46a994272793;Parent=53995c3f42cd8ad8;Sampled=1")

// fix issue: "The requested service provider could not be loaded or initialized."
// Guess: The env for Windows in GitHub action is incomplete
if runtime.GOOS == "windows" && os.Getenv("SYSTEMROOT") == "" {
_ = os.Setenv("SYSTEMROOT", `C:\Windows`)
}
}

// Vars for end to end testing
Expand Down Expand Up @@ -161,7 +168,7 @@ func TestWrapEndToEnd(t *testing.T) {
customerHandler := func() (string, error) {
return "hello world", nil
}
mockCollector := runMockCollectorAtEndpoint(t, "localhost:4317")
mockCollector := runMockCollectorAtEndpoint(t, ":4317")
defer func() {
_ = mockCollector.Stop()
}()
Expand Down
19 changes: 9 additions & 10 deletions instrumentation/host/host_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,10 @@ import (
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"go.opentelemetry.io/contrib/instrumentation/host"
"go.opentelemetry.io/otel/attribute"
"go.opentelemetry.io/otel/metric/metrictest"

"go.opentelemetry.io/contrib/instrumentation/host"
)

func getMetric(provider *metrictest.MeterProvider, name string, lbl attribute.KeyValue) float64 {
Expand Down Expand Up @@ -218,20 +219,18 @@ func TestHostNetwork(t *testing.T) {
require.NoError(t, err)

// As we are going to read the /proc file system for this info, sleep a while:
time.Sleep(time.Second)

provider.RunAsyncInstruments()
require.Eventually(t, func() bool {
hostAfter, err := net.IOCountersWithContext(ctx, false)
require.NoError(t, err)

hostAfter, err := net.IOCountersWithContext(ctx, false)
require.NoError(t, err)
return uint64(howMuch) <= hostAfter[0].BytesSent-hostBefore[0].BytesSent &&
uint64(howMuch) <= hostAfter[0].BytesRecv-hostBefore[0].BytesRecv
Comment on lines +226 to +227
Copy link
Member

Choose a reason for hiding this comment

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

Can you keep these as assertions? Doing so should help in understanding where a failure occurred if this test starts failing in the future.

Copy link
Contributor Author

@laojianzi laojianzi Nov 1, 2021

Choose a reason for hiding this comment

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

Thanks @Aneurysm9

Because it is necessary to wait for BytesSent and BytesRecv all the time, they are not assert here.

There should only be one timeout error here and it will be tracked to the require.Eventually line, but 30 seconds will cause the go test to time out without printing the error tracking message, so consider updating it to <30 seconds?

#1047 (comment) The idea. WDYT?

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 error trace info for the 20s timeout

=== RUN TestHostNetwork
host_test.go:222:
Error Trace: host_test.go:222
Error: Condition never satisfied
Test: TestHostNetwork
--- FAIL: TestHostNetwork (20.00s)

}, 30*time.Second, time.Second/2)

provider.RunAsyncInstruments()
hostTransmit := getMetric(provider, "system.network.io", host.AttributeNetworkTransmit[0])
hostReceive := getMetric(provider, "system.network.io", host.AttributeNetworkReceive[0])

// Check that the network transmit/receive used is greater than before:
require.LessOrEqual(t, uint64(howMuch), hostAfter[0].BytesSent-hostBefore[0].BytesSent)
require.LessOrEqual(t, uint64(howMuch), hostAfter[0].BytesRecv-hostBefore[0].BytesRecv)

// Check that the recorded measurements reflect the same change:
require.LessOrEqual(t, uint64(howMuch), uint64(hostTransmit)-hostBefore[0].BytesSent)
require.LessOrEqual(t, uint64(howMuch), uint64(hostReceive)-hostBefore[0].BytesRecv)
Expand Down
11 changes: 10 additions & 1 deletion instrumentation/net/http/otelhttp/test/transport_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"io/ioutil"
"net/http"
"net/http/httptest"
"runtime"
"strings"
"testing"

Expand Down Expand Up @@ -117,7 +118,15 @@ func TestTransportErrorStatus(t *testing.T) {
t.Errorf("expected error status code on span; got: %q", got)
}

if got := span.Status().Description; !strings.Contains(got, "connect: connection refused") {
errSubstr := "connect: connection refused"
if runtime.GOOS == "windows" {
// tls.Dial returns an error that does not contain the substring "connection refused"
// on Windows machines
//
// ref: "dial tcp 127.0.0.1:50115: connectex: No connection could be made because the target machine actively refused it."
errSubstr = "No connection could be made because the target machine actively refused it"
}
if got := span.Status().Description; !strings.Contains(got, errSubstr) {
t.Errorf("expected error status message on span; got: %q", got)
}
}
Expand Down