fix: test failure cases on windows #1047

laojianzi · 2021-09-03T09:41:22Z

~~But it's not clear what the problem is here~~

https://github.com/laojianzi/opentelemetry-go-contrib/runs/3504741867?check_suite_focus=true#step:6:297

=== RUN ExampleExporter
--- FAIL: ExampleExporter (0.00s)
got:
myrecorder.max:100|g|#env:dev,l:1,service.name:unknown_service:datadog.test.exe,telemetry.sdk.language:go,telemetry.sdk.name:opentelemetry,telemetry.sdk.version:1.0.0-RC1
want:
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.0.0-RC1
FAIL
FAIL go.opentelemetry.io/contrib/exporters/metric/datadog 0.026s
FAIL
mingw32-make: *** [Makefile:130: test-short] Error 1
Error: Process completed with exit code 1.

Resolved in #1047 (comment)

Aneurysm9 · 2021-09-03T15:20:45Z

The DataDog test failure seems to be caused by the default service name being unknown_service:datadog.test.exe on windows and the .exe is unexpected. It should be possible to solve that by giving the TracerProvider a resource with an explicitly set service name so that it doesn't vary by platform.

laojianzi · 2021-09-05T06:51:15Z

The DataDog test failure seems to be caused by the default service name being unknown_service:datadog.test.exe on windows and the .exe is unexpected. It should be possible to solve that by giving the TracerProvider a resource with an explicitly set service name so that it doesn't vary by platform.

thx, I think I found the answer here 😃

https://github.com/open-telemetry/opentelemetry-go/blob/v1.0.0-RC2/semconv/v1.4.0/resource.go#L785

pellared

nice 👍

…environment

laojianzi · 2021-09-13T13:43:26Z

@Aneurysm9 pls review again 🙃

exporters/metric/datadog/example_test.go

….name`

…machines

laojianzi · 2021-09-15T03:38:01Z

=== RUN   TestHostNetwork
    assertion_compare.go:342:
        	Error Trace:	host_test.go:235
        	Error:      	"10000" is not less than or equal to "7837"
        	Test:       	TestHostNetwork
        	Messages:   	[]
--- FAIL: TestHostNetwork

This mistake was accidental and increasing sleep can only improve the chances of success, any ideas to help?

on windows

laojianzi · 2021-09-24T13:15:08Z

=== RUN   TestHostNetwork
    assertion_compare.go:342:
        	Error Trace:	host_test.go:235
        	Error:      	"10000" is not less than or equal to "7837"
        	Test:       	TestHostNetwork
        	Messages:   	[]
--- FAIL: TestHostNetwork

This mistake was accidental and increasing sleep can only improve the chances of success, any ideas to help?

on windows

@Aneurysm9 I used the following method to debug and the test passed in 3s/8s/10s ... etc, so I can't use sleep to guarantee the success of the test, do you think it's reasonable to wait for sent/recv

Is it reasonable to wait until sent/recv is satisfied until the go test times out?

opentelemetry-go-contrib/instrumentation/host/host_test.go

Lines 222 to 223 in 8e44f3f

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

opentelemetry-go-contrib/instrumentation/host/host_test.go

Lines 222 to 232 in 330ca09

    
           // As we are going to read the /proc file system for this info, sleep a while: 
        
           for { 
        
           	hostAfter, err := net.IOCountersWithContext(ctx, false) 
        
           	require.NoError(t, err) 
        
           	time.Sleep(time.Second / 2) 
        
           	if uint64(howMuch) <= hostAfter[0].BytesSent-hostBefore[0].BytesSent && 
        
           		uint64(howMuch) <= hostAfter[0].BytesRecv-hostBefore[0].BytesRecv { 
        
           		break 
        
           	} 
        
           }

pellared · 2021-09-28T09:06:29Z

I used the following method to debug and the test passed in 3s/8s/10s ... etc, so I can't use sleep to guarantee the success of the test, do you think it's reasonable to wait for sent/recv

At first glance it looks like a good candidate to use require.Eventually. WDYT?

laojianzi · 2021-09-28T09:44:50Z

At first glance it looks like a good candidate to use require.Eventually. WDYT?

Of course, but how long should we wait ? as this process seems very unstable on Windows 🤔

pellared · 2021-09-28T10:11:16Z

Of course, but how long should we wait ? as this process seems very unstable on Windows

I would try max 30s - should be long enough I guess 😉 intervals one or half of a second?

laojianzi · 2021-09-29T09:10:18Z

@pellared Thank you for your help 👍

Aneurysm9 · 2021-10-29T20:23:11Z

instrumentation/host/host_test.go

+		return uint64(howMuch) <= hostAfter[0].BytesSent-hostBefore[0].BytesSent &&
+			uint64(howMuch) <= hostAfter[0].BytesRecv-hostBefore[0].BytesRecv


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

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?

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)

Aneurysm9 · 2021-10-29T20:24:31Z

exporters/metric/datadog/example_test.go

-		pusher := controller.New(processor, controller.WithExporter(exp), controller.WithCollectPeriod(time.Second*10))
+		pusher := controller.New(processor, controller.WithExporter(exp), controller.WithCollectPeriod(time.Second*10),
+			controller.WithResource(resource.Default()),
+			controller.WithResource(resource.NewSchemaless(semconv.ServiceNameKey.String("ExampleExporter"))))


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

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))

MadVikingGod

Can you please merge in main to resolve some conflicts?

Other than the outstanding comments it LGTM

laojianzi · 2021-12-03T03:33:59Z

Can you please merge in main to resolve some conflicts?

Other than the outstanding comments it LGTM

@MadVikingGod Done

laojianzi requested review from Aneurysm9, dashpole, evantorrie, jmacd, MadVikingGod, MrAlias, paivagustavo, pellared and XSAM as code owners September 3, 2021 09:41

laojianzi force-pushed the fix/issues-1010 branch from 190b194 to 355398d Compare September 5, 2021 06:46

laojianzi force-pushed the fix/issues-1010 branch 2 times, most recently from 2b7bd82 to b296754 Compare September 5, 2021 06:56

pellared approved these changes Sep 6, 2021

View reviewed changes

laojianzi added 2 commits September 13, 2021 21:29

fix: utils.NewConfig returns a file not found error in windows os

4b9fb4f

Fix: set service.name so that Output does not fail due to system …

bac4231

…environment

laojianzi force-pushed the fix/issues-1010 branch from b296754 to bac4231 Compare September 13, 2021 13:39

Aneurysm9 reviewed Sep 13, 2021

View reviewed changes

exporters/metric/datadog/example_test.go Outdated Show resolved Hide resolved

laojianzi force-pushed the fix/issues-1010 branch from f1228dc to fe5d241 Compare September 14, 2021 11:09

laojianzi added 2 commits September 14, 2021 21:00

fix: golang.org/x/sys version mismatch

5fbe2c8

chore: use WithResource in the ExampleExporter to change `service…

072939c

….name`

laojianzi force-pushed the fix/issues-1010 branch from accdfa7 to 072939c Compare September 14, 2021 13:03

dashpole approved these changes Sep 14, 2021

View reviewed changes

fix: TestTransportErrorStatus connection refused assert on Windows …

3c59acf

…machines

laojianzi changed the title ~~fix: utils.NewConfig returns a file not found error in windows os~~ fix: test failure cases on windows Sep 15, 2021

Merge remote-tracking branch 'upstream/main' into fix/issues-1010

08b8025

test: TestHostNetwork Wait for results until go test times out

f7764b4

laojianzi force-pushed the fix/issues-1010 branch from 330ca09 to f7764b4 Compare September 29, 2021 07:36

pellared approved these changes Sep 29, 2021

View reviewed changes

Aneurysm9 reviewed Oct 29, 2021

View reviewed changes

MadVikingGod approved these changes Dec 1, 2021

View reviewed changes

Merge remote-tracking branch 'main' into fix/issues-1010

73e1e61

laojianzi force-pushed the fix/issues-1010 branch 12 times, most recently from 5edf45d to cacb124 Compare December 3, 2021 03:22

fix: windows net listen failed

ca6b0a3

laojianzi force-pushed the fix/issues-1010 branch from cacb124 to ca6b0a3 Compare December 3, 2021 03:24

Merge branch 'main' into fix/issues-1010

948f3dc

Aneurysm9 merged commit dac6cdb into open-telemetry:main Dec 6, 2021

pellared added this to the untracked milestone Nov 8, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: test failure cases on windows #1047

fix: test failure cases on windows #1047

laojianzi commented Sep 3, 2021 •

edited

Loading

Aneurysm9 commented Sep 3, 2021

laojianzi commented Sep 5, 2021

pellared left a comment

laojianzi commented Sep 13, 2021

laojianzi commented Sep 15, 2021 •

edited

Loading

laojianzi commented Sep 24, 2021

pellared commented Sep 28, 2021

laojianzi commented Sep 28, 2021

pellared commented Sep 28, 2021 •

edited

Loading

laojianzi commented Sep 29, 2021

Aneurysm9 Oct 29, 2021

laojianzi Nov 1, 2021 •

edited

Loading

laojianzi Nov 1, 2021

Aneurysm9 Oct 29, 2021

laojianzi Nov 1, 2021

MadVikingGod left a comment

laojianzi commented Dec 3, 2021

		return uint64(howMuch) <= hostAfter[0].BytesSent-hostBefore[0].BytesSent &&
		uint64(howMuch) <= hostAfter[0].BytesRecv-hostBefore[0].BytesRecv

fix: test failure cases on windows #1047

fix: test failure cases on windows #1047

Conversation

laojianzi commented Sep 3, 2021 • edited Loading

Aneurysm9 commented Sep 3, 2021

laojianzi commented Sep 5, 2021

pellared left a comment

Choose a reason for hiding this comment

laojianzi commented Sep 13, 2021

laojianzi commented Sep 15, 2021 • edited Loading

laojianzi commented Sep 24, 2021

pellared commented Sep 28, 2021

laojianzi commented Sep 28, 2021

pellared commented Sep 28, 2021 • edited Loading

laojianzi commented Sep 29, 2021

Aneurysm9 Oct 29, 2021

Choose a reason for hiding this comment

laojianzi Nov 1, 2021 • edited Loading

Choose a reason for hiding this comment

laojianzi Nov 1, 2021

Choose a reason for hiding this comment

Aneurysm9 Oct 29, 2021

Choose a reason for hiding this comment

laojianzi Nov 1, 2021

Choose a reason for hiding this comment

MadVikingGod left a comment

Choose a reason for hiding this comment

laojianzi commented Dec 3, 2021

laojianzi commented Sep 3, 2021 •

edited

Loading

laojianzi commented Sep 15, 2021 •

edited

Loading

pellared commented Sep 28, 2021 •

edited

Loading

laojianzi Nov 1, 2021 •

edited

Loading