Fix reading agent version (#2289)

* fix reading agent version

* add few tests on agent packaging

* update changelog

Author: SylvainJuge

CHANGELOG.asciidoc

@@ -27,6 +27,8 @@ endif::[]
 ===== Bug fixes
 * Fixing missing Micrometer metrics in Spring boot due to premature initialization - {pull}2255[#2255]
 * Fixing hostname trimming of FQDN too aggressive - {pull}2286[#2286]
+* Fixing agent `unknown` version - {pull}2289[#2289]
+
 
 [[release-notes-1.x]]
 === Java Agent version 1.x

apm-agent-core/src/main/java/co/elastic/apm/agent/configuration/StartupInfo.java

@@ -64,7 +64,16 @@ public void init(ElasticApmTracer tracer) {
     void logConfiguration(ConfigurationRegistry configurationRegistry, Logger logger) {
         final String serviceName = configurationRegistry.getConfig(CoreConfiguration.class).getServiceName();
         final String serviceVersion = configurationRegistry.getConfig(CoreConfiguration.class).getServiceVersion();
-        logger.info("Starting Elastic APM {} as {} ({}) on {}", elasticApmVersion, serviceName, serviceVersion, getJvmAndOsVersionString());
+
+        StringBuilder serviceNameAndVersion = new StringBuilder(serviceName);
+        if (serviceVersion != null) {
+            serviceNameAndVersion.append(" (").append(serviceVersion).append(")");
+        }
+
+        logger.info("Starting Elastic APM {} as {} on {}",
+            elasticApmVersion,
+            serviceNameAndVersion,
+            getJvmAndOsVersionString());
         logger.debug("VM Arguments: {}", ManagementFactory.getRuntimeMXBean().getInputArguments());
         for (List<ConfigurationOption<?>> options : configurationRegistry.getConfigurationOptionsByCategory().values()) {
             for (ConfigurationOption<?> option : options) {

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

@@ -38,9 +38,10 @@ public final class VersionUtils {
     private static final String AGENT_VERSION;
 
     static {
-        String version = getVersion(VersionUtils.class, "co.elastic.apm", "elastic-apm-agent");
+        File agentJar = ElasticApmAgent.getAgentJarFile();
+        String version = getManifestEntry(agentJar, "Implementation-Version");
         if (version != null && version.endsWith("SNAPSHOT")) {
-            String gitRev = getManifestEntry(ElasticApmAgent.getAgentJarFile(), "SCM-Revision");
+            String gitRev = getManifestEntry(agentJar, "SCM-Revision");
             if (gitRev != null) {
                 version = version + "." + gitRev;
             }

apm-agent/pom.xml

@@ -21,8 +21,6 @@
     <properties>
         <apm-agent-parent.base.dir>${project.basedir}/..</apm-agent-parent.base.dir>
 
-        <!-- default value that will be overloaded through commit-id plugin -->
-        <git.commit.id.abbrev>UNKNOWN</git.commit.id.abbrev>
     </properties>
 
     <dependencies>
@@ -445,7 +443,6 @@
                                 <transformer implementation="org.apache.maven.plugins.shade.resource.ManifestResourceTransformer">
                                     <manifestEntries>
                                         <Automatic-Module-Name>${project.groupId}.agent</Automatic-Module-Name>
-                                        <SCM-Revision>${git.commit.id.abbrev}</SCM-Revision>
                                     </manifestEntries>
                                 </transformer>
                                 <transformer implementation="org.apache.maven.plugins.shade.resource.ApacheNoticeResourceTransformer" />
@@ -477,19 +474,6 @@
                     </execution>
                 </executions>
             </plugin>
-            <plugin>
-                <groupId>pl.project13.maven</groupId>
-                <artifactId>git-commit-id-plugin</artifactId>
-                <version>4.0.2</version>
-                <executions>
-                    <execution>
-                        <id>get-the-git-infos</id>
-                        <goals>
-                            <goal>revision</goal>
-                        </goals>
-                    </execution>
-                </executions>
-            </plugin>
         </plugins>
     </build>
 

elastic-apm-agent/pom.xml

@@ -13,6 +13,9 @@
 
     <properties>
         <apm-agent-parent.base.dir>${project.basedir}/..</apm-agent-parent.base.dir>
+
+        <!-- default value that will be overloaded through commit-id plugin -->
+        <git.commit.id.abbrev>UNKNOWN</git.commit.id.abbrev>
     </properties>
 
     <dependencies>
@@ -102,7 +105,6 @@
                 </executions>
             </plugin>
             <plugin>
-                <groupId>org.apache.maven.plugins</groupId>
                 <artifactId>maven-antrun-plugin</artifactId>
                 <version>3.0.0</version>
                 <executions>
@@ -167,6 +169,19 @@
                     </execution>
                 </executions>
             </plugin>
+            <plugin>
+                <groupId>pl.project13.maven</groupId>
+                <artifactId>git-commit-id-plugin</artifactId>
+                <version>4.0.2</version>
+                <executions>
+                    <execution>
+                        <id>get-the-git-infos</id>
+                        <goals>
+                            <goal>revision</goal>
+                        </goals>
+                    </execution>
+                </executions>
+            </plugin>
             <plugin>
                 <artifactId>maven-shade-plugin</artifactId>
                 <executions>
@@ -190,6 +205,7 @@
                                     <manifestEntries>
                                         <Premain-Class>co.elastic.apm.agent.premain.AgentMain</Premain-Class>
                                         <Agent-Class>co.elastic.apm.agent.premain.AgentMain</Agent-Class>
+                                        <SCM-Revision>${git.commit.id.abbrev}</SCM-Revision>
                                         <Can-Redefine-Classes>true</Can-Redefine-Classes>
                                         <Can-Retransform-Classes>true</Can-Retransform-Classes>
                                         <Can-Set-Native-Method-Prefix>true</Can-Set-Native-Method-Prefix>

elastic-apm-agent/src/test/java/co/elastic/apm/agent/premain/AgentPackagingIT.java

@@ -0,0 +1,94 @@
+/*
+ * 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.premain;
+
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Test;
+
+import java.io.BufferedReader;
+import java.io.IOException;
+import java.net.URISyntaxException;
+import java.net.URL;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.Properties;
+import java.util.jar.Attributes;
+import java.util.jar.JarFile;
+import java.util.jar.Manifest;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+public class AgentPackagingIT {
+
+    private static Path agentJar;
+    private static String agentVersion;
+
+    @BeforeAll
+    static void before() throws URISyntaxException, IOException {
+        // find packaged jar location
+
+        String classResource = AgentPackagingIT.class.getCanonicalName().replace(".", "/") + ".class";
+        URL url = AgentPackagingIT.class.getClassLoader().getResource(classResource);
+        assertThat(url).isNotNull();
+
+        Path path = Paths.get(url.toURI());
+        Path targetFolder = null;
+        while (path != null && targetFolder == null) {
+            if (path.endsWith("target")) {
+                targetFolder = path;
+            }
+            path = path.getParent();
+        }
+        assertThat(targetFolder).isNotNull();
+
+        Path mavenPropertiesFile = targetFolder.resolve("maven-archiver").resolve("pom.properties");
+        assertThat(mavenPropertiesFile).isRegularFile();
+
+        Properties mavenProperties = new Properties();
+        try (BufferedReader reader = Files.newBufferedReader(mavenPropertiesFile)) {
+            mavenProperties.load(reader);
+        }
+
+        String artifactId = mavenProperties.getProperty("artifactId");
+        assertThat(artifactId).isEqualTo("elastic-apm-agent");
+
+        agentVersion = mavenProperties.getProperty("version");
+        assertThat(agentVersion).isNotNull();
+
+        agentJar = targetFolder.resolve(String.format("%s-%s.jar", artifactId, agentVersion));
+        assertThat(agentJar).isRegularFile();
+    }
+
+    @Test
+    void agentManifest() throws IOException {
+
+        JarFile jarFile = new JarFile(agentJar.toFile());
+
+        Manifest manifest = jarFile.getManifest();
+
+        Attributes attributes = manifest.getMainAttributes();
+        assertThat(attributes.getValue("Implementation-Version")).isEqualTo(agentVersion);
+        String agentClass = attributes.getValue("Agent-Class");
+        assertThat(agentClass).isNotEmpty();
+        assertThat(attributes.getValue("Premain-Class")).isEqualTo(agentClass);
+        assertThat(attributes.getValue("SCM-Revision")).isNotEmpty();
+    }
+
+}