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

proposal: contrib/testing: add an integration to support tracing tests under CI systems #772

Closed
jirikuncar opened this issue Nov 3, 2020 · 9 comments
Labels
apm:ecosystem contrib/* related feature requests or bugs enhancement quick change/addition that does not need full team approval needs-investigation requires work from team member proposal more in depth change that requires full team approval

Comments

@jirikuncar
Copy link

jirikuncar commented Nov 3, 2020

Along the same lines as the Python and Ruby tracers have done in DataDog/dd-trace-py#1741 and DataDog/dd-trace-rb#1216, we want to add some basic support for tracing go tests running in a CI environment.

This mostly consists of detecting the CI environment and adding specific tags that the backend understands to spans representing the go tests.

We want to provide a simple integration that allows the user to create spans with these tags for their tests. One challenge is making sure the go tracer is started when the tests run and shut down when they finish.

Helper method for tracer configuration

Usage

main_test.go

package test

import (
	"testing"

	ddtesting "github.com/DataDog/dd-trace-go/v1/contrib/testing"
)

// TestMain starts the tracer.
func TestMain(m *testing.M) {
	ddtesting.ConfigureTracer(m)
}

Implementation

package testing

import (
	gotesting "testing"
)

// ConfigureTracer starts the tracer.
func ConfigureTracer(m *gotesting.M) {
	service, ok := os.LookupEnv("DD_SERVICE")
	if !ok {
		service = "go.testing"
	}
	tracer.Start(
		tracer.WithService(service),
	)
	code := m.Run()
	tracer.Stop()
	os.Exit(code)
}

Helper method to test execution

project_test.go

package test

import "testing"

// Go 1.14+
func TestFoo(t *testing.T) {
	ctx := TraceTest(t)
	// span finish is automatically registered using https://golang.org/pkg/testing/#T.Cleanup

	t.Run("subtest", func(t *testing.T) {
		ctx := TraceTest(t, WithContext(ctx))
	})
}

// Go 1.12-13
func TestBar(t *testing.T) {
	ctx := TraceTest(t)
	defer FinishSpanFromContext(ctx)
}
  • WithContext
  • WithNameGenerator(f func(ctx context.Context, t *testing.T)) - this would be nice to have as tracer option

Implementation

func TraceTest(t *testing.T, options ...TestOptions) context.Context {

}

Prior work

@jirikuncar jirikuncar added enhancement quick change/addition that does not need full team approval apm:ecosystem contrib/* related feature requests or bugs needs-investigation requires work from team member proposal more in depth change that requires full team approval labels Nov 3, 2020
@knusbaum
Copy link
Contributor

knusbaum commented Nov 3, 2020

Can we clarify a bit the difference between the go1.14+ and go1.12-1.13 interface and why it's different?

I'm concerned that ConfigureTracer takes over the TestMain function. It means that the function must be the last thing called in TestMain and doesn't let the user do post-test processing. I think a defer pattern would be preferable.

This relates to #770 and #771

@jirikuncar
Copy link
Author

@knusbaum go 1.14+ has t.Cleanup(f func()) method which can be used to automatically register span closing. The FinishSpanFromContext(ctx) would be a noop function in go 1.14+.

@gbbr
Copy link
Contributor

gbbr commented Nov 4, 2020

What is currently stopping us from using the existing tracing package and functions as they are, in tests? What is missing? Can we have a problem definition (e.g. what we are trying to solve) ?

If we can do it with the tracing package, I don't think we need helper methods.

However, once we have a clear problem definition, we may be able to come up with a testing package along the lines of what @knusbaum was suggesting contrib/testing. Ideally users would instrument both their code and tests using the same code.

@jirikuncar
Copy link
Author

What is currently stopping us from using the existing tracing package and functions as they are, in tests?

@gbbr we want consistency in span tags with other languages, so they show up correctly in UI.

@knusbaum
Copy link
Contributor

knusbaum commented Nov 4, 2020

I've added a description to the issue.
@jirikuncar please update it further if I've misstated anything or left something out.

@knusbaum knusbaum changed the title proposal: contrib/testing: add a helper method to instrument tests and sub-tests proposal: contrib/testing: add an integration to support tracing tests under CI systems Nov 4, 2020
@gbbr
Copy link
Contributor

gbbr commented Nov 5, 2020

The description helps. Sounds like we need a contrib/testing package. Let's define the things that this package should do, such as:

@gbbr
Copy link
Contributor

gbbr commented Nov 5, 2020

Going deeper on the second point, I'd vote against the ConfigureTracer proposal because this takes using m and m.Run out of the user's hands and abstracts it away, stopping him from making any kind of additional TestMain changes. Instead, we should just include this into the example file which illustrates usage and have the user get 100% control of running m.Run()

It may already be possible to automatically add all the CI tags within TestMain using WithGlobalTag

@knusbaum
Copy link
Contributor

Closing as stale.

We can create a new proposal when test tracing comes back up.

@jirikuncar
Copy link
Author

Follow https://github.com/DataDog/dd-sdk-go-testing for information about test tracing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
apm:ecosystem contrib/* related feature requests or bugs enhancement quick change/addition that does not need full team approval needs-investigation requires work from team member proposal more in depth change that requires full team approval
Projects
None yet
Development

No branches or pull requests

3 participants