documentation icon indicating copy to clipboard operation
documentation copied to clipboard

Update CI Visibility Java/Gradle code example

Open erodewald opened this issue 3 years ago • 1 comments

What does this PR do?

The example provided is destructive to existing JVM args fed prior to this gradle script being evaluated. For example, if a root project script adds --enable-preview (arbitrary flag example) to the JVM args, the current Gradle example script will overwrite it with the user not realizing it until their CI tests run and behave aberrantly.

Motivation

My CI tests broke when I was using Java 17 experimental features using the aforementioned JVM flag, but I didn't figure that out until later.

Additional Notes

This is super easy to test and it worked really well for me, but beware that the Gradle script I provided is only checked against Groovy Gradle 7+. I am unaware of what language or Gradle version your documentation targets.

I encourage you to not believe me, run this yourselves to check my math. 😄


Reviewer checklist

  • [x] Review the changed files.
  • [ ] Review the URLs listed in the Preview section.
  • [ ] Check images for PII
  • [x] Review any mentions of "Contact Datadog support" for internal support documentation.

erodewald avatar Oct 10 '22 21:10 erodewald

Thanks @erodewald for opening this PR! We're reviewing it and will get back to you.

maycmlee avatar Oct 11 '22 19:10 maycmlee

Thanks @erodewald for your PR

Unfortunately, this approach does not seem to work in Java 1.8 :( However, I think we can obtain the same behavior by just changing the = to += in the jvmArgs = line.

Something like:

jvmArgs += ["-javaagent:${configurations.ddTracerAgent.asPath}", "-Ddd.service=my-java-app", "-Ddd.civisibility.enabled=true"]

I tested it using this code snippet and it seems to work:

    jvmArgs += ["--enable-preview"]
    if(project.hasProperty("dd-civisibility")) {
        jvmArgs += ["-javaagent:${configurations.ddTracerAgent.asPath}"]
        jvmArgs += ["-Ddd.service=my-app"]
        jvmArgs += ["-Ddd.civisibility.enabled=true"]
    }

This approach works for every JVM version.

Let me know what you think!

drodriguezhdez avatar Nov 10 '22 09:11 drodriguezhdez

Hello, I'm going to close this PR as it is stale and now showing a merge conflict. If you'd like to implement the changes suggested above by Daniel, please feel free to open up a new PR in the future!

jtappa avatar Dec 20 '22 21:12 jtappa