Improve hostname calculation logic

src/main/java/org/datadog/jenkins/plugins/datadog/DatadogUtilities.java

@@ -25,8 +25,10 @@ of this software and associated documentation files (the "Software"), to deal
 
 package org.datadog.jenkins.plugins.datadog;
 
+import edu.umd.cs.findbugs.annotations.NonNull;
 import hudson.EnvVars;
 import hudson.ExtensionList;
+import hudson.FilePath;
 import hudson.PluginManager;
 import hudson.PluginWrapper;
 import hudson.model.Actionable;
@@ -39,6 +41,7 @@ of this software and associated documentation files (the "Software"), to deal
 import hudson.model.User;
 import hudson.model.labels.LabelAtom;
 import java.io.BufferedReader;
+import java.io.File;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.InputStreamReader;
@@ -58,16 +61,19 @@ of this software and associated documentation files (the "Software"), to deal
 import java.util.Set;
 import java.util.TimeZone;
 import java.util.function.Function;
+import java.util.logging.Level;
 import java.util.logging.Logger;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 import java.util.stream.Collectors;
 import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
 import jenkins.model.Jenkins;
+import jenkins.security.MasterToSlaveCallable;
 import org.apache.commons.lang.StringEscapeUtils;
 import org.apache.commons.lang.StringUtils;
 import org.apache.commons.lang.exception.ExceptionUtils;
+import org.datadog.jenkins.plugins.datadog.apm.ShellCommandCallable;
 import org.datadog.jenkins.plugins.datadog.clients.HttpClient;
 import org.datadog.jenkins.plugins.datadog.model.DatadogPluginAction;
 import org.datadog.jenkins.plugins.datadog.steps.DatadogPipelineAction;
@@ -96,6 +102,7 @@ public class DatadogUtilities {
     private static final Logger logger = Logger.getLogger(DatadogUtilities.class.getName());
 
     private static final Integer MAX_HOSTNAME_LEN = 255;
+    private static final int HOSTNAME_CMD_TIMEOUT_MILLIS = 3_000;
     private static final String DATE_FORMAT_ISO8601 = "yyyy-MM-dd'T'HH:mm:ss.SSSXXX";
     private static final List<String> UNIX_OS = Arrays.asList("mac", "linux", "freebsd", "sunos");
 
@@ -750,17 +757,45 @@ public static String getNodeHostname(@Nullable EnvVars envVars, @Nullable Comput
                 if (DatadogUtilities.isValidHostname(computerHostName)) {
                     return computerHostName;
                 }
+
+                Node node = computer.getNode();
+                if (node != null) {
+                    FilePath rootPath = node.getRootPath();
+                    if (isUnix(rootPath)) {
+                        ShellCommandCallable hostnameCommand = new ShellCommandCallable(
+                                Collections.emptyMap(), HOSTNAME_CMD_TIMEOUT_MILLIS, "hostname", "-f");
+                        String shellHostname = rootPath.act(hostnameCommand).trim();
+                        if (DatadogUtilities.isValidHostname(shellHostname)) {
+                            return shellHostname;
+                        }
+                    }
+                }
             }
         } catch (InterruptedException e){
             Thread.currentThread().interrupt();
-            logger.fine("Interrupted while trying to extract hostname from StepContext.");
+            logException(logger, Level.FINE, "Interrupted while trying to extract hostname from StepContext.", e);
 
-        } catch (IOException e){
-            logger.fine("Unable to extract hostname from StepContext.");
+        } catch (Exception e){
+            logException(logger, Level.FINE, "Unable to get hostname for node " + computer.getName(), e);
         }
         return null;
     }
 
+    private static boolean isUnix(FilePath filePath) throws IOException, InterruptedException {
+        return filePath != null && filePath.act(new IsUnix());
+    }
+
+    // copied from hudson.FilePath.IsUnix
+    private static final class IsUnix extends MasterToSlaveCallable<Boolean, IOException> {
+        private static final long serialVersionUID = 1L;
+
+        @Override
+        @NonNull
+        public Boolean call() {
+            return File.pathSeparatorChar == ':';
+        }
+    }
+
     @SuppressFBWarnings("NP_NULL_ON_SOME_PATH_FROM_RETURN_VALUE")
     public static Set<String> getNodeLabels(Computer computer) {
         Set<LabelAtom> labels;
@@ -904,14 +939,17 @@ public static String statusFromResult(String result) {
         }
     }
 
-    @SuppressFBWarnings("NP_NULL_ON_SOME_PATH")
     public static void severe(Logger logger, Throwable e, String message) {
+        logException(logger, Level.SEVERE, message, e);
+    }
+
+    public static void logException(Logger logger, Level logLevel, String message, Throwable e) {
         if (e != null) {
             String stackTrace = ExceptionUtils.getStackTrace(e);
-            message = (message != null ? message : "An unexpected error occurred: ") + stackTrace;
+            message = (message != null ? message + " " : "An unexpected error occurred: ") + stackTrace;
         }
         if (StringUtils.isNotEmpty(message)) {
-            logger.severe(message);
+            logger.log(logLevel, message);
         }
     }
 

src/main/java/org/datadog/jenkins/plugins/datadog/model/BuildData.java

@@ -38,11 +38,14 @@ of this software and associated documentation files (the "Software"), to deal
 import com.cloudbees.plugins.credentials.CredentialsParameterValue;
 import hudson.EnvVars;
 import hudson.matrix.MatrixProject;
+import hudson.model.AbstractBuild;
 import hudson.model.BooleanParameterValue;
 import hudson.model.Cause;
 import hudson.model.CauseAction;
+import hudson.model.Computer;
 import hudson.model.ItemGroup;
 import hudson.model.Job;
+import hudson.model.Node;
 import hudson.model.ParameterValue;
 import hudson.model.ParametersAction;
 import hudson.model.Result;
@@ -276,6 +279,11 @@ public BuildData(Run<?, ?> run, @Nullable TaskListener listener) throws IOExcept
             // the job is run on the master node, checking plugin config and locally available info.
             // (nodeName == null) condition is there to preserve existing behavior
             this.hostname = DatadogUtilities.getHostname(envVars);
+        } else if (run instanceof AbstractBuild) {
+            AbstractBuild<?, ?> build = (AbstractBuild<?, ?>) run;
+            Node node = build.getBuiltOn();
+            Computer computer = node != null ? node.toComputer() : null;
+            this.hostname = DatadogUtilities.getNodeHostname(envVars, computer);
         } else if (envVars.containsKey(DatadogGlobalConfiguration.DD_CI_HOSTNAME)) {
             // the job is run on an agent node, querying DD_CI_HOSTNAME set explicitly on agent
             this.hostname = envVars.get(DatadogGlobalConfiguration.DD_CI_HOSTNAME);

src/test/java/org/datadog/jenkins/plugins/datadog/model/BuildDataTest.java

@@ -8,9 +8,12 @@
 import hudson.EnvVars;
 import hudson.matrix.MatrixConfiguration;
 import hudson.matrix.MatrixProject;
+import hudson.model.Computer;
+import hudson.model.FreeStyleBuild;
 import hudson.model.Hudson;
 import hudson.model.ItemGroup;
 import hudson.model.Job;
+import hudson.model.Node;
 import hudson.model.Run;
 import hudson.model.TaskListener;
 import java.io.IOException;
@@ -22,7 +25,6 @@
 import org.jenkinsci.plugins.workflow.job.WorkflowRun;
 import org.jenkinsci.plugins.workflow.multibranch.WorkflowMultiBranchProject;
 import org.junit.Test;
-import org.mockito.invocation.InvocationOnMock;
 import org.mockito.stubbing.Answer;
 
 public class BuildDataTest {
@@ -100,7 +102,28 @@ public void testBuildTagFallsBackToAlternativeEnvVars() throws Exception {
         assertEquals("jenkins-jobName-buildNumber", buildData.getBuildTag(""));
     }
 
+    @Test
+    public void testNodeHostnameIsUsedForFreestyleBuilds() throws Exception {
+        String computerHostname = "computer-hostname";
+
+        Computer computer = mock(Computer.class);
+        when(computer.getHostName()).thenReturn(computerHostname);
+
+        Node node = mock(Node.class);
+        when(node.toComputer()).thenReturn(computer);
+
+        FreeStyleBuild build = givenJobRun(FreeStyleBuild.class, "jobName", "jobParentName", mock(Hudson.class), Collections.emptyMap());
+        when(build.getBuiltOn()).thenReturn(node);
+
+        BuildData buildData = whenCreatingBuildData(build);
+        assertEquals(computerHostname, buildData.getHostname(""));
+    }
+
     private Run<?, ?> givenJobRun(String jobName, String jobParentName, ItemGroup<?> jobParent, Map<String, String> environment) throws Exception {
+        return givenJobRun(Run.class, jobName, jobParentName, jobParent, environment);
+    }
+
+    private <T extends Run> T givenJobRun(Class<T> runClass, String jobName, String jobParentName, ItemGroup<?> jobParent, Map<String, String> environment) throws Exception {
         when(jobParent.getFullName()).thenReturn(jobParentName);
 
         Job job = mock(Job.class);
@@ -110,7 +133,7 @@ public void testBuildTagFallsBackToAlternativeEnvVars() throws Exception {
         EnvVars envVars = new EnvVars();
         envVars.putAll(environment);
 
-        Run run = mock(Run.class);
+        T run = mock(runClass);
         when(run.getEnvironment(any())).thenReturn(envVars);
         when(run.getParent()).thenReturn(job);
         when(run.getCharset()).thenReturn(StandardCharsets.UTF_8);

src/test/java/org/datadog/jenkins/plugins/datadog/stubs/BuildStub.java

@@ -2,6 +2,7 @@
 
 import hudson.EnvVars;
 import hudson.model.Build;
+import hudson.model.Node;
 import hudson.model.Result;
 import hudson.model.TaskListener;
 import java.io.IOException;
@@ -31,6 +32,11 @@ public BuildStub(@Nonnull ProjectStub project, Result result, EnvVars envVars, B
         this.previousNotFailedBuild = previousNotFailedBuild;
     }
 
+    @Override
+    public Node getBuiltOn() {
+        return null;
+    }
+
     protected BuildStub(@Nonnull ProjectStub project) throws IOException {
         super(project);
     }