Skip to content

Commit

Permalink
Merge pull request #1619 from DataDog/tyler/spotless
Browse files Browse the repository at this point in the history
  • Loading branch information
tylerbenson committed Jun 23, 2020
2 parents 10abf11 + 945ec67 commit 391c60c
Show file tree
Hide file tree
Showing 54 changed files with 481 additions and 271 deletions.
14 changes: 14 additions & 0 deletions .githooks/pre-commit
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
#!/bin/bash
# http:https://redsymbol.net/articles/unofficial-bash-strict-mode/
set -euo pipefail
IFS=$'\n\t'

if ! ./gradlew spotlessCheck; then
./gradlew spotlessApply
echo ""
echo ""
echo -e "\033[0;33mCode has been formatted; please git diff/add and recommit."
echo ""
echo ""
exit 1
fi
31 changes: 26 additions & 5 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,17 +9,38 @@ To build the full project from the command line you need to have JDK versions fo

In contrast to the [IntelliJ IDEA setup](#intellij-idea) the default JVM to build and run tests from the command line should be Java 8.

## Code Style
# Automatic code formatting

This project includes a `.editorconfig` file for basic editor settings. This file is supported by most common text editors.

Java files must be formatted using [google-java-format](https://github.com/google/google-java-format). Please run the following task to ensure files are formatted before committing:
We have automatic code formatting enabled in Gradle configuration using [Spotless](https://github.com/diffplug/spotless)
[Gradle plugin](https://github.com/diffplug/spotless/tree/master/plugin-gradle).
Main goal is to avoid extensive reformatting caused by different IDEs having different opinion about how things should
be formatted by establishing single 'point of truth'.

```shell
./gradlew googleJavaFormat
Running

```bash
./gradlew spotlessApply
```

reformats all the files that need reformatting.

Running

```bash
./gradlew spotlessCheck
```

Other source files (Groovy, Scala, etc) should ideally be formatted by Intellij Idea's default formatting, but are not enforced.
runs formatting verify task only.

## Pre-commit hook

There is a pre-commit hook setup to verify formatting before committing. It can be activated with this command:

```bash
git config core.hooksPath .githooks
```

## Intellij IDEA

Expand Down
1 change: 1 addition & 0 deletions buildSrc/build.gradle.kts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
plugins {
groovy
`java-gradle-plugin`
id("com.diffplug.gradle.spotless") version "4.3.0"
}

gradlePlugin {
Expand Down
2 changes: 1 addition & 1 deletion dd-java-agent/README.md
Original file line number Diff line number Diff line change
@@ -1 +1 @@
# Datadog Java Agent for APM
# Datadog Java Agent for APM
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
Jar file has a structure similar to the agent jar:

```bash
jar -tf testjar
jar -tf testjar
META-INF/
META-INF/MANIFEST.MF
parent/
Expand Down
1 change: 0 additions & 1 deletion dd-java-agent/agent-jmxfetch/copy-metric-configs.sh
Original file line number Diff line number Diff line change
Expand Up @@ -51,4 +51,3 @@ for input_file in $metrics_files ; do
output_file="$build_resources_output_directory/metricconfigs/$output_file.yaml"
cp $input_file $output_file
done

Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,10 @@ dependencies {
}

/*
Setup here is as following:
* We compile with Java11 compiler to get JFR definitions.
* We specify source/target as Java8 to get code that is loadable on Java8 - JFR defs are Java8 compatible.
* We force IDEA to treat this as Java11 project with 'idea' plugin below.
Setup here is as following:
* We compile with Java11 compiler to get JFR definitions.
* We specify source/target as Java8 to get code that is loadable on Java8 - JFR defs are Java8 compatible.
* We force IDEA to treat this as Java11 project with 'idea' plugin below.
*/
sourceCompatibility = JavaVersion.VERSION_1_8
targetCompatibility = JavaVersion.VERSION_1_8
Expand All @@ -36,7 +36,9 @@ targetCompatibility = JavaVersion.VERSION_1_8
// Disable '-processing' because some annotations are not claimed.
// Disable '-options' because we are compiling for java8 without specifying bootstrap - intentionally.
// Disable '-path' because we do not have some of the paths seem to be missing.
options.compilerArgs.addAll(['-Xlint:all,-processing,-options,-path'/*, '-Werror'*/])
options.compilerArgs.addAll([
'-Xlint:all,-processing,-options,-path'/*, '-Werror'*/
])
options.fork = true
options.forkOptions.javaHome = file(System.env.JAVA_11_HOME)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,7 @@ public static Multimap<String, Object> parseProfilingRequestParameters(
}

public static Map<String, String> parseTags(final Collection<Object> params) {
return params
.stream()
return params.stream()
.map(p -> ((String) p).split(":", 2))
.collect(Collectors.toMap(p -> p[0], p -> p[1]));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -376,8 +376,7 @@ private boolean canEnqueueMoreRequests() {
}

private List<String> tagsToList(final Map<String, String> tags) {
return tags.entrySet()
.stream()
return tags.entrySet().stream()
.filter(e -> e.getValue() != null && !e.getValue().isEmpty())
.map(e -> e.getKey() + ":" + e.getValue())
.collect(Collectors.toList());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,6 @@ dependencies {

jar {
manifest {
attributes(
"Main-Class": "datadog.perftest.jetty.JettyPerftest"
)
attributes("Main-Class": "datadog.perftest.jetty.JettyPerftest")
}
}
29 changes: 16 additions & 13 deletions dd-java-agent/benchmark/benchmark.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -15,23 +15,26 @@ jmh {
timeOnIteration = '20s'
iterations = 1 // Number of measurement iterations to do.
fork = 1 // How many times to forks a single benchmark. Use 0 to disable forking altogether
jvmArgs = ["-Ddd.jmxfetch.enabled=false", "-Ddd.writer.type=LoggingWriter"]
// jvmArgs += ["-XX:+UnlockDiagnosticVMOptions", "-XX:+DebugNonSafepoints", "-XX:StartFlightRecording=delay=5s,dumponexit=true,name=jmh-benchmark,filename=$rootDir/dd-java-agent/benchmark/build/reports/jmh/jmh-benchmark.jfr"]
// jvmArgs += ["-agentpath:$rootDir/dd-java-agent/benchmark/src/jmh/resources/libasyncProfiler.so=start,collapsed,file=$rootDir/dd-java-agent/benchmark/build/reports/jmh/profiler.txt".toString()]
jvmArgs = [
"-Ddd.jmxfetch.enabled=false",
"-Ddd.writer.type=LoggingWriter"
]
// jvmArgs += ["-XX:+UnlockDiagnosticVMOptions", "-XX:+DebugNonSafepoints", "-XX:StartFlightRecording=delay=5s,dumponexit=true,name=jmh-benchmark,filename=$rootDir/dd-java-agent/benchmark/build/reports/jmh/jmh-benchmark.jfr"]
// jvmArgs += ["-agentpath:$rootDir/dd-java-agent/benchmark/src/jmh/resources/libasyncProfiler.so=start,collapsed,file=$rootDir/dd-java-agent/benchmark/build/reports/jmh/profiler.txt".toString()]
failOnError = true // Should JMH fail immediately if any benchmark had experienced the unrecoverable error?
warmup = '5s' // Time to spend at each warmup iteration.
// warmupBatchSize = 10 // Warmup batch size: number of benchmark method calls per operation.
// warmupBatchSize = 10 // Warmup batch size: number of benchmark method calls per operation.
warmupForks = 0 // How many warmup forks to make for a single benchmark. 0 to disable warmup forks.
warmupIterations = 1 // Number of warmup iterations to do.

// profilers = ['stack:lines=5;detailLine=true;period=5;excludePackages=true']
// profilers = ['stack:lines=5;detailLine=true;period=5;excludePackages=true']
// Use profilers to collect additional data. Supported profilers: [cl, comp, gc, stack, perf, perfnorm, perfasm, xperf, xperfasm, hs_cl, hs_comp, hs_gc, hs_rt, hs_thr]

// humanOutputFile = project.file("${project.buildDir}/reports/jmh/human.txt") // human-readable output file
// operationsPerInvocation = 10 // Operations per invocation.
// synchronizeIterations = false // Synchronize iterations?
// humanOutputFile = project.file("${project.buildDir}/reports/jmh/human.txt") // human-readable output file
// operationsPerInvocation = 10 // Operations per invocation.
// synchronizeIterations = false // Synchronize iterations?
timeout = '5s' // Timeout for benchmark iteration.
// includeTests = false
// includeTests = false
// Allows to include test sources into generate JMH jar, i.e. use it when benchmarks depend on the test classes.

duplicateClassesStrategy = DuplicatesStrategy.EXCLUDE
Expand All @@ -41,9 +44,9 @@ jmh {
tasks.jmh.dependsOn project(':dd-java-agent').shadowJar

/*
If using libasyncProfiler, use the following to generate nice svg flamegraphs.
sed '/unknown/d' dd-java-agent/benchmark/build/reports/jmh/profiler.txt | sed '/^thread_start/d' | sed '/not_walkable/d' > dd-java-agent/benchmark/build/reports/jmh/profiler-cleaned.txt
(using https://github.com/brendangregg/FlameGraph)
./flamegraph.pl --color=java dd-java-agent/benchmark/build/reports/jmh/profiler-cleaned.txt > dd-java-agent/benchmark/build/reports/jmh/jmh-master.svg
If using libasyncProfiler, use the following to generate nice svg flamegraphs.
sed '/unknown/d' dd-java-agent/benchmark/build/reports/jmh/profiler.txt | sed '/^thread_start/d' | sed '/not_walkable/d' > dd-java-agent/benchmark/build/reports/jmh/profiler-cleaned.txt
(using https://github.com/brendangregg/FlameGraph)
./flamegraph.pl --color=java dd-java-agent/benchmark/build/reports/jmh/profiler-cleaned.txt > dd-java-agent/benchmark/build/reports/jmh/jmh-master.svg
*/

2 changes: 1 addition & 1 deletion dd-java-agent/dd-java-agent.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ shadowJar generalShadowJarConfig >> {
"Premain-Class": "datadog.trace.bootstrap.AgentBootstrap",
"Can-Redefine-Classes": true,
"Can-Retransform-Classes": true,
)
)
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,20 +18,27 @@ object AkkaHttpTestAsyncWebServer {
val asyncHandler: HttpRequest => Future[HttpResponse] = {
case HttpRequest(GET, uri: Uri, _, _, _) =>
Future {
val endpoint = HttpServerTest.ServerEndpoint.forPath(uri.path.toString())
HttpServerTest.controller(endpoint, new Closure[HttpResponse](()) {
def doCall(): HttpResponse = {
val resp = HttpResponse(status = endpoint.getStatus) //.withHeaders(headers.Type)resp.contentType = "text/plain"
endpoint match {
case SUCCESS => resp.withEntity(endpoint.getBody)
case QUERY_PARAM => resp.withEntity(uri.queryString().orNull)
case REDIRECT => resp.withHeaders(headers.Location(endpoint.getBody))
case ERROR => resp.withEntity(endpoint.getBody)
case EXCEPTION => throw new Exception(endpoint.getBody)
case _ => HttpResponse(status = NOT_FOUND.getStatus).withEntity(NOT_FOUND.getBody)
val endpoint =
HttpServerTest.ServerEndpoint.forPath(uri.path.toString())
HttpServerTest.controller(
endpoint,
new Closure[HttpResponse](()) {
def doCall(): HttpResponse = {
val resp = HttpResponse(status = endpoint.getStatus) //.withHeaders(headers.Type)resp.contentType = "text/plain"
endpoint match {
case SUCCESS => resp.withEntity(endpoint.getBody)
case QUERY_PARAM => resp.withEntity(uri.queryString().orNull)
case REDIRECT =>
resp.withHeaders(headers.Location(endpoint.getBody))
case ERROR => resp.withEntity(endpoint.getBody)
case EXCEPTION => throw new Exception(endpoint.getBody)
case _ =>
HttpResponse(status = NOT_FOUND.getStatus)
.withEntity(NOT_FOUND.getBody)
}
}
}
})
)
}
}

Expand All @@ -40,7 +47,10 @@ object AkkaHttpTestAsyncWebServer {
def start(port: Int): Unit = synchronized {
if (null == binding) {
import scala.concurrent.duration._
binding = Await.result(Http().bindAndHandleAsync(asyncHandler, "localhost", port), 10 seconds)
binding = Await.result(
Http().bindAndHandleAsync(asyncHandler, "localhost", port),
10 seconds
)
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,19 +18,25 @@ object AkkaHttpTestSyncWebServer {
val syncHandler: HttpRequest => HttpResponse = {
case HttpRequest(GET, uri: Uri, _, _, _) => {
val endpoint = HttpServerTest.ServerEndpoint.forPath(uri.path.toString())
HttpServerTest.controller(endpoint, new Closure[HttpResponse](()) {
def doCall(): HttpResponse = {
val resp = HttpResponse(status = endpoint.getStatus)
endpoint match {
case SUCCESS => resp.withEntity(endpoint.getBody)
case QUERY_PARAM => resp.withEntity(uri.queryString().orNull)
case REDIRECT => resp.withHeaders(headers.Location(endpoint.getBody))
case ERROR => resp.withEntity(endpoint.getBody)
case EXCEPTION => throw new Exception(endpoint.getBody)
case _ => HttpResponse(status = NOT_FOUND.getStatus).withEntity(NOT_FOUND.getBody)
HttpServerTest.controller(
endpoint,
new Closure[HttpResponse](()) {
def doCall(): HttpResponse = {
val resp = HttpResponse(status = endpoint.getStatus)
endpoint match {
case SUCCESS => resp.withEntity(endpoint.getBody)
case QUERY_PARAM => resp.withEntity(uri.queryString().orNull)
case REDIRECT =>
resp.withHeaders(headers.Location(endpoint.getBody))
case ERROR => resp.withEntity(endpoint.getBody)
case EXCEPTION => throw new Exception(endpoint.getBody)
case _ =>
HttpResponse(status = NOT_FOUND.getStatus)
.withEntity(NOT_FOUND.getBody)
}
}
}
})
)
}
}

Expand All @@ -39,7 +45,10 @@ object AkkaHttpTestSyncWebServer {
def start(port: Int): Unit = synchronized {
if (null == binding) {
import scala.concurrent.duration._
binding = Await.result(Http().bindAndHandleSync(syncHandler, "localhost", port), 10 seconds)
binding = Await.result(
Http().bindAndHandleSync(syncHandler, "localhost", port),
10 seconds
)
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,21 @@ object AkkaHttpTestWebServer {
implicit val executionContext = system.dispatcher

val exceptionHandler = ExceptionHandler {
case ex: Exception => complete(HttpResponse(status = EXCEPTION.getStatus).withEntity(ex.getMessage))
case ex: Exception =>
complete(
HttpResponse(status = EXCEPTION.getStatus).withEntity(ex.getMessage)
)
}

val route = { //handleExceptions(exceptionHandler) {
path(SUCCESS.rawPath) {
complete(HttpResponse(status = SUCCESS.getStatus).withEntity(SUCCESS.getBody))
complete(
HttpResponse(status = SUCCESS.getStatus).withEntity(SUCCESS.getBody)
)
} ~ path(QUERY_PARAM.rawPath) {
complete(HttpResponse(status = QUERY_PARAM.getStatus).withEntity(SUCCESS.getBody))
complete(
HttpResponse(status = QUERY_PARAM.getStatus).withEntity(SUCCESS.getBody)
)
} ~ path(REDIRECT.rawPath) {
redirect(Uri(REDIRECT.getBody), StatusCodes.Found)
} ~ path(ERROR.rawPath) {
Expand All @@ -39,7 +46,8 @@ object AkkaHttpTestWebServer {
def start(port: Int): Unit = synchronized {
if (null == binding) {
import scala.concurrent.duration._
binding = Await.result(Http().bindAndHandle(route, "localhost", port), 10 seconds)
binding =
Await.result(Http().bindAndHandle(route, "localhost", port), 10 seconds)
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ testSets {

dependencies {
compileOnly group: 'org.apache.httpcomponents', name: 'httpasyncclient', version: '4.0'

testCompile group: 'org.apache.httpcomponents', name: 'httpasyncclient', version: '4.0'

latestDepTestCompile group: 'org.apache.httpcomponents', name: 'httpasyncclient', version: '+'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,8 @@

import datadog.trace.bootstrap.instrumentation.api.AgentScope;
import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
import java.util.function.Consumer;

import datadog.trace.bootstrap.instrumentation.api.InstrumentationTags;
import java.util.function.Consumer;
import software.amazon.awssdk.core.client.builder.SdkClientBuilder;
import software.amazon.awssdk.core.client.config.ClientOverrideConfiguration;
import software.amazon.awssdk.core.interceptor.Context;
Expand Down
2 changes: 1 addition & 1 deletion dd-java-agent/instrumentation/dropwizard/dropwizard.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -18,5 +18,5 @@ dependencies {
testCompile group: 'com.fasterxml.jackson.module', name: 'jackson-module-afterburner', version: '2.9.10'

// Anything 1.0+ fails with a java.lang.NoClassDefFoundError: org/eclipse/jetty/server/RequestLog
// latestDepTestCompile group: 'io.dropwizard', name: 'dropwizard-testing', version: '1.+'
// latestDepTestCompile group: 'io.dropwizard', name: 'dropwizard-testing', version: '1.+'
}
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,13 @@ targetCompatibility = JavaVersion.VERSION_1_7
// Disable '-options' because we are compiling for java8 without specifying bootstrap - intentionally.
// Disable '-path' because we do not have some of the paths seem to be missing.
// Compile to 8 compatible byte code
options.compilerArgs.addAll(['-source', '8', '-target', '8', '-Xlint:all,-processing,-options,-path'])
options.compilerArgs.addAll([
'-source',
'8',
'-target',
'8',
'-Xlint:all,-processing,-options,-path'
])
options.fork = true
options.forkOptions.javaHome = file(System.env.JAVA_11_HOME)
}
Expand Down
Loading

0 comments on commit 391c60c

Please sign in to comment.