From 494e3a27574afef95c1a7bc1b451b155828497cb Mon Sep 17 00:00:00 2001 From: Nikita Tkachenko Date: Tue, 14 May 2024 19:02:52 +0200 Subject: [PATCH 1/5] Improve hostname calculation logic --- .../plugins/datadog/DatadogUtilities.java | 20 +++++++++++++++++-- .../plugins/datadog/model/BuildData.java | 8 ++++++++ 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/datadog/jenkins/plugins/datadog/DatadogUtilities.java b/src/main/java/org/datadog/jenkins/plugins/datadog/DatadogUtilities.java index cf35cfe5..26cd3c38 100644 --- a/src/main/java/org/datadog/jenkins/plugins/datadog/DatadogUtilities.java +++ b/src/main/java/org/datadog/jenkins/plugins/datadog/DatadogUtilities.java @@ -27,6 +27,7 @@ of this software and associated documentation files (the "Software"), to deal import hudson.EnvVars; import hudson.ExtensionList; +import hudson.FilePath; import hudson.PluginManager; import hudson.PluginWrapper; import hudson.model.Actionable; @@ -68,6 +69,7 @@ of this software and associated documentation files (the "Software"), to deal 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 +98,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 UNIX_OS = Arrays.asList("mac", "linux", "freebsd", "sunos"); @@ -750,13 +753,26 @@ 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 (rootPath != null) { + 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."); - } catch (IOException e){ - logger.fine("Unable to extract hostname from StepContext."); + } catch (Exception e){ + logger.fine("Unable to extract hostname from StepContext"); } return null; } diff --git a/src/main/java/org/datadog/jenkins/plugins/datadog/model/BuildData.java b/src/main/java/org/datadog/jenkins/plugins/datadog/model/BuildData.java index 6eddfb6e..5b6bde90 100644 --- a/src/main/java/org/datadog/jenkins/plugins/datadog/model/BuildData.java +++ b/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); From 48c36ebf5afbad2dbff498f42e1669cc88f73819 Mon Sep 17 00:00:00 2001 From: Nikita Tkachenko Date: Tue, 14 May 2024 19:13:51 +0200 Subject: [PATCH 2/5] Add a unit test --- .../plugins/datadog/model/BuildDataTest.java | 27 +++++++++++++++++-- 1 file changed, 25 insertions(+), 2 deletions(-) diff --git a/src/test/java/org/datadog/jenkins/plugins/datadog/model/BuildDataTest.java b/src/test/java/org/datadog/jenkins/plugins/datadog/model/BuildDataTest.java index 7dc14cfd..b6db4b6b 100644 --- a/src/test/java/org/datadog/jenkins/plugins/datadog/model/BuildDataTest.java +++ b/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 environment) throws Exception { + return givenJobRun(Run.class, jobName, jobParentName, jobParent, environment); + } + + private T givenJobRun(Class runClass, String jobName, String jobParentName, ItemGroup jobParent, Map 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); From 9e31a06793655958e1cc377bcab97cc9794d5495 Mon Sep 17 00:00:00 2001 From: Nikita Tkachenko Date: Thu, 16 May 2024 14:05:07 +0200 Subject: [PATCH 3/5] Fix build stub to not fail when getBuiltOn() is called in absence of Jenkins --- .../datadog/jenkins/plugins/datadog/stubs/BuildStub.java | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/test/java/org/datadog/jenkins/plugins/datadog/stubs/BuildStub.java b/src/test/java/org/datadog/jenkins/plugins/datadog/stubs/BuildStub.java index e1d0b99d..a8cdaaac 100644 --- a/src/test/java/org/datadog/jenkins/plugins/datadog/stubs/BuildStub.java +++ b/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); } From 5a2659c82d163ca0a88e747c39cbbf5d4dd279e9 Mon Sep 17 00:00:00 2001 From: Nikita Tkachenko Date: Thu, 16 May 2024 22:15:38 +0200 Subject: [PATCH 4/5] Do not try to execute hostname command on non-Unix node --- .../plugins/datadog/DatadogUtilities.java | 20 ++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/datadog/jenkins/plugins/datadog/DatadogUtilities.java b/src/main/java/org/datadog/jenkins/plugins/datadog/DatadogUtilities.java index 26cd3c38..0ccda5dd 100644 --- a/src/main/java/org/datadog/jenkins/plugins/datadog/DatadogUtilities.java +++ b/src/main/java/org/datadog/jenkins/plugins/datadog/DatadogUtilities.java @@ -25,6 +25,7 @@ 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; @@ -40,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; @@ -66,6 +68,7 @@ of this software and associated documentation files (the "Software"), to deal 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; @@ -757,7 +760,7 @@ public static String getNodeHostname(@Nullable EnvVars envVars, @Nullable Comput Node node = computer.getNode(); if (node != null) { FilePath rootPath = node.getRootPath(); - if (rootPath != null) { + if (isUnix(rootPath)) { ShellCommandCallable hostnameCommand = new ShellCommandCallable( Collections.emptyMap(), HOSTNAME_CMD_TIMEOUT_MILLIS, "hostname", "-f"); String shellHostname = rootPath.act(hostnameCommand).trim(); @@ -777,6 +780,21 @@ public static String getNodeHostname(@Nullable EnvVars envVars, @Nullable Comput 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 { + 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 getNodeLabels(Computer computer) { Set labels; From 7ec9031e3a5d87ca5d7b8387ce4bcf256de5e4b5 Mon Sep 17 00:00:00 2001 From: Nikita Tkachenko Date: Tue, 21 May 2024 09:44:20 +0200 Subject: [PATCH 5/5] Add context to error message --- .../jenkins/plugins/datadog/DatadogUtilities.java | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/src/main/java/org/datadog/jenkins/plugins/datadog/DatadogUtilities.java b/src/main/java/org/datadog/jenkins/plugins/datadog/DatadogUtilities.java index 0ccda5dd..221fc735 100644 --- a/src/main/java/org/datadog/jenkins/plugins/datadog/DatadogUtilities.java +++ b/src/main/java/org/datadog/jenkins/plugins/datadog/DatadogUtilities.java @@ -61,6 +61,7 @@ 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; @@ -772,10 +773,10 @@ public static String getNodeHostname(@Nullable EnvVars envVars, @Nullable Comput } } 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 (Exception e){ - logger.fine("Unable to extract hostname from StepContext"); + logException(logger, Level.FINE, "Unable to get hostname for node " + computer.getName(), e); } return null; } @@ -938,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); } }