Cleaning up metrics support

CodexDistTestCore/DistTest.cs

@@ -96,29 +96,6 @@ namespace CodexDistTestCore
             return metricsAggregator.BeginCollectingMetricsFor(onlineNodes);
         }
 
-        public void AssertWithTimeout<T>(Func<T> operation, T isEqualTo, string message)
-        {
-            AssertWithTimeout(operation, isEqualTo, TimeSpan.FromMinutes(10), message);
-        }
-
-        public void AssertWithTimeout<T>(Func<T> operation, T isEqualTo, TimeSpan timeout, string message)
-        {
-            var start = DateTime.UtcNow;
-
-            while (true)
-            {
-                var result = operation();
-                if (result!.Equals(isEqualTo)) return;
-                if (DateTime.UtcNow - start > timeout)
-                {
-                    Assert.That(result, Is.EqualTo(isEqualTo), message);
-                    return;
-                }
-
-                Utils.Sleep(TimeSpan.FromSeconds(2));
-            }
-        }
-
         private void IncludeLogsAndMetricsOnTestFailure()
         {
             var result = TestContext.CurrentContext.Result;

CodexDistTestCore/FileManager.cs

@@ -80,28 +80,30 @@ namespace CodexDistTestCore
             return info.Length;
         }
 
-        public void AssertIsEqual(TestFile? other)
+        public void AssertIsEqual(TestFile? actual)
         {
-            if (other == null) Assert.Fail("TestFile is null.");
+            if (actual == null) Assert.Fail("TestFile is null.");
-            if (other == this || other!.Filename == Filename) Assert.Fail("TestFile is compared to itself.");
+            if (actual == this || actual!.Filename == Filename) Assert.Fail("TestFile is compared to itself.");
 
-            using var stream1 = new FileStream(Filename, FileMode.Open, FileAccess.Read);
+            Assert.That(actual.GetFileSize(), Is.EqualTo(GetFileSize()), "Files are not of equal length.");
-            using var stream2 = new FileStream(other.Filename, FileMode.Open, FileAccess.Read);
 
-            var bytes1 = new byte[FileManager.ChunkSize];
+            using var streamExpected = new FileStream(Filename, FileMode.Open, FileAccess.Read);
-            var bytes2 = new byte[FileManager.ChunkSize];
+            using var streamActual = new FileStream(actual.Filename, FileMode.Open, FileAccess.Read);
 
-            var read1 = 0;
+            var bytesExpected = new byte[FileManager.ChunkSize];
-            var read2 = 0;
+            var bytesActual = new byte[FileManager.ChunkSize];
+
+            var readExpected = 0;
+            var readActual = 0;
 
             while (true)
             {
-                read1 = stream1.Read(bytes1, 0, FileManager.ChunkSize);
+                readExpected = streamExpected.Read(bytesExpected, 0, FileManager.ChunkSize);
-                read2 = stream2.Read(bytes2, 0, FileManager.ChunkSize);
+                readActual = streamActual.Read(bytesActual, 0, FileManager.ChunkSize);
 
-                if (read1 == 0 && read2 == 0) return;
+                if (readExpected == 0 && readActual == 0) return;
-                Assert.That(read1, Is.EqualTo(read2), "Files are not of equal length.");
+                Assert.That(readActual, Is.EqualTo(readExpected), "Unable to read buffers of equal length.");
-                CollectionAssert.AreEqual(bytes1, bytes2, "Files are not binary-equal.");
+                CollectionAssert.AreEqual(bytesExpected, bytesActual, "Files are not binary-equal.");
             }
         }
     }

CodexDistTestCore/MetricsAccess.cs

@@ -1,11 +1,12 @@
 using CodexDistTestCore.Config;
 using NUnit.Framework;
+using NUnit.Framework.Constraints;
 
 namespace CodexDistTestCore
 {
     public interface IMetricsAccess
     {
-        int? GetMostRecentInt(string metricName, IOnlineCodexNode node);
+        void AssertThat(IOnlineCodexNode node, string metricName, IResolveConstraint constraint, string message = "");
     }
 
     public class MetricsAccess : IMetricsAccess
@@ -23,27 +24,59 @@ namespace CodexDistTestCore
             this.nodes = nodes;
         }
 
-        public int? GetMostRecentInt(string metricName, IOnlineCodexNode node)
+        public void AssertThat(IOnlineCodexNode node, string metricName, IResolveConstraint constraint, string message = "")
+        {
+            var metricValue = GetMetricWithTimeout(metricName, node);
+            Assert.That(metricValue, constraint, message);
+        }
+
+        private double GetMetricWithTimeout(string metricName, IOnlineCodexNode node)
+        {
+            var start = DateTime.UtcNow;
+
+            while (true)
+            {
+                var mostRecent = GetMostRecent(metricName, node);
+                if (mostRecent != null) return Convert.ToDouble(mostRecent);
+                if (DateTime.UtcNow - start > Timing.WaitForMetricTimeout())
+                {
+                    Assert.Fail($"Timeout: Unable to get metric '{metricName}'.");
+                    throw new TimeoutException();
+                }
+
+                Utils.Sleep(TimeSpan.FromSeconds(2));
+            }
+        }
+
+        private object? GetMostRecent(string metricName, IOnlineCodexNode node)
         {
             var n = (OnlineCodexNode)node;
             CollectionAssert.Contains(nodes, n, "Incorrect test setup: Attempt to get metrics for OnlineCodexNode from the wrong MetricsAccess object. " +
                 "(This CodexNode is tracked by a different instance.)");
 
-            var pod = n.Group.PodInfo!;
+            var response = GetMetric(metricName);
+            if (response == null) return null;
 
+            var value = GetValueFromResponse(n, response);
+            if (value == null) return null;
+            if (value.Length != 2) throw new InvalidOperationException("Expected value to be [double, string].");
+            return value[1];
+        }
+
+        private PrometheusQueryResponse? GetMetric(string metricName)
+        {
             var response = http.HttpGetJson<PrometheusQueryResponse>($"query?query=last_over_time({metricName}[12h])");
             if (response.status != "success") return null;
+            return response;
+        }
 
-            var forNode = response.data.result.SingleOrDefault(d => d.metric.instance == $"{pod.Ip}:{n.Container.MetricsPort}");
+        private object[]? GetValueFromResponse(OnlineCodexNode node, PrometheusQueryResponse response)
+        {
+            var pod = node.Group.PodInfo!;
+            var forNode = response.data.result.SingleOrDefault(d => d.metric.instance == $"{pod.Ip}:{node.Container.MetricsPort}");
             if (forNode == null) return null;
-
             if (forNode.value == null || forNode.value.Length == 0) return null;
-
+            return forNode.value;
-            if (forNode.value.Length != 2) throw new InvalidOperationException("Expected value to be [double, string].");
-            // [0] = double, timestamp
-            // [1] = string, value
-
-            return Convert.ToInt32(forNode.value[1]);
         }
     }
 

CodexDistTestCore/TestLog.cs

@@ -1,5 +1,6 @@
 using CodexDistTestCore.Config;
 using NUnit.Framework;
+using System.Xml.Linq;
 
 namespace CodexDistTestCore
 {
@@ -39,6 +40,16 @@ namespace CodexDistTestCore
                 Log(result.Message);
                 Log($"{result.StackTrace}");
             }
+
+            if (result.Outcome.Status == NUnit.Framework.Interfaces.TestStatus.Failed)
+            {
+                RenameLogFile();
+            }
+        }
+
+        private void RenameLogFile()
+        {
+            file.ConcatToFilename("_FAILED");
         }
 
         public LogFile CreateSubfile()
@@ -63,10 +74,15 @@ namespace CodexDistTestCore
 
     public class LogFile
     {
+        private readonly DateTime now;
+        private string name;
         private readonly string filepath;
 
         public LogFile(DateTime now, string name)
         {
+            this.now = now;
+            this.name = name;
+
             filepath = Path.Join(
                 LogConfig.LogRoot,
                 $"{now.Year}-{Pad(now.Month)}",
@@ -74,12 +90,11 @@ namespace CodexDistTestCore
 
             Directory.CreateDirectory(filepath);
 
-            FilenameWithoutPath = $"{Pad(now.Hour)}-{Pad(now.Minute)}-{Pad(now.Second)}Z_{name.Replace('.', '-')}.log";
+            GenerateFilename();
-            FullFilename = Path.Combine(filepath, FilenameWithoutPath);
         }
 
-        public string FullFilename { get; }
+        public string FullFilename { get; private set; } = string.Empty;
-        public string FilenameWithoutPath { get; }
+        public string FilenameWithoutPath { get; private set; } = string.Empty;
 
         public void Write(string message)
         {
@@ -98,6 +113,17 @@ namespace CodexDistTestCore
             }
         }
 
+        public void ConcatToFilename(string toAdd)
+        {
+            var oldFullName = FullFilename;
+
+            name += toAdd;
+
+            GenerateFilename();
+
+            File.Move(oldFullName, FullFilename);
+        }
+
         private static string Pad(int n)
         {
             return n.ToString().PadLeft(2, '0');
@@ -107,5 +133,11 @@ namespace CodexDistTestCore
         {
             return $"[{DateTime.UtcNow.ToString("u")}]";
         }
+
+        private void GenerateFilename()
+        {
+            FilenameWithoutPath = $"{Pad(now.Hour)}-{Pad(now.Minute)}-{Pad(now.Second)}Z_{name.Replace('.', '-')}.log";
+            FullFilename = Path.Combine(filepath, FilenameWithoutPath);
+        }
     }
 }

CodexDistTestCore/Timing.cs

@@ -40,6 +40,11 @@ namespace CodexDistTestCore
             return GetTimes().K8sOperationTimeout();
         }
 
+        public static TimeSpan WaitForMetricTimeout()
+        {
+            return GetTimes().WaitForMetricTimeout();
+        }
+
         private static ITimeSet GetTimes()
         {
             var testProperties = TestContext.CurrentContext.Test.Properties;
@@ -55,6 +60,7 @@ namespace CodexDistTestCore
         TimeSpan HttpCallRetryDelay();
         TimeSpan WaitForK8sServiceDelay();
         TimeSpan K8sOperationTimeout();
+        TimeSpan WaitForMetricTimeout();
     }
 
     public class DefaultTimeSet : ITimeSet
@@ -83,6 +89,11 @@ namespace CodexDistTestCore
         {
             return TimeSpan.FromMinutes(5);
         }
+
+        public TimeSpan WaitForMetricTimeout()
+        {
+            return TimeSpan.FromSeconds(30);
+        }
     }
 
     public class LongTimeSet : ITimeSet
@@ -111,5 +122,10 @@ namespace CodexDistTestCore
         {
             return TimeSpan.FromMinutes(15);
         }
+
+        public TimeSpan WaitForMetricTimeout()
+        {
+            return TimeSpan.FromMinutes(5);
+        }
     }
 }

Tests/BasicTests/SimpleTests.cs

@@ -31,7 +31,7 @@ namespace Tests.BasicTests
         }
 
         [Test]
-        public void MetricsExample()
+        public void TwoMetricsExample()
         {
             var group = SetupCodexNodes(2)
                         .EnableMetrics()
@@ -53,15 +53,8 @@ namespace Tests.BasicTests
             primary.ConnectToPeer(secondary);
             primary2.ConnectToPeer(secondary2);
 
-            AssertWithTimeout(
+            metrics.AssertThat(primary, "libp2p_peers", Is.EqualTo(1));
-                () => metrics.GetMostRecentInt("libp2p_peers", primary),
+            metrics2.AssertThat(primary2, "libp2p_peers", Is.EqualTo(1));
-                isEqualTo: 1,
-                "Number of peers metric was incorrect.");
-
-            AssertWithTimeout(
-                () => metrics2.GetMostRecentInt("libp2p_peers", primary2),
-                isEqualTo: 1,
-                "Aaa");
         }
 
         [Test]