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

[chore] Update go versions used in GitHub workflows #24213

Merged
merged 16 commits into from
Jul 12, 2023

Conversation

bryan-aguilar
Copy link
Contributor

Description: Update go versions used in GitHub workflows. Also changes to a stricter pattern match in some cases to provide more awareness on which version will be selected.

@MovieStoreGuy
Copy link
Contributor

Please rebase with main

@songy23 songy23 added the ready to merge Code review completed; ready to merge by maintainers label Jul 12, 2023
@djaglowski
Copy link
Member

I've rebased but now we have several integration tests failures in the docker observer:

--- FAIL: TestObserverEmitsEndpointsIntegration (2.19s)
    integration_test.go:53: 
        	Error Trace:	/home/runner/work/opentelemetry-collector-contrib/opentelemetry-collector-contrib/extension/observer/dockerobserver/integration_test.go:53
        	Error:      	Expected nil, but got: &fmt.wrapError{msg:"http: invalid Host header, host port waiting failed: could not start container: creating reaper failed: failed to create container", err:(*fmt.wrapError)(0xc0003df7a0)}
        	Test:       	TestObserverEmitsEndpointsIntegration

@jpkrohling
Copy link
Member

This test seems to be failing consistently:

--- FAIL: TestObserverEmitsEndpointsIntegration (2.29s)
    integration_test.go:53: 
        	Error Trace:	/home/runner/work/opentelemetry-collector-contrib/opentelemetry-collector-contrib/extension/observer/dockerobserver/integration_test.go:53
        	Error:      	Expected nil, but got: &fmt.wrapError{msg:"http: invalid Host header, host port waiting failed: could not start container: creating reaper failed: failed to create container", err:(*fmt.wrapError)(0xc0003e1960)}
        	Test:       	TestObserverEmitsEndpointsIntegration

@bryan-aguilar
Copy link
Contributor Author

Taking a look

@bryan-aguilar
Copy link
Contributor Author

I believe this is the same issue. which may be an issue with testcontainers-go. Currently looking for a workaround.

@djaglowski
Copy link
Member

@bryan-aguilar, I've made a PR to your branch that would skip all container creation via testcontainers-go. bryan-aguilar#3

@bryan-aguilar bryan-aguilar requested a review from a team July 12, 2023 15:44
@bryan-aguilar
Copy link
Contributor Author

bryan-aguilar commented Jul 12, 2023

@djaglowski I will write up an issue for these skipped test cases using test containers.

Copy link
Contributor

@codeboten codeboten left a comment

Choose a reason for hiding this comment

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

Looks like another lint failure:

level=info msg="File cache stats: 15 entries of total size 325.0KiB"
	params, ctx, cancel := paramsAndContext(t)
	^
Error: dockerstatsreceiver/integration_test.go:143:2: SA4006: this value of `params` is never used (staticcheck)
	params, ctx, cancel := paramsAndContext(t)
	^

@bryan-aguilar
Copy link
Contributor Author

Yup.....I'm taking a look at this already. fun™

@bryan-aguilar
Copy link
Contributor Author

The linting failure doesn't make sense right now. I can replicate it locally though. That value is used in the 10 line later.

	params, ctx, cancel := paramsAndContext(t)
	defer cancel()

	container := createNginxContainer(ctx, t)

	f, config := factory()
	config.ExcludedImages = append(config.ExcludedImages, "*nginx*")

	consumer := new(consumertest.MetricsSink)
	recv, err := f.CreateMetricsReceiver(ctx, params, config, consumer)

@bryan-aguilar
Copy link
Contributor Author

I suspect dominikh/go-tools#364

@djaglowski
Copy link
Member

This was written to trick the unused linter, but it may work here too.

@bryan-aguilar
Copy link
Contributor Author

I have ignored the staticcheck linting failures for now.

@bryan-aguilar
Copy link
Contributor Author

 panic: test timed out after 6m0s

goroutine 9233 [running]:
testing.(*M).startAlarm.func1()
	/opt/hostedtoolcache/go/1.19.11/x64/src/testing/testing.go:2036 +0xbb
created by time.goFunc
	/opt/hostedtoolcache/go/1.19.11/x64/src/time/sleep.go:176 +0x48

goroutine 1 [chan receive, 6 minutes]:
testing.(*T).Run(0xc000104680, {0x15aeaeb, 0xf}, 0x1606ce8)
	/opt/hostedtoolcache/go/1.19.11/x64/src/testing/testing.go:1494 +0x789
testing.runTests.func1(0x0?)
	/opt/hostedtoolcache/go/1.19.11/x64/src/testing/testing.go:1846 +0x9a
testing.tRunner(0xc000104680, 0xc0005bfba0)
	/opt/hostedtoolcache/go/1.19.11/x64/src/testing/testing.go:1446 +0x217
testing.runTests(0xc000134820?, {0x1ed1540, 0x9, 0x9}, {0x40?, 0x7f3871831328?, 0x1ee1580?})
	/opt/hostedtoolcache/go/1.19.11/x64/src/testing/testing.go:1844 +0x7ed
testing.(*M).Run(0xc000134820)
	/opt/hostedtoolcache/go/1.19.11/x64/src/testing/testing.go:1726 +0xa85
main.main()
	_testmain.go:65 +0x2ea

goroutine 4 [select]:
go.opencensus.io/stats/view.(*worker).start(0xc0000fca80)
	/home/runner/go/pkg/mod/go.opencensus.io@v0.24.0/stats/view/worker.go:292 +0x185
created by go.opencensus.io/stats/view.init.0
	/home/runner/go/pkg/mod/go.opencensus.io@v0.24.0/stats/view/worker.go:34 +0xf7

goroutine 6 [chan receive, 6 minutes]:
testing.(*T).Run(0xc000104820, {0x15a33de, 0x3}, 0xc00004d750)
	/opt/hostedtoolcache/go/1.19.11/x64/src/testing/testing.go:1494 +0x789
github.com/open-telemetry/opentelemetry-collector-contrib/receiver/aerospikereceiver.TestIntegration(0x0?)
	/home/runner/work/opentelemetry-collector-contrib/opentelemetry-collector-contrib/receiver/aerospikereceiver/integration_test.go:31 +0x4a
testing.tRunner(0xc000104820, 0x1606ce8)
	/opt/hostedtoolcache/go/1.19.11/x64/src/testing/testing.go:1446 +0x217
created by testing.(*T).Run
	/opt/hostedtoolcache/go/1.19.11/x64/src/testing/testing.go:1493 +0x75e

goroutine 7 [semacquire, 6 minutes]:
sync.runtime_Semacquire(0xc0004a4178?)
	/opt/hostedtoolcache/go/1.19.11/x64/src/runtime/sema.go:62 +0x25
sync.(*WaitGroup).Wait(0xc0004a4170)
	/opt/hostedtoolcache/go/1.19.11/x64/src/sync/waitgroup.go:139 +0xa6
github.com/open-telemetry/opentelemetry-collector-contrib/internal/coreinternal/scraperinttest.(*IntegrationTest).createContainers(0xc000114580, 0xc000104b60)
	/home/runner/work/opentelemetry-collector-contrib/opentelemetry-collector-contrib/internal/coreinternal/scraperinttest/scraperint.go:192 +0x1a5
github.com/open-telemetry/opentelemetry-collector-contrib/internal/coreinternal/scraperinttest.(*IntegrationTest).Run(0xc000114580, 0xc000104b60)
	/home/runner/work/opentelemetry-collector-contrib/opentelemetry-collector-contrib/internal/coreinternal/scraperinttest/scraperint.go:76 +0x105
testing.tRunner(0xc000104b60, 0xc00004d750)
	/opt/hostedtoolcache/go/1.19.11/x64/src/testing/testing.go:1446 +0x217
created by testing.(*T).Run
	/opt/hostedtoolcache/go/1.19.11/x64/src/testing/testing.go:1493 +0x75e
FAIL	github.com/open-telemetry/opentelemetry-collector-contrib/receiver/aerospikereceiver	360.041s
ok  	github.com/open-telemetry/opentelemetry-collector-contrib/receiver/aerospikereceiver/cluster	0.027s [no tests to run]
?   	github.com/open-telemetry/opentelemetry-collector-contrib/receiver/aerospikereceiver/cluster/mocks	[no test files]
ok  	github.com/open-telemetry/opentelemetry-collector-contrib/receiver/aerospikereceiver/internal/metadata	0.061s [no tests to run]
?   	github.com/open-telemetry/opentelemetry-collector-contrib/receiver/aerospikereceiver/mocks	[no test files]
FAIL
make[2]: Leaving directory '/home/runner/work/opentelemetry-collector-contrib/opentelemetry-collector-contrib/receiver/aerospikereceiver'

:(

Could this be a flake? I see it depends on scraperint but it is not skipped.

@bryan-aguilar
Copy link
Contributor Author

I found an additional location containers were being created that was not triggering a skip.

@mx-psi
Copy link
Member

mx-psi commented Jul 12, 2023

snmpreceiver/integration_test.go:29:2: SA4006: this value of `testCases` is never used (staticcheck)

@bryan-aguilar
Copy link
Contributor Author

I'm on it. Give me a few. I think I will have a solution that should clear this up.

@bryan-aguilar
Copy link
Contributor Author

Just an update. I'm going to try to use the suggestion @djaglowski made on the skip var workaround. I am planning on moving it to the testutil package so that it can be easily reused. I've hit some snag that are all GoLand IDE related.

@bryan-aguilar
Copy link
Contributor Author

that didn't work. Going back to inserting static check ignores.

@bryan-aguilar
Copy link
Contributor Author

Once this is merged I will prep a PR to remove all the test and static check skips. I will put it into draft status until the upstream dependencies are fixed. But that should expedite the reenablement of the tests.

@djaglowski djaglowski merged commit 2dde029 into open-telemetry:main Jul 12, 2023
90 checks passed
@github-actions github-actions bot added this to the next release milestone Jul 12, 2023
@bryan-aguilar bryan-aguilar deleted the updateGo1206 branch July 12, 2023 19:53
dmitryax pushed a commit that referenced this pull request Jul 21, 2023
…issue (#24247)

**Description:** Dependent on
testcontainers/testcontainers-go#1359 being
resolved.

Enable tests that were skipped in #24213

**Link to tracking Issue:**
#24240
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants