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.

Sign up for GitHub

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

Jump to bottom

[receiver/sqlquery] Broken Test TestPostgresIntegrationLogsTrackingWithStorage #31782

Closed
TylerHelmuth opened this issue Mar 15, 2024 · 6 comments · Fixed by #31785
Closed

[receiver/sqlquery] Broken Test TestPostgresIntegrationLogsTrackingWithStorage #31782

TylerHelmuth opened this issue Mar 15, 2024 · 6 comments · Fixed by #31785
Labels
bug Something isn't working needs triage New item requiring triage receiver/sqlquery SQL query receiver

Comments

@TylerHelmuth
Copy link
Member

Component(s)

receiver/sqlquery

What happened?

Since #31667 was merged the TestPostgresIntegrationLogsTrackingWithStorage integration test has started failing silently for some reason.

You can see the failures inside the Run Integration Test step in jobs like:

image

For some reason my update-otel PR is actually showing the failure: https://github.com/open-telemetry/opentelemetry-collector-contrib/actions/runs/8300689476/job/22720973013

Collector version

Latest

Environment information

No response

OpenTelemetry Collector configuration

No response

Log output

No response

Additional context

No response
@TylerHelmuth TylerHelmuth added bug Something isn't working needs triage New item requiring triage receiver/sqlquery SQL query receiver labels Mar 15, 2024
@github-actions GitHub Actions
Copy link
Contributor

Pinging code owners:

See Adding Labels via Comments if you do not have permissions to add labels yourself.

@github-actions GitHub Actions
Copy link
Contributor

Pinging code owners for receiver/sqlquery: @dmitryax @crobert-1. See Adding Labels via Comments if you do not have permissions to add labels yourself.

@TylerHelmuth TylerHelmuth mentioned this issue Mar 15, 2024
Closed
@crobert-1
Copy link
Member

Thanks for filing, I'll take a look when I get a chance 👍

Pasting the test failure output:

=== FAIL: . TestPostgresIntegrationLogsTrackingWithStorage (10.80s)
2024/03/15 19:21:26 🐳 Creating container for image postgres:9.6.24
2024/03/15 19:21:26 ✅ Container created: 8b4be6dec43e
2024/03/15 19:21:26 🐳 Starting container: 8b4be6dec43e
2024/03/15 19:21:26 ✅ Container started: 8b4be6dec43e
2024/03/15 19:21:26 🚧 Waiting for container id 8b4be6dec43e image: postgres:9.6.24. Waiting for: &{Port:5432 timeout:0xc000bee0f8 PollInterval:100ms}
{"level":"debug","msg":"starting..."}
{"level":"debug","msg":"started."}
{"level":"debug","msg":"Running query"}
{"level":"debug","msg":"Running query"}
{"level":"debug","msg":"stopping..."}
{"level":"debug","msg":"stopped."}
{"level":"debug","msg":"starting..."}
make[2]: *** [../../Makefile.Common:142: mod-integration-test] Error 1
make[1]: *** [Makefile:165: receiver/sqlqueryreceiver] Error 2
make: *** [Makefile:122: gointegration-test] Error 2
{"level":"debug","msg":"started."}
{"level":"debug","msg":"Running query"}
{"level":"debug","msg":"Running query"}
{"level":"debug","msg":"Running query"}
{"level":"debug","msg":"Running query"}
{"level":"debug","msg":"stopping..."}
{"level":"debug","msg":"Running query"}
{"level":"debug","msg":"stopped."}
    integration_test.go:205: 
        	Error Trace:	/home/runner/work/opentelemetry-collector-contrib/opentelemetry-collector-contrib/receiver/sqlqueryreceiver/integration_test.go:205
        	Error:      	Not equal: 
        	            	expected: 0
        	            	actual  : 5
        	Test:       	TestPostgresIntegrationLogsTrackingWithStorage
2024/03/15 19:21:37 🐳 Terminating container: 8b4be6dec43e
2024/03/15 19:21:37 🚫 Container terminated: 8b4be6dec43e

=== FAIL: . TestPostgresIntegrationLogsTrackingWithStorage (re-run 1) (11.37s)
2024/03/15 19:22:08 github.com/testcontainers/testcontainers-go - Connected to docker: 
  Server Version: 24.0.9
  API Version: 1.43
  Operating System: Ubuntu 22.04.4 LTS
  Total Memory: 15981 MB
  Resolved Docker Host: unix:///var/run/docker.sock
  Resolved Docker Socket Path: /var/run/docker.sock
  Test SessionID: [255](https://github.com/open-telemetry/opentelemetry-collector-contrib/actions/runs/8300689476/job/22720973013#step:5:256)3bd2f08ff28a5fd5182f13a3edcdd5e001a7d0df1549d51ff6a924ce069e3
  Test ProcessID: bc91307c-2e72-4b5c-8d69-17e4ca4f2c6f
2024/03/15 19:22:08 🐳 Creating container for image testcontainers/ryuk:0.6.0
2024/03/15 19:22:08 ✅ Container created: 162027f87ee3
2024/03/15 19:22:08 🐳 Starting container: 162027f87ee3
2024/03/15 19:22:08 ✅ Container started: 162027f87ee3
2024/03/15 19:22:08 🚧 Waiting for container id 162027f87ee3 image: testcontainers/ryuk:0.6.0. Waiting for: &{Port:8080/tcp timeout:<nil> PollInterval:100ms}
2024/03/15 19:22:08 🐳 Creating container for image postgres:9.6.24
2024/03/15 19:22:08 ✅ Container created: b71564ce7d82
2024/03/15 19:22:08 🐳 Starting container: b71564ce7d82
2024/03/15 19:22:08 ✅ Container started: b71564ce7d82
2024/03/15 19:22:08 🚧 Waiting for container id b71564ce7d82 image: postgres:9.6.24. Waiting for: &{Port:5432 timeout:0xc00078c5c8 PollInterval:100ms}
{"level":"debug","msg":"starting..."}
{"level":"debug","msg":"started."}
{"level":"debug","msg":"Running query"}
{"level":"debug","msg":"Running query"}
{"level":"debug","msg":"stopping..."}
{"level":"debug","msg":"stopped."}
{"level":"debug","msg":"starting..."}
{"level":"debug","msg":"started."}
{"level":"debug","msg":"Running query"}
{"level":"debug","msg":"Running query"}
{"level":"debug","msg":"Running query"}
{"level":"debug","msg":"Running query"}
{"level":"debug","msg":"stopping..."}
{"level":"debug","msg":"Running query"}
{"level":"debug","msg":"stopped."}
    integration_test.go:205: 
        	Error Trace:	/home/runner/work/opentelemetry-collector-contrib/opentelemetry-collector-contrib/receiver/sqlqueryreceiver/integration_test.go:205
        	Error:      	Not equal: 
        	            	expected: 0
        	            	actual  : 5
        	Test:       	TestPostgresIntegrationLogsTrackingWithStorage
2024/03/15 19:22:19 🐳 Terminating container: b71564ce7d82
2024/03/15 19:22:19 🚫 Container terminated: b71564ce7d82

DONE 2 runs, 6 tests, 1 skipped, 2 failures in 105.373s
make[1]: Leaving directory '/home/runner/work/opentelemetry-collector-contrib/opentelemetry-collector-contrib'
Error: Process completed with exit code 2.

@TylerHelmuth
Copy link
Member Author

@crobert-1 I need to keep investigating #31778 as it is possible the issue isn't the test but something in core.

@crobert-1
Copy link
Member

Just a note, nothing in that PR is relevant to this test failure. A couple specific tests were changed (unrelated to this failure), and goleak was added. Failures from goleak start with the following:

goleak: Errors on successful test run: found unexpected goroutines:

@TylerHelmuth
Copy link
Member Author

Ya IDK why it switched the green check to a red x for those tests, so that is suspicious, but I dont think anymore that it is causing the failure in my PR. If you have any ideas why #31778 is failing that test with the latest from core please let me know, im stumped at the moment.

@crobert-1 crobert-1 mentioned this issue Mar 15, 2024
Merged
TylerHelmuth pushed a commit that referenced this issue Mar 16, 2024
@crobert-1
[receiver/sqlquery] Fix memory leak and failing tests (#31785)
794cd24 
**Description:** <Describe what has changed.>
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue.
Ex. Adding a feature - Explain what this achieves.-->
This PR includes two changes that are dependent on each other.

1. Fix test failing in
#31778.
Explanation given
[here.](#31778 (comment))
All changes in `integration_test.go` are related to this.
2. When the test was fixed, `goleak` started failing. The logs receiver
opens a DB connection when it's started, but shutdown does not close the
DB. This DB needs to be closed during shutdown to avoid a leaked
goroutine. All changes outside of `integration_test.go` are for this.
3. Since the memory leak changes were modifying errors, I moved from
using `multierr.append` to `errors.Join` as well.

**Link to tracking Issue:** <Issue number if applicable>
Resolves #31782
Related to
#31778

**Testing:** <Describe what testing was performed and which tests were
added.>
Tests are passing
This was referenced Mar 19, 2024
Closed
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working needs triage New item requiring triage receiver/sqlquery SQL query receiver
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[receiver/sqlquery] Fix memory leak and failing tests crobert-1/opentelemetry-collector-contrib
2 participants
@TylerHelmuth @crobert-1