Add support for test.codeowners and test.source.* tags#4597
Conversation
…cessary AST nodes
nikita-tkachenko-datadog
added
the
comp: ci visibility
nikita-tkachenko-datadog
changed the title
… add codeowners entry for new CI visibility module
| public @Nullable Collection<String> getOwners(String path) { | ||
| if (!path.startsWith(repoRoot)) { | ||
| return Collections.emptyList(); | ||
| return null; |
There was a problem hiding this comment.
What was the reason using null ?
I'd rather keep empty list to avoid possible NPE.
There was a problem hiding this comment.
The idea was to use an empty list for files that have no owners (it's possible to have a CODEOWNERS entry that says file/folder is not owned by anyone), and null for the cases when owners could not be determined.
Anyway, this looks like an older version of the code - this PR has a bunch of commits that I will squash during merge - in the PR's last commit this code looks a bit different.
|
|
||
| @Nullable | ||
| @Override | ||
| public String getSourcePath(Class<?> c) { |
There was a problem hiding this comment.
I think, there is missing null checker for Class<?> c
There was a problem hiding this comment.
From my point of view an appropriate response to getSourcePath(null) would be an NPE, since this likely indicates a programming error, which is best signalled in a fail-fast manner. Swallowing a null argument might cover this sort of programming issues, making them harder to discover.
Currently this method will never be called with a null argument, as the calling code does the necessary checks.
|
@ValentinZakharov, thank you for the review! |
2 participants
What Does This Do
This PR updates CI-visibility part of the tracer, adding support for the following tags:
test.codeowners: teams/people who own the test (in accordance with theCODEOWNERSfile, if one is available in the client's repo)test.source.file: file path where the test is located, relative to the project's source roottest.source.start: the line in the test source file where the test definition startstest.source.end: the line of the source code where the test endsMotivation
These tags are part of the CI visibility tracers specification.
Populating them will allow the clients to use additional features offered by CI visibility: showing source code, notifying code owners about test failures, etc.
Additional Notes
In scope of this PR a new gradle module was created for logic that is specific to CI visibility:
dd-java-agent:agent-ci-visibility.For now, in order to keep the scope of the PR smaller, the new module only contains logic introduced to support the codeowners/source tag features.
The intention is to later move all the CI visibility logic to the new module, with only the interfaces remaining in
internal-apimodule - that way CI visibility will be consistent with the other subsystems residing in the tracer.