contrib: add validation tests using test-agent #2047
Conversation
rarguelloF
force-pushed
the
rarguelloF/test-agent
branch
from
00f9452
to
6e77417
Compare
| memcachetest "gopkg.in/DataDog/dd-trace-go.v1/contrib/internal/validationtest/integrations/gomemcache/memcache" | ||
| dnstest "gopkg.in/DataDog/dd-trace-go.v1/contrib/internal/validationtest/integrations/miekg/dns" |
There was a problem hiding this comment.
Instead of these super long paths and instead of creating a new contrib/internal/validationtest/integrations directory, how about putting the test utils of each contrib in an internal dir under the corresponding and existing contrib package (i.e contrib/<path>/<to>/<dir>/internal) ?
| memcachetest "gopkg.in/DataDog/dd-trace-go.v1/contrib/internal/validationtest/integrations/gomemcache/memcache" | |
| dnstest "gopkg.in/DataDog/dd-trace-go.v1/contrib/internal/validationtest/integrations/miekg/dns" | |
| memcachetest "gopkg.in/DataDog/dd-trace-go.v1/contrib/gomemcache/memcache/internal" | |
| dnstest "gopkg.in/DataDog/dd-trace-go.v1/contrib/miekg/dns/internal" |
There was a problem hiding this comment.
Doing this results in a use of internal package ... not allowed to compile error when trying to import the tests within the validationtest file.
There was a problem hiding this comment.
How about putting the test utils in contrib/<path>/<to>/<dir>/internal/test instead of contrib/<path>/<to>/<dir>/internal ?
There was a problem hiding this comment.
Bumping this thread as I believe we can avoid re-creating individual and long contrib folders.
e.g contrib/internal/validationtest/contrib/miekg/dns/integration.go should be moved to contrib/miekg/dns/internal/test/integration.go. The contrib/miekg/dns dir already exists.
There was a problem hiding this comment.
The problem is we cannot import an internal sub-package from here since the Go compiler doesn't allow so:
When the go command sees an import of a package with internal in its path, it verifies that the package doing the import is within the tree rooted at the parent of the internal directory. For example, a package .../a/b/c/internal/d/e/f can be imported only by code in the directory tree rooted at .../a/b/c. It cannot be imported by code in .../a/b/g or in any other repository.
So in this case we have:
contrib/internal/validationtest
And the proposed internal subpackages would be:
contrib/miekg/dns/internal/test
So since the only common root from both directories is contrib/, this wouldn't be possible and any shared internal code needs to be placed under contrib/internal (sadly for us 😢 ).
There was a problem hiding this comment.
An extra alternative would be to have these sub-packages not being internal, like contrib/miekg/dns/test - not sure how we feel about this approach...
There was a problem hiding this comment.
Thanks for the explanation @rarguelloF !
I think an alternative to have shorter paths would be to have a big package contrib/internal/, where each integration is in its own file, like miekg_dns.go, net_http.go, etc.
I like the idea! We can do it this way for now and move files to separate packages per integration in the future if necessary.
There was a problem hiding this comment.
miekg_dns.go, net_http.go, etc can live under contrib/internal/validationtest or even contrib/internal/validationtest/contribs, right?
There was a problem hiding this comment.
Since we redeclare the Integration struct for each contrib test file, they need to have separate packages and therefore live in separate folders, otherwise I was getting errors trying to put everything in the same directory. I changed the structure to be: contrib/internal/validationtest/miekg/dns.go, contrib/internal/validationtest/bradfitz/memcache.go ... etc
There was a problem hiding this comment.
@wconti27 if we follow the big package approach, each struct needs to be named differently as they share the same namespace. e.g. NetHTTP, etc.
contrib/internal/validationtest/integrations/miekg/dns/integration.go
Outdated
Show resolved
Hide resolved
contrib/internal/validationtest/integrations/miekg/dns/integration.go
Outdated
Show resolved
Hide resolved
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
|
@wconti27 & @rarguelloF I won't be available for the next few weeks! @dianashevchenko will own the review of this PR and help move it forward. Thanks! |
Co-authored-by: Rodrigo Arguello <[email protected]>
…)" This reverts commit b417ae3.
| componentName := ig.Name() | ||
| if componentName == "jmoiron/sqlx" { | ||
| componentName = "database/sql" | ||
| } |
There was a problem hiding this comment.
Mind explaining this special case? Why should the jmoiron/sqlx contrib return database/sql as name?
There was a problem hiding this comment.
Removing this, I honestly cant remember why I added this but I can cross that bridge once those integrations are added.
| memcachetest "gopkg.in/DataDog/dd-trace-go.v1/contrib/internal/validationtest/integrations/gomemcache/memcache" | ||
| dnstest "gopkg.in/DataDog/dd-trace-go.v1/contrib/internal/validationtest/integrations/miekg/dns" |
There was a problem hiding this comment.
Bumping this thread as I believe we can avoid re-creating individual and long contrib folders.
e.g contrib/internal/validationtest/contrib/miekg/dns/integration.go should be moved to contrib/miekg/dns/internal/test/integration.go. The contrib/miekg/dns dir already exists.
ahmed-mez
changed the title
Draft PR for PoC of Adding Datadog APM Test Agent alongside Go Tracer contrib tests
Two integrations were added as examples of implementing the new testing interface.
A CI step to show all Datadog APM Test Agent logs for traces received, trace checks run, failing trace checks, etc.
What does this PR do?
This PR adds the APM Test Agent to run alongside unit / contrib tests in CI testing. A testing interface is created in
contrib/internal/validationtest, which can be implemented by each contrib integration in order to generate test traces which are emitted to the APM Test Agent using the real Golang tracer. These traces are tested against a number of centrally located checks written within the Test-Agent, including:The APM Test Agent is being added to every DD APM Tracer with the primary purpose of providing a centralized location for writing tests which will subsequently be used for asserting on handled traces. This strategy will help bring consistency to Datadog APM Tagging/ integration support.
Motivation
We're currently working on integrating the test agent into each tracer's test suites, and for the test agent to handle traces produced by integrations during testing. Within the Test-Agent, we are then creating checks for Service Naming, Unified Naming Conventions tagging, etc in order to be a centralized location for asserting on integration traces. Allows us to leverage traces produced from integrations during testing.
Testing
See above for passing / non passing integration tests
Reviewer's Checklist