[mdatagen] Automatically generate goleak tests for packages

[crobert-1]

Component(s)

cmd/mdatagen

Is your feature request related to a problem? Please describe.

While working on #30438 a lot of identical code is being copied into many different places in the repository.

Describe the solution you'd like

As @dmitryax suggested here:

It would be great to generate this test along with #27849. If we need to skip it for some packages we can add skip_goleak: true with a comment to the tests section metadata.yaml

Configurable metadata options:

1. There should be an option to skip the goleak tests if necessary. This would likely mean the package_test.go file is not generated, and even deleted if it currently exists. To my knowledge, there's no way to include TestMain with the goleak call and also skip the resulting goleak check.
2. A configurable list of leaks to ignore. This would make use of the function goleak.IgnoreTopFunction. There are sometimes known issues that can't be addressed and irrelevant to the collector itself that can be safely ignored. PR [chore] Enable goleak for tests failing on opencensus-go #30457 has examples of this.

[github-actions]

Pinging code owners:

• cmd/mdatagen: @dmitryax

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

[github-actions]

Pinging code owners for cmd/mdatagen: @dmitryax. See Adding Labels via Comments if you do not have permissions to add labels yourself.

[crobert-1]

Removing needs triage as this suggestion was made by the mdatagen code owner, and multiple maintainers/approvers agreed with the idea.

[crobert-1]

Currently, the level of granularity between metadata.yaml files and goleak test packages is different. There's currently one metadata.yaml file for each component, but goleak is required for each golang package. An example is helpful to understand the difference.

The spanmetrics connector has a single top-level metadata.yaml file. However, for all tests to be covered by goleak, it requires ./package_test.go, internal/cache/package_test.go, and internal/metrics/package_test.go.

This means there will need to be a way to have multiple paths provided for generating goleak tests, each with its own configuration options, as proposed.

[crobert-1]

Another thing to consider is what I pointed out here, the fact that some packages already have TestMain. I need to investigate how to get existing functionality to work with adding goleak for those cases.

[dmitryax]

Currently, the level of granularity between metadata.yaml files and goleak test packages is different. There's currently one metadata.yaml file for each component, but goleak is required for each golang package. An example is helpful to understand the difference.

The spanmetrics connector has a single top-level metadata.yaml file. However, for all tests to be covered by goleak, it requires ./package_test.go, internal/cache/package_test.go, and internal/metrics/package_test.go.

This means there will need to be a way to have multiple paths provided for generating goleak tests, each with its own configuration options, as proposed.

As the first step, we should make sure that the test is generated per component/module. Then, we will see if it can be generated per module

[github-actions]

This issue has been inactive for 60 days. It will be closed in 60 days if there is no activity. To ping code owners by adding a component label, see Adding Labels via Comments, or if you are unsure of which component this issue relates to, please ping @open-telemetry/collector-contrib-triagers. If this issue is still relevant, please ping the code owners or leave a comment explaining why it is still relevant. Otherwise, please close it.

[dmitryax]

Another thing to consider is what I pointed out #30438 (comment) the fact that some packages already have TestMain. I need to investigate how to get existing functionality to work with adding goleak for those cases.

We can have another option in metadata like tests.goleak.addtional_test=true which would generate a function call like

func TestMain(m *testing.M) {
	goleak.VerifyTestMain(m)
	additionalTestMain(m)
}

end expect users to implement additionalTestMain in a separate file

[crobert-1]

We can have another option in metadata like tests.goleak.addtional_test=true which would generate a function call like

That would be super helpful. There are a number of test suites that rely on an existing TestMain 👍

However, I believe the way it's written (with additionalTestMain) would just execute all of the tests twice. The first test run would have goleak checks enabled, but tests would possibly fail because the original TestMain's setup wasn't done. It would also leak resources as the original TestMain's teardown isn't run, leading to potentially flaky tests. The second test suite run, additionalTestMain, runs with the package's proper setup and teardown for tests, but without goleak checks enabled.

The options added here may need to be something like:

tests.goleak.addtional_test_setup=true
tests.goleak.addtional_test_teardown=true

And then:

func TestMain(m *testing.M) {
        additionalTestSetup()
	goleak.VerifyTestMain(m)
        additionalTestTeardown()
}