Merge pull request #13939 from vespa-engine/olaa/get-logs-from-specific-node

Support fetching logs from specific node

6 files changed, 38 insertions, 33 deletions

diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/ApplicationRepository.java b/configserver/src/main/java/com/yahoo/vespa/config/server/ApplicationRepository.java
index 937cf4dfe7f..58b7d0ec5bb 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/ApplicationRepository.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/ApplicationRepository.java
@@ -874,12 +874,8 @@ public class ApplicationRepository implements com.yahoo.config.provision.Deploye
         // We make no validation that the hostname is actually allocated to the given application since
         // most applications under hosted-vespa are not known to the model and it's OK for a user to get
         // logs for any host if they are authorized for the hosted-vespa tenant.
-        if (hostname.isPresent()) {
-            if (HOSTED_VESPA_TENANT.equals(applicationId.tenant()))
-                return "http://" + hostname.get() + ":8080/logs";
-            else
-                throw new IllegalArgumentException("Using hostname parameter when getting logs is not supported for application "
-                                                   + applicationId);
+        if (hostname.isPresent() && HOSTED_VESPA_TENANT.equals(applicationId.tenant())) {
+            return "http://" + hostname.get() + ":8080/logs";
         }
 
         Application application = getApplication(applicationId);
diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/ApplicationRepositoryTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/ApplicationRepositoryTest.java
index a4b2bfb8902..e20fef0909c 100644
--- a/configserver/src/test/java/com/yahoo/vespa/config/server/ApplicationRepositoryTest.java
+++ b/configserver/src/test/java/com/yahoo/vespa/config/server/ApplicationRepositoryTest.java
@@ -234,14 +234,6 @@ public class ApplicationRepositoryTest {
         assertEquals(200, response.getStatus());
     }
 
-    @Test(expected = IllegalArgumentException.class)
-    public void refuseToGetLogsFromHostnameNotInApplication() {
-        applicationRepository = createApplicationRepository();
-        deployApp(testAppLogServerWithContainer);
-        HttpResponse response = applicationRepository.getLogs(applicationId(), Optional.of("host123.fake.yahoo.com"), "");
-        assertEquals(200, response.getStatus());
-    }
-
     @Test
     public void deleteUnusedFileReferences() throws IOException {
         File fileReferencesDir = temporaryFolder.newFolder();
diff --git a/container-core/src/main/java/com/yahoo/container/handler/LogHandler.java b/container-core/src/main/java/com/yahoo/container/handler/LogHandler.java
index f3149ed4998..b2a156862eb 100644
--- a/container-core/src/main/java/com/yahoo/container/handler/LogHandler.java
+++ b/container-core/src/main/java/com/yahoo/container/handler/LogHandler.java
@@ -35,11 +35,13 @@ public class LogHandler extends ThreadedHttpRequestHandler {
         Instant to = Optional.ofNullable(request.getProperty("to"))
                              .map(Long::valueOf).map(Instant::ofEpochMilli).orElse(Instant.MAX);
 
+        Optional<String> hostname = Optional.ofNullable(request.getProperty("hostname"));
+
         return new HttpResponse(200) {
             @Override
             public void render(OutputStream outputStream) {
                 try {
-                    logReader.writeLogs(outputStream, from, to);
+                    logReader.writeLogs(outputStream, from, to, hostname);
                 }
                 catch (Throwable t) {
                     log.log(Level.WARNING, "Failed reading logs from " + from + " to " + to, t);
diff --git a/container-core/src/main/java/com/yahoo/container/handler/LogReader.java b/container-core/src/main/java/com/yahoo/container/handler/LogReader.java
index 3cf849a6835..8e4b9aea9b8 100644
--- a/container-core/src/main/java/com/yahoo/container/handler/LogReader.java
+++ b/container-core/src/main/java/com/yahoo/container/handler/LogReader.java
@@ -27,6 +27,7 @@ import java.util.ArrayList;
 import java.util.Comparator;
 import java.util.Iterator;
 import java.util.List;
+import java.util.Optional;
 import java.util.TreeMap;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
@@ -57,7 +58,7 @@ class LogReader {
         this.logFilePattern = logFilePattern;
     }
 
-    void writeLogs(OutputStream out, Instant from, Instant to) {
+    void writeLogs(OutputStream out, Instant from, Instant to, Optional<String> hostname) {
         double fromSeconds = from.getEpochSecond() + from.getNano() / 1e9;
         double toSeconds = to.getEpochSecond() + to.getNano() / 1e9;
         BufferedWriter writer = new BufferedWriter(new OutputStreamWriter(out));
@@ -67,7 +68,7 @@ class LogReader {
                 try {
                     // Logs in each sub-list contain entries covering the same time interval, so do a merge sort while reading
                     for (Path log : logs)
-                        logLineIterators.add(new LogLineIterator(log, fromSeconds, toSeconds));
+                        logLineIterators.add(new LogLineIterator(log, fromSeconds, toSeconds, hostname));
 
                     Iterator<LineWithTimestamp> lines = Iterators.mergeSorted(logLineIterators,
                                                                               Comparator.comparingDouble(LineWithTimestamp::timestamp));
@@ -96,14 +97,16 @@ class LogReader {
         private final BufferedReader reader;
         private final double from;
         private final double to;
+        private final Optional<String> hostname;
         private LineWithTimestamp next;
 
-        private LogLineIterator(Path log, double from, double to) throws IOException {
+        private LogLineIterator(Path log, double from, double to, Optional<String> hostname) throws IOException {
             boolean zipped = log.toString().endsWith(".gz");
             InputStream in = Files.newInputStream(log);
             this.reader = new BufferedReader(new InputStreamReader(zipped ? new GZIPInputStream(in) : in, UTF_8));
             this.from = from;
             this.to = to;
+            this.hostname = hostname;
             this.next = readNext();
         }
 
@@ -131,6 +134,9 @@ class LogReader {
                     if (parts.length != 7)
                         continue;
 
+                    if (hostname.map(host -> !host.equals(parts[1])).orElse(false))
+                        continue;
+
                     double timestamp = Double.parseDouble(parts[0]);
                     if (timestamp > to)
                         return null;
diff --git a/container-core/src/test/java/com/yahoo/container/handler/LogHandlerTest.java b/container-core/src/test/java/com/yahoo/container/handler/LogHandlerTest.java
index ab0d0d54675..97aa8864eae 100644
--- a/container-core/src/test/java/com/yahoo/container/handler/LogHandlerTest.java
+++ b/container-core/src/test/java/com/yahoo/container/handler/LogHandlerTest.java
@@ -9,6 +9,7 @@ import java.io.ByteArrayOutputStream;
 import java.io.IOException;
 import java.io.OutputStream;
 import java.time.Instant;
+import java.util.Optional;
 import java.util.concurrent.Executor;
 
 import static org.junit.Assert.assertEquals;
@@ -47,7 +48,7 @@ public class LogHandlerTest {
         }
 
         @Override
-        protected void writeLogs(OutputStream out, Instant from, Instant to)  {
+        protected void writeLogs(OutputStream out, Instant from, Instant to, Optional<String> hostname)  {
             try {
                 if (to.isAfter(Instant.ofEpochMilli(1000))) {
                     out.write("newer log".getBytes());
diff --git a/container-core/src/test/java/com/yahoo/container/handler/LogReaderTest.java b/container-core/src/test/java/com/yahoo/container/handler/LogReaderTest.java
index 3f7a78e13be..ad9398a5eec 100644
--- a/container-core/src/test/java/com/yahoo/container/handler/LogReaderTest.java
+++ b/container-core/src/test/java/com/yahoo/container/handler/LogReaderTest.java
@@ -5,7 +5,6 @@ import com.yahoo.vespa.test.file.TestFileSystem;
 import org.junit.Before;
 import org.junit.Test;
 
-import java.io.ByteArrayInputStream;
 import java.io.ByteArrayOutputStream;
 import java.io.IOException;
 import java.io.OutputStream;
@@ -14,8 +13,8 @@ import java.nio.file.Files;
 import java.nio.file.Path;
 import java.time.Duration;
 import java.time.Instant;
+import java.util.Optional;
 import java.util.regex.Pattern;
-import java.util.zip.GZIPInputStream;
 import java.util.zip.GZIPOutputStream;
 
 import static java.nio.charset.StandardCharsets.UTF_8;
@@ -26,12 +25,12 @@ public class LogReaderTest {
     private final FileSystem fileSystem = TestFileSystem.create();
     private final Path logDirectory = fileSystem.getPath("/opt/vespa/logs");
 
-    private static final String logv11 = "3600.2\t17491290-v6-1.ostk.bm2.prod.ne1.yahoo.com\t5480\tcontainer\tstdout\tinfo\tfourth\n";
-    private static final String logv   = "90000.1\t17491290-v6-1.ostk.bm2.prod.ne1.yahoo.com\t5480\tcontainer\tstdout\tinfo\tlast\n";
-    private static final String log100 = "0.2\t17491290-v6-1.ostk.bm2.prod.ne1.yahoo.com\t5480\tcontainer\tstdout\tinfo\tsecond\n";
-    private static final String log101 = "0.1\t17491290-v6-1.ostk.bm2.prod.ne1.yahoo.com\t5480\tcontainer\tstdout\tinfo\tERROR: Bundle canary-application [71] Unable to get module class path. (java.lang.NullPointerException)\n";
-    private static final String log110 = "3600.1\t17491290-v6-1.ostk.bm2.prod.ne1.yahoo.com\t5480\tcontainer\tstderr\twarning\tthird\n";
-    private static final String log200 = "86400.1\t17491290-v6-1.ostk.bm2.prod.ne1.yahoo.com\t5480\tcontainer\tstderr\twarning\tjava.lang.NullPointerException\\n\\tat org.apache.felix.framework.BundleRevisionImpl.calculateContentPath(BundleRevisionImpl.java:438)\\n\\tat org.apache.felix.framework.BundleRevisionImpl.initializeContentPath(BundleRevisionImpl.java:371)\n";
+    private static final String logv11 = "3600.2\tnode1.com\t5480\tcontainer\tstdout\tinfo\tfourth\n";
+    private static final String logv   = "90000.1\tnode1.com\t5480\tcontainer\tstdout\tinfo\tlast\n";
+    private static final String log100 = "0.2\tnode2.com\t5480\tcontainer\tstdout\tinfo\tsecond\n";
+    private static final String log101 = "0.1\tnode2.com\t5480\tcontainer\tstdout\tinfo\tERROR: Bundle canary-application [71] Unable to get module class path. (java.lang.NullPointerException)\n";
+    private static final String log110 = "3600.1\tnode1.com\t5480\tcontainer\tstderr\twarning\tthird\n";
+    private static final String log200 = "86400.1\tnode2.com\t5480\tcontainer\tstderr\twarning\tjava.lang.NullPointerException\\n\\tat org.apache.felix.framework.BundleRevisionImpl.calculateContentPath(BundleRevisionImpl.java:438)\\n\\tat org.apache.felix.framework.BundleRevisionImpl.initializeContentPath(BundleRevisionImpl.java:371)\n";
 
     @Before
     public void setup() throws IOException {
@@ -52,32 +51,41 @@ public class LogReaderTest {
     }
 
     @Test
-    public void testThatLogsOutsideRangeAreExcluded() throws Exception {
+    public void testThatLogsOutsideRangeAreExcluded() {
         ByteArrayOutputStream baos = new ByteArrayOutputStream();
         LogReader logReader = new LogReader(logDirectory, Pattern.compile(".*"));
-        logReader.writeLogs(baos, Instant.ofEpochMilli(150), Instant.ofEpochMilli(3601050));
+        logReader.writeLogs(baos, Instant.ofEpochMilli(150), Instant.ofEpochMilli(3601050), Optional.empty());
 
         assertEquals(log100 + logv11 + log110, baos.toString(UTF_8));
     }
 
     @Test
-    public void testThatLogsNotMatchingRegexAreExcluded() throws Exception {
+    public void testThatLogsNotMatchingRegexAreExcluded() {
         ByteArrayOutputStream baos = new ByteArrayOutputStream();
         LogReader logReader = new LogReader(logDirectory, Pattern.compile(".*-1.*"));
-        logReader.writeLogs(baos, Instant.EPOCH, Instant.EPOCH.plus(Duration.ofDays(2)));
+        logReader.writeLogs(baos, Instant.EPOCH, Instant.EPOCH.plus(Duration.ofDays(2)), Optional.empty());
 
         assertEquals(log101 + logv11, baos.toString(UTF_8));
     }
 
     @Test
-    public void testZippedStreaming() throws IOException {
+    public void testZippedStreaming() {
         ByteArrayOutputStream zippedBaos = new ByteArrayOutputStream();
         LogReader logReader = new LogReader(logDirectory, Pattern.compile(".*"));
-        logReader.writeLogs(zippedBaos, Instant.EPOCH, Instant.EPOCH.plus(Duration.ofDays(2)));
+        logReader.writeLogs(zippedBaos, Instant.EPOCH, Instant.EPOCH.plus(Duration.ofDays(2)), Optional.empty());
 
         assertEquals(log101 + log100 + logv11 + log110 + log200 + logv, zippedBaos.toString(UTF_8));
     }
 
+    @Test
+    public void logsForSingeNodeIsRetrieved() {
+        ByteArrayOutputStream baos = new ByteArrayOutputStream();
+        LogReader logReader = new LogReader(logDirectory, Pattern.compile(".*"));
+        logReader.writeLogs(baos, Instant.EPOCH, Instant.EPOCH.plus(Duration.ofDays(2)), Optional.of("node2.com"));
+
+        assertEquals(log101 + log100 + log200, baos.toString(UTF_8));
+    }
+
     private byte[] compress(String input) throws IOException {
         ByteArrayOutputStream baos = new ByteArrayOutputStream();
         OutputStream zip = new GZIPOutputStream(baos);