Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use Spotless for formatting #1619

Merged
merged 7 commits into from
Jun 23, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
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