Fix APM Server version parsing (#2313)

Author: eyalkoren

CHANGELOG.asciidoc

@@ -33,6 +33,7 @@ endif::[]
 
 [float]
 ===== Bug fixes
+* Fix error with parsing APM Server version for 7.16+ - {pull}2313[#2313]
 
 [[release-notes-1.x]]
 === Java Agent version 1.x

apm-agent-core/src/main/java/co/elastic/apm/agent/report/ApmServerHealthChecker.java

@@ -22,16 +22,17 @@
 import co.elastic.apm.agent.util.Version;
 import com.dslplatform.json.DslJson;
 import com.dslplatform.json.JsonReader;
-import com.dslplatform.json.MapConverter;
 import com.dslplatform.json.Nullable;
 import com.dslplatform.json.ObjectConverter;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 import java.net.HttpURLConnection;
 import java.util.Collections;
+import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 import java.util.concurrent.Callable;
 import java.util.concurrent.Future;
 import java.util.concurrent.ThreadPoolExecutor;
@@ -41,8 +42,9 @@
 public class ApmServerHealthChecker implements Callable<Version> {
     private static final Logger logger = LoggerFactory.getLogger(ApmServerHealthChecker.class);
 
+    private static final DslJson<Object> dslJson = new DslJson<>(new DslJson.Settings<>());
+
     private final ApmServerClient apmServerClient;
-    private final DslJson<Object> dslJson = new DslJson<>(new DslJson.Settings<>());
 
     public ApmServerHealthChecker(ApmServerClient apmServerClient) {
         this.apmServerClient = apmServerClient;
@@ -81,24 +83,11 @@ public Version withConnection(HttpURLConnection connection) {
                             // prints out the version info of the APM Server
                             String body = HttpUtils.readToString(connection.getInputStream());
                             logger.info("Elastic APM server is available: {}", body);
-                            JsonReader<Object> reader = dslJson.newReader(body.getBytes(UTF_8));
-                            reader.startObject();
-                            String versionString;
-                            try {
-                                // newer APM server versions contain a flat map at the JSON root
-                                versionString = MapConverter.deserialize(reader).get("version");
-                            } catch (Exception e) {
-                                // 6.x APM server versions' JSON has a root object of which value is the same map
-                                reader = dslJson.newReader(body.getBytes(UTF_8));
-                                reader.startObject();
-                                Map<String, Object> root = ObjectConverter.deserializeMap(reader);
-                                //noinspection unchecked
-                                versionString = ((Map<String, String>) root.get("ok")).get("version");
-                            }
+                            Version version = parseVersion(body);
                             if (logger.isDebugEnabled()) {
-                                logger.debug("APM server {} version is: {}", connection.getURL(), versionString);
+                                logger.debug("APM server {} version is: {}", connection.getURL(), version);
                             }
-                            return Version.of(versionString);
+                            return version;
                         } catch (Exception e) {
                             logger.warn("Failed to parse version of APM server {}: {}", connection.getURL(), e.getMessage());
                         }
@@ -115,4 +104,23 @@ public Version withConnection(HttpURLConnection connection) {
         }
         return Version.UNKNOWN_VERSION;
     }
+
+    static Version parseVersion(String body) throws java.io.IOException {
+        JsonReader<Object> reader = dslJson.newReader(body.getBytes(UTF_8));
+        reader.startObject();
+        String versionString;
+        try {
+            // APM server 7.0+ contain a flat map at the JSON root
+            LinkedHashMap<String, Object> responseJsonMap = ObjectConverter.deserializeMap(reader);
+            versionString = (String) Objects.requireNonNull(responseJsonMap.get("version"));
+        } catch (Exception e) {
+            // 6.x APM server versions' JSON has a root object of which value is the same map
+            reader = dslJson.newReader(body.getBytes(UTF_8));
+            reader.startObject();
+            Map<String, Object> root = ObjectConverter.deserializeMap(reader);
+            //noinspection unchecked
+            versionString = ((Map<String, String>) root.get("ok")).get("version");
+        }
+        return Version.of(versionString);
+    }
 }

apm-agent-core/src/main/java/co/elastic/apm/agent/util/Version.java

@@ -72,4 +72,16 @@ public int compareTo(Version another) {
         }
         return 0;
     }
+
+    @Override
+    public String toString() {
+        StringBuilder sb = new StringBuilder();
+        for (int i = 0; i < numbers.length; i++) {
+            sb.append(numbers[i]);
+            if (i < numbers.length - 1) {
+                sb.append('.');
+            }
+        }
+        return sb.toString();
+    }
 }

apm-agent-core/src/test/java/co/elastic/apm/agent/report/ApmServerHealthCheckerTest.java

@@ -0,0 +1,42 @@
+/*
+ * Licensed to Elasticsearch B.V. under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Elasticsearch B.V. licenses this file to you 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 co.elastic.apm.agent.report;
+
+import co.elastic.apm.agent.util.Version;
+import org.junit.jupiter.api.Test;
+
+import java.io.IOException;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+class ApmServerHealthCheckerTest {
+
+    @Test
+    void testParseVersion() throws IOException {
+        // 7.16+
+        String body = "{  \"build_date\": \"2021-11-04T12:50:39Z\",  \"build_sha\": \"74a2ccb4be966ebab82b9727caef355fe9097340\",  \"publish_ready\": true,  \"version\": \"8.0.0\"}";
+        assertThat(ApmServerHealthChecker.parseVersion(body).compareTo(Version.of("8.0.0"))).isEqualTo(0);
+        // 7.0 -> 7.15
+        body = "{  \"build_date\": \"2021-03-06T04:41:35Z\",  \"build_sha\": \"b706a93fac838d7ca44622d8d9686d2c3b3c8bde\",  \"version\": \"7.11.2\"}";
+        assertThat(ApmServerHealthChecker.parseVersion(body).compareTo(Version.of("7.11.2"))).isEqualTo(0);
+        // 6.x
+        body = "{\"ok\":{\"build_date\":\"2021-10-13T17:29:41Z\",\"build_sha\":\"04a84d8d3d0358af5e73b3581c4ba37fbdbc979e\",\"version\":\"6.8.20\"}}";
+        assertThat(ApmServerHealthChecker.parseVersion(body).compareTo(Version.of("6.8.20"))).isEqualTo(0);
+    }
+}

apm-agent-core/src/test/java/co/elastic/apm/agent/util/VersionTest.java

@@ -36,4 +36,14 @@ void testVersion() {
         assertThat(Version.of("httpclient.4.5.13.redhat")).isEqualByComparingTo(Version.of("4.5.13"));
         assertThat(Version.of("httpclient.4.5.13-redhat")).isEqualByComparingTo(Version.of("4.5.13"));
     }
+
+    @Test
+    void testToString() {
+        System.out.println(Version.of(""));
+        System.out.println(Version.of("1"));
+        System.out.println(Version.of("1.2"));
+        System.out.println(Version.of("1.2.3"));
+        System.out.println(Version.of("1.2.3.4"));
+        System.out.println(Version.of("1.2.3.ignore"));
+    }
 }