okhttp3: pins main version to 4.x and tests 3.x in invoker (#223)

See #205 (comment) for discussion Signed-off-by: Adrian Cole <adrian@tetrate.io>
openzipkin · Dec 14, 2023 · 70d7855 · 70d7855
1 parent 963caa4
commit 70d7855
Show file tree

Hide file tree

Showing 12 changed files with 264 additions and 10 deletions.
diff --git a/README.md b/README.md
@@ -181,7 +181,7 @@ Ex. in your dependencies section, import the BOM like this:
 ```
 
 Now, you can leave off the version when choosing any supported
-instrumentation. Also any indirect use will have versions aligned:
+instrumentation. Also, any indirect use will have versions aligned:
 ```xml
 <dependency>
   <groupId>io.zipkin.reporter2</groupId>

diff --git a/okhttp3/README.md b/okhttp3/README.md
@@ -0,0 +1,13 @@
+# zipkin-sender-okhttp3
+This module contains a span sender for [OkHttp](https://github.com/square/okhttp) 3.x.
+
+Please view [OkHttpSender](src/main/java/zipkin2/reporter/okhttp3/OkHttpSender.java)
+for usage details.
+
+## Compatability
+
+In order to keep new users current, this assigns the most recent major version
+of OkHttp 4.x. This reduces distraction around CVEs at the cost of more
+dependencies, notably Kotlin, vs the 3.x versions. That said, this module is
+tested to be runtime compatible with 3.x, for users who need to support Java 7
+or non-Kotlin use cases.
diff --git a/okhttp3/pom.xml b/okhttp3/pom.xml
@@ -31,6 +31,8 @@
     <module.name>zipkin2.reporter.okhttp3</module.name>
 
     <main.basedir>${project.basedir}/..</main.basedir>
+    <!-- pre-1.8, pre-kotlin, and last version before Call.timeout. -->
+    <old-okhttp.version>3.11.0</old-okhttp.version>
   </properties>
 
   <dependencies>
@@ -62,6 +64,37 @@
     </dependency>
   </dependencies>
 
+  <build>
+    <plugins>
+      <plugin>
+        <artifactId>maven-invoker-plugin</artifactId>
+      </plugin>
+      <plugin>
+        <groupId>de.qaware.maven</groupId>
+        <artifactId>go-offline-maven-plugin</artifactId>
+        <executions>
+          <execution>
+            <phase>package</phase>
+            <goals>
+              <goal>resolve-dependencies</goal>
+            </goals>
+          </execution>
+        </executions>
+        <configuration>
+          <!-- Add dependencies specific to invoker tests so that they cache on go-offline -->
+          <dynamicDependencies>
+            <DynamicDependency>
+              <groupId>com.squareup.okhttp3</groupId>
+              <artifactId>mockwebserver</artifactId>
+              <version>${old-okhttp.version}</version>
+              <repositoryType>MAIN</repositoryType>
+            </DynamicDependency>
+          </dynamicDependencies>
+        </configuration>
+      </plugin>
+    </plugins>
+  </build>
+
   <profiles>
     <profile>
       <id>release</id>

diff --git a/okhttp3/src/it/okhttp3_v3/README.md b/okhttp3/src/it/okhttp3_v3/README.md
@@ -0,0 +1,2 @@
+# okhttp3_v3
+This tests that OkHttpSender can be used with okhttp3 <3.12
diff --git a/okhttp3/src/it/okhttp3_v3/pom.xml b/okhttp3/src/it/okhttp3_v3/pom.xml
@@ -0,0 +1,95 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+
+    Copyright 2016-2023 The OpenZipkin Authors
+
+    Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except
+    in compliance with the License. You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+    Unless required by applicable law or agreed to in writing, software distributed under the License
+    is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
+    or implied. See the License for the specific language governing permissions and limitations under
+    the License.
+
+-->
+<project xmlns="http://maven.apache.org/POM/4.0.0"
+    xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+    xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+  <modelVersion>4.0.0</modelVersion>
+
+  <groupId>@project.groupId@</groupId>
+  <artifactId>okhttp3_v3</artifactId>
+  <version>@project.version@</version>
+  <name>okhttp3_v3</name>
+
+  <properties>
+    <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
+    <project.build.resourceEncoding>UTF-8</project.build.resourceEncoding>
+
+    <maven.compiler.source>1.8</maven.compiler.source>
+    <maven.compiler.target>1.8</maven.compiler.target>
+  </properties>
+
+  <dependencies>
+    <dependency>
+      <groupId>@project.groupId@</groupId>
+      <artifactId>zipkin-sender-okhttp3</artifactId>
+      <version>@project.version@</version>
+      <exclusions>
+        <exclusion>
+          <groupId>com.squareup.okhttp3</groupId>
+          <artifactId>*</artifactId>
+        </exclusion>
+      </exclusions>
+    </dependency>
+
+    <dependency>
+      <groupId>org.junit.jupiter</groupId>
+      <artifactId>junit-jupiter</artifactId>
+      <version>@junit-jupiter.version@</version>
+    </dependency>
+
+    <dependency>
+      <groupId>org.assertj</groupId>
+      <artifactId>assertj-core</artifactId>
+      <version>@assertj.version@</version>
+    </dependency>
+
+    <dependency>
+      <groupId>com.squareup.okhttp3</groupId>
+      <artifactId>mockwebserver</artifactId>
+      <version>@old-okhttp.version@</version>
+    </dependency>
+
+    <dependency>
+      <groupId>io.zipkin.zipkin2</groupId>
+      <artifactId>zipkin-tests</artifactId>
+      <version>@zipkin.version@</version>
+    </dependency>
+  </dependencies>
+
+  <build>
+    <sourceDirectory>@project.build.testSourceDirectory@</sourceDirectory>
+    <plugins>
+      <plugin>
+        <artifactId>maven-compiler-plugin</artifactId>
+        <version>@maven-compiler-plugin.version@</version>
+        <configuration>
+          <includes>
+            <include>**/*Test.java</include>
+          </includes>
+        </configuration>
+      </plugin>
+      <plugin>
+        <artifactId>maven-surefire-plugin</artifactId>
+        <version>@maven-surefire-plugin.version@</version>
+        <configuration>
+          <!-- Ensures root cause ends up in the console -->
+          <trimStackTrace>false</trimStackTrace>
+        </configuration>
+      </plugin>
+    </plugins>
+  </build>
+</project>
diff --git a/okhttp3/src/it/okhttp3_v3/src/test/java/brave/okhttp3_v3/HttpCallTest.java b/okhttp3/src/it/okhttp3_v3/src/test/java/brave/okhttp3_v3/HttpCallTest.java
@@ -0,0 +1,17 @@
+/*
+ * Copyright 2016-2023 The OpenZipkin Authors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except
+ * in compliance with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
+ * or implied. See the License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package zipkin2.reporter.okhttp3.okhttp3_v3;
+
+public class HttpCallTest extends zipkin2.reporter.okhttp3.HttpCallTest {
+}
diff --git a/okhttp3/src/it/okhttp3_v3/src/test/java/brave/okhttp3_v3/OkHttpSenderTest.java b/okhttp3/src/it/okhttp3_v3/src/test/java/brave/okhttp3_v3/OkHttpSenderTest.java
@@ -0,0 +1,17 @@
+/*
+ * Copyright 2016-2023 The OpenZipkin Authors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except
+ * in compliance with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
+ * or implied. See the License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package zipkin2.reporter.okhttp3.okhttp3_v3;
+
+public class OkHttpSenderTest extends zipkin2.reporter.okhttp3.OkHttpSenderTest {
+}
diff --git a/okhttp3/src/test/java/zipkin2/reporter/okhttp3/HttpCallTest.java b/okhttp3/src/test/java/zipkin2/reporter/okhttp3/HttpCallTest.java
@@ -25,7 +25,7 @@
 
 import static org.assertj.core.api.Assertions.assertThat;
 
-class HttpCallTest {
+public class HttpCallTest { // public for use in src/it
   @Test void parseResponse_closesBody() throws Exception {
 
     // It is difficult to prove close was called, this approach looks at an underlying stream

diff --git a/okhttp3/src/test/java/zipkin2/reporter/okhttp3/OkHttpSenderTest.java b/okhttp3/src/test/java/zipkin2/reporter/okhttp3/OkHttpSenderTest.java
@@ -44,7 +44,7 @@
 import static org.assertj.core.api.Assertions.failBecauseExceptionWasNotThrown;
 import static zipkin2.TestObjects.CLIENT_SPAN;
 
-class OkHttpSenderTest {
+public class OkHttpSenderTest { // public for use in src/it
   MockWebServer server = new MockWebServer();
 
   @AfterEach void closeServer() throws IOException {

diff --git a/pom.xml b/pom.xml
@@ -63,9 +63,8 @@
     <brave.version>5.16.0</brave.version>
 
     <junit-jupiter.version>5.10.1</junit-jupiter.version>
-    <!-- use pre-kotlin version -->
-    <okhttp.version>3.14.9</okhttp.version>
-    <okhttp5.version>5.0.0-alpha.11</okhttp5.version>
+    <assertj.version>3.24.2</assertj.version>
+    <okhttp.version>4.12.0</okhttp.version>
     <!-- use pre slf4j 2 version -->
     <logback.version>1.2.13</logback.version>
     <testcontainers.version>1.19.3</testcontainers.version>
@@ -85,6 +84,7 @@
     <!-- Use same version as https://github.com/openzipkin/docker-java -->
     <maven-help-plugin.version>3.4.0</maven-help-plugin.version>
     <maven-install-plugin.version>3.1.1</maven-install-plugin.version>
+    <maven-invoker-plugin.version>3.6.0</maven-invoker-plugin.version>
     <maven-javadoc-plugin.version>3.6.2</maven-javadoc-plugin.version>
     <maven-jar-plugin.version>3.3.0</maven-jar-plugin.version>
     <maven-release-plugin.version>3.0.1</maven-release-plugin.version>
@@ -175,7 +175,7 @@
     <dependency>
       <groupId>org.assertj</groupId>
       <artifactId>assertj-core</artifactId>
-      <version>3.24.2</version>
+      <version>${assertj.version}</version>
       <scope>test</scope>
     </dependency>
 
@@ -319,6 +319,33 @@
           <artifactId>maven-shade-plugin</artifactId>
           <version>${maven-shade-plugin.version}</version>
         </plugin>
+
+        <plugin>
+          <artifactId>maven-invoker-plugin</artifactId>
+          <version>${maven-invoker-plugin.version}</version>
+          <configuration>
+            <cloneProjectsTo>${project.build.directory}/it</cloneProjectsTo>
+            <settingsFile>${main.basedir}/src/it/settings.xml</settingsFile>
+            <profiles>
+              <profile>!release</profile>
+            </profiles>
+            <localRepositoryPath>${project.build.directory}/local-repo</localRepositoryPath>
+            <postBuildHookScript>verify</postBuildHookScript>
+            <addTestClassPath>true</addTestClassPath>
+            <skipInvocation>${skipTests}</skipInvocation>
+            <!-- don't log when "downloading" from local repo -->
+            <streamLogs>true</streamLogs>
+            <mavenOpts>-Dorg.slf4j.simpleLogger.defaultLogLevel=WARN</mavenOpts>
+          </configuration>
+          <executions>
+            <execution>
+              <goals>
+                <goal>install</goal>
+                <goal>run</goal>
+              </goals>
+            </execution>
+          </executions>
+        </plugin>
       </plugins>
     </pluginManagement>
 

diff --git a/spring-beans/src/test/java/zipkin2/reporter/beans/OkHttpSenderFactoryBeanTest.java b/spring-beans/src/test/java/zipkin2/reporter/beans/OkHttpSenderFactoryBeanTest.java
@@ -51,7 +51,7 @@ class OkHttpSenderFactoryBeanTest {
     );
 
     assertThat(context.getBean("sender", OkHttpSender.class))
-        .extracting("client.connectTimeout")
+        .extracting("client.connectTimeoutMillis")
         .isEqualTo(1000);
   }
 
@@ -64,7 +64,7 @@ class OkHttpSenderFactoryBeanTest {
     );
 
     assertThat(context.getBean("sender", OkHttpSender.class))
-        .extracting("client.writeTimeout")
+        .extracting("client.writeTimeoutMillis")
         .isEqualTo(1000);
   }
 
@@ -77,7 +77,7 @@ class OkHttpSenderFactoryBeanTest {
     );
 
     assertThat(context.getBean("sender", OkHttpSender.class))
-        .extracting("client.readTimeout")
+        .extracting("client.readTimeoutMillis")
         .isEqualTo(1000);
   }
 

diff --git a/src/it/settings.xml b/src/it/settings.xml
@@ -0,0 +1,50 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+
+    Copyright 2016-2023 The OpenZipkin Authors
+
+    Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except
+    in compliance with the License. You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+    Unless required by applicable law or agreed to in writing, software distributed under the License
+    is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
+    or implied. See the License for the specific language governing permissions and limitations under
+    the License.
+
+-->
+<settings>
+  <profiles>
+    <profile>
+      <id>it-repo</id>
+      <activation>
+        <activeByDefault>true</activeByDefault>
+      </activation>
+      <repositories>
+        <repository>
+          <id>local.central</id>
+          <url>@localRepositoryUrl@</url>
+          <releases>
+            <enabled>true</enabled>
+          </releases>
+          <snapshots>
+            <enabled>true</enabled>
+          </snapshots>
+        </repository>
+      </repositories>
+      <pluginRepositories>
+        <pluginRepository>
+          <id>local.central</id>
+          <url>@localRepositoryUrl@</url>
+          <releases>
+            <enabled>true</enabled>
+          </releases>
+          <snapshots>
+            <enabled>true</enabled>
+          </snapshots>
+        </pluginRepository>
+      </pluginRepositories>
+    </profile>
+  </profiles>
+</settings>