Use automatic Gradle daemon instrumentation with CI Visibility instead of manual test tasks instrumentation by nikita-tkachenko-datadog · Pull Request #1804 · DataDog/dd-sdk-android

nikita-tkachenko-datadog · 2024-01-02T15:40:59Z

What does this PR do?

Updates CI Visibility instrumentation: rather than configuring each test task to fork a JVM with the DD Java tracer injected, now the tracer is injected into the Gradle daemon that runs the build.

Motivation

Instrumenting the Gradle daemon has the following advantages:

No need to manually change the build files, every test task is instrumented automatically.
All tests executed within a single build are reported as part of a single test session, and it is possible to see the duration and status for the entire build or individual modules.
Some background operations (such as reporting Git data or building an index of the repository) are done once for the entire build and not for every test task.

Additional Notes

Review checklist (to be filled by reviewers)

Feature or bugfix MUST have appropriate tests (unit, integration, e2e)
Make sure you discussed the feature or bugfix with the maintaining team in an Issue
Make sure each commit and the PR mention the Issue number (cf the CONTRIBUTING doc)

codecov-commenter · 2024-01-02T17:45:05Z

Codecov Report

Merging #1804 (afe5bd6) into develop (00fe2ad) will increase coverage by 0.17%.
The diff coverage is n/a.

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #1804      +/-   ##
===========================================
+ Coverage    83.31%   83.48%   +0.17%     
===========================================
  Files          466      466              
  Lines        16365    16374       +9     
  Branches      2453     2455       +2     
===========================================
+ Hits         13633    13669      +36     
+ Misses        2056     2039      -17     
+ Partials       676      666      -10

see 26 files with indirect coverage changes

nikita-tkachenko-datadog · 2024-01-02T19:53:02Z

Not sure why the agent Jar was stored directly in the repo - I updated the Docker image to download the agent from Maven. If there's a reason it had to be done that particular way, I'm happy to revert this part of the changes

nikita-tkachenko-datadog · 2024-01-02T19:56:35Z

  script:
    - rm -rf ~/.gradle/daemon/
    - export DD_AGENT_HOST="$BUILDENV_HOST_IP"
-    - GRADLE_OPTS="-Xmx3072m" ./gradlew :unitTestDebug --stacktrace --no-daemon --build-cache --gradle-user-home cache/


I added DD_ENV, DD_TAGS and other config params as agent properties rather than env vars because previously they were only set for the test tasks. The idea was to err on the side of caution, since I was not sure what the implications of setting them for the entire pipeline will be.

If there are no other jobs that need their own DD_ENV and DD_TAGS, then I'd rather move these settings to the variables section.

mariusc83 · 2024-01-03T09:50:07Z

 RUN npm install -g @datadog/datadog-ci
+
+# Install Datadog Java tracer
+ENV DD_TRACER_FOLDER $PWD/dd-java-tracer


To not make a confusion here with the dd-java-tracer module that we are using for APM in our code now I would change the name of this folder to dd-hava-agent. What do you think @0xnm

I'm fine with any, it won't be inside a project folder anyway (it will be inside the entry directory of the docker container).

0xnm · 2024-01-03T13:08:34Z

-  DD_ENV_TESTS: "ci"
-  DD_INTEGRATION_JUNIT_5_ENABLED: "true"
  DD_CIVISIBILITY_ENABLED: "true"
-  DD_INSIDE_CI: "true"


I would propose to remove only DD_INTEGRATION_JUNIT_5_ENABLED variable, other two seems not to be related to the change made.

Sure, reverted the extra changes

0xnm · 2024-01-03T13:11:26Z

    - rm -rf ~/.gradle/daemon/
    - export DD_AGENT_HOST="$BUILDENV_HOST_IP"
-    - GRADLE_OPTS="-Xmx3072m" ./gradlew :unitTestDebug --stacktrace --no-daemon --build-cache --gradle-user-home cache/
+    - GRADLE_OPTS="-Xmx3072m" ./gradlew :unitTestDebug --stacktrace --no-daemon --build-cache --gradle-user-home cache/ -Dorg.gradle.jvmargs=-javaagent:$DD_TRACER_FOLDER/dd-java-agent.jar=dd.env=ci,dd.trace.enabled=false,dd.jmx.fetch.enabled=false,dd.tags=test.configuration.variant:debug


maybe we can extract the common part of agent arguments inside a variable to be stored in the variables section do avoid the repetition. Ex. DD_COMMON_AGENT_CONFIG="-Dorg.gradle.jvmargs=-javaagent:$DD_TRACER_FOLDER/dd-java-agent.jar=dd.env=ci,dd.trace.enabled=false,dd.jmx.fetch.enabled=false", so that the line looks like:

Suggested change

- GRADLE_OPTS="-Xmx3072m" ./gradlew :unitTestDebug --stacktrace --no-daemon --build-cache --gradle-user-home cache/ -Dorg.gradle.jvmargs=-javaagent:$DD_TRACER_FOLDER/dd-java-agent.jar=dd.env=ci,dd.trace.enabled=false,dd.jmx.fetch.enabled=false,dd.tags=test.configuration.variant:debug

- GRADLE_OPTS="-Xmx3072m" ./gradlew :unitTestDebug --stacktrace --no-daemon --build-cache --gradle-user-home cache/ -D$DD_COMMON_AGENT_CONFIG -Ddd.tags=test.configuration.variant:debug

Makes sense, done

That didn't seem to work, I think because of referencing an env var defined inside the Docker image from an env var defined inside the pipeline. So I had to change it slightly, to only have the actual agent args inside the "common config" variable.

0xnm · 2024-01-03T13:13:59Z

 RUN npm install -g @datadog/datadog-ci
+
+# Install Datadog Java tracer
+ENV DD_TRACER_FOLDER $PWD/dd-java-tracer


I'm fine with any, it won't be inside a project folder anyway (it will be inside the entry directory of the docker container).

0xnm · 2024-01-03T13:15:39Z

+# Install Datadog Java tracer
+ENV DD_TRACER_FOLDER $PWD/dd-java-tracer
+RUN mkdir -p $DD_TRACER_FOLDER
+RUN wget -O $DD_TRACER_FOLDER/dd-java-agent.jar 'https://repo1.maven.org/maven2/com/datadoghq/dd-java-agent/1.26.1/dd-java-agent-1.26.1.jar'


Can you please extract the version of the agent inside the env variable in the block above (around L27-L32)? So that we have a single place to manage the versions.

0xnm · 2024-01-03T15:07:50Z

-            // set the `env` tag for the test spans
-            environment("DD_ENV", "ci")
-            // add custom tags based on the module and variant (debug/release, flavors, …)
-            environment("DD_TAGS", "test.module:$moduleName,test.variant:$variant")


It seems that we are losing these tags? I can see that test.configuration.variant was added in the gitlab-ci.yml file, but the name of the tag is different, and test.module tag is completely missing. Are these 2 added automatically by the agent by some chance?

Yes, the module tag is added automatically, its value is slightly different though (the path of the Gradle task).

I changed the name of the variant tag to make it a part of the test configuration - that way the same test executed via a different task will be considered a different test case in the CI Vis backend. I wasn't exactly sure if this is a possible case - if not, I can revert the tag to its original name.

ok, clear enough, let's try to have these changes then.

nikita-tkachenko-datadog · 2024-01-03T15:44:02Z

/merge

dd-devflow · 2024-01-03T15:44:07Z

🚂 MergeQueue

This merge request is not mergeable yet, because of pending checks/missing approvals. It will be added to the queue as soon as checks pass and/or get approvals.
Note: if you pushed new commits since the last approval, you may need additional approval.
You can remove it from the waiting list with /remove command.

you can cancel this operation by commenting your pull request with /merge -c!

…d of manual test tasks instrumentation

dd-devflow · 2024-01-03T16:59:13Z

🚂 MergeQueue

Added to the queue.

This build is going to start soon! (estimated merge in less than 0s)

you can cancel this operation by commenting your pull request with /merge -c!

nikita-tkachenko-datadog force-pushed the nikita-tkachenko/ci-visibility-update branch from 1221ba6 to 44138b1 Compare January 2, 2024 19:51

nikita-tkachenko-datadog commented Jan 2, 2024

View reviewed changes

mariusc83 reviewed Jan 3, 2024

View reviewed changes

Base automatically changed from nogorodnikov/move-setting-dd-agent-host-to-stage to develop January 3, 2024 09:55

nikita-tkachenko-datadog marked this pull request as ready for review January 3, 2024 10:11

nikita-tkachenko-datadog requested review from a team as code owners January 3, 2024 10:11

nikita-tkachenko-datadog requested a review from 0xnm January 3, 2024 10:11

0xnm reviewed Jan 3, 2024

View reviewed changes

nikita-tkachenko-datadog force-pushed the nikita-tkachenko/ci-visibility-update branch from 44138b1 to 880c266 Compare January 3, 2024 14:21

0xnm reviewed Jan 3, 2024

View reviewed changes

0xnm approved these changes Jan 3, 2024

View reviewed changes

dd-devflow Bot added the mergequeue-status: waiting label Jan 3, 2024

Use automatic Gradle daemon instrumentation with CI Visibility instea…

afe5bd6

…d of manual test tasks instrumentation

nikita-tkachenko-datadog force-pushed the nikita-tkachenko/ci-visibility-update branch from a20e6fd to afe5bd6 Compare January 3, 2024 16:32

dd-devflow Bot added mergequeue-status: queued and removed mergequeue-status: waiting labels Jan 3, 2024

dd-devflow Bot added mergequeue-status: in_progress and removed mergequeue-status: queued labels Jan 3, 2024

dd-mergequeue Bot merged commit a21c278 into develop Jan 3, 2024

dd-mergequeue Bot deleted the nikita-tkachenko/ci-visibility-update branch January 3, 2024 17:42

dd-devflow Bot added mergequeue-status: done and removed mergequeue-status: in_progress labels Jan 3, 2024

xgouchet added this to the 2.5.0 milestone Feb 19, 2024

	- GRADLE_OPTS="-Xmx3072m" ./gradlew :unitTestDebug --stacktrace --no-daemon --build-cache --gradle-user-home cache/ -Dorg.gradle.jvmargs=-javaagent:$DD_TRACER_FOLDER/dd-java-agent.jar=dd.env=ci,dd.trace.enabled=false,dd.jmx.fetch.enabled=false,dd.tags=test.configuration.variant:debug
	- GRADLE_OPTS="-Xmx3072m" ./gradlew :unitTestDebug --stacktrace --no-daemon --build-cache --gradle-user-home cache/ -D$DD_COMMON_AGENT_CONFIG -Ddd.tags=test.configuration.variant:debug

Uh oh!

Conversation

nikita-tkachenko-datadog commented Jan 2, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What does this PR do?

Motivation

Additional Notes

Review checklist (to be filled by reviewers)

Uh oh!

codecov-commenter commented Jan 2, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nikita-tkachenko-datadog commented Jan 3, 2024

Uh oh!

dd-devflow Bot commented Jan 3, 2024

Uh oh!

dd-devflow Bot commented Jan 3, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

nikita-tkachenko-datadog commented Jan 2, 2024 •

edited

Loading

codecov-commenter commented Jan 2, 2024 •

edited

Loading