Merge pull request #9353 from mono/backport-pr-9314-to-release-8.4

[release-8.4] Instead of using custom ISerializable to serialize private fields, just expose it via ReadOnlyCollection

3 files changed, 48 insertions, 74 deletions

diff --git a/main/src/core/MonoDevelop.Core/MonoDevelop.Core.Instrumentation/Counter.cs b/main/src/core/MonoDevelop.Core/MonoDevelop.Core.Instrumentation/Counter.cs
index 7e5e81a677..cbb8a2afa5 100644
--- a/main/src/core/MonoDevelop.Core/MonoDevelop.Core.Instrumentation/Counter.cs
+++ b/main/src/core/MonoDevelop.Core/MonoDevelop.Core.Instrumentation/Counter.cs
@@ -28,6 +28,7 @@
 
 using System;
 using System.Collections.Generic;
+using System.Collections.ObjectModel;
 using System.IO;
 using System.Linq;
 using System.Runtime.Serialization;
@@ -35,7 +36,7 @@ using System.Runtime.Serialization;
 namespace MonoDevelop.Core.Instrumentation
 {
 	[Serializable]
-	public class Counter: MarshalByRefObject, ISerializable
+	public class Counter: MarshalByRefObject
 	{
 		internal int count;
 		internal int totalCount;
@@ -99,22 +100,30 @@ namespace MonoDevelop.Core.Instrumentation
 			get { return this.logMessages; }
 			set { this.logMessages = value; }
 		}
-		
+
 		public int Count {
 			get { return count; }
 		}
-		
+
 		public bool Disposed {
 			get { return disposed; }
 			internal set { disposed = value; }
 		}
-		
+
 		public int TotalCount {
 			get { return totalCount; }
 		}
 
 		public virtual CounterDisplayMode DisplayMode => CounterDisplayMode.Block;
-		
+
+		public IReadOnlyList<CounterValue> AllValues {
+			get {
+				lock (values) {
+					return new ReadOnlyCollection<CounterValue> (new List<CounterValue> (values));
+				}
+			}
+		}
+
 		public IEnumerable<CounterValue> GetValues ()
 		{
 			lock (values) {
@@ -353,26 +362,6 @@ namespace MonoDevelop.Core.Instrumentation
 		{
 			return null;
 		}
-
-		public virtual void GetObjectData (SerializationInfo info, StreamingContext context)
-			=> PopulateSerializableMembers (info, context);
-
-		protected void PopulateSerializableMembers (SerializationInfo info, StreamingContext context)
-		{
-			info.AddValue (nameof (this.StoreValues), this.StoreValues);
-			info.AddValue (nameof (this.Resolution), this.Resolution);
-			info.AddValue (nameof (this.values), this.values);
-			info.AddValue (nameof (this.TotalCount), this.TotalCount);
-			info.AddValue (nameof (this.Name), this.Name);
-			info.AddValue (nameof (this.LogMessages), this.LogMessages);
-			info.AddValue (nameof (this.LastValue), this.LastValue);
-			info.AddValue (nameof (this.Id), this.Id);
-			info.AddValue (nameof (this.Handlers), this.Handlers);
-			info.AddValue (nameof (this.Category), this.Category);
-			info.AddValue (nameof (this.Count), this.Count);
-			info.AddValue (nameof (this.DisplayMode), this.DisplayMode);
-			info.AddValue (nameof (this.Enabled), this.Enabled);
-		}
 	}
 
 	public class Counter<T>: Counter where T : CounterMetadata, new()
diff --git a/main/src/core/MonoDevelop.Core/MonoDevelop.Core.Instrumentation/TimerCounter.cs b/main/src/core/MonoDevelop.Core/MonoDevelop.Core.Instrumentation/TimerCounter.cs
index b1dd9f4c79..23f4ffa3f0 100644
--- a/main/src/core/MonoDevelop.Core/MonoDevelop.Core.Instrumentation/TimerCounter.cs
+++ b/main/src/core/MonoDevelop.Core/MonoDevelop.Core.Instrumentation/TimerCounter.cs
@@ -38,7 +38,7 @@ using System.Runtime.Serialization;
 namespace MonoDevelop.Core.Instrumentation
 {
 	[Serializable]
-	public class TimerCounter : Counter , ISerializable
+	public class TimerCounter : Counter
 	{
 		double minSeconds;
 		TimeSpan totalTime;
@@ -142,17 +142,6 @@ namespace MonoDevelop.Core.Instrumentation
 			}
 			return c;
 		}
-
-		public override void GetObjectData (SerializationInfo info, StreamingContext context)
-		{
-			base.PopulateSerializableMembers (info, context);
-			info.AddValue (nameof (this.MinSeconds), this.MinSeconds);
-			info.AddValue (nameof (this.TotalTime), this.TotalTime);
-			info.AddValue (nameof (this.AverageTime), this.AverageTime);
-			info.AddValue (nameof (this.MinTime), this.MinTime);
-			info.AddValue (nameof (this.MaxTime), this.MaxTime);
-			info.AddValue (nameof (this.CountWithDuration), this.CountWithDuration);
-		}
 	}
 
 	[Serializable]
diff --git a/main/tests/MonoDevelop.Core.Tests/MonoDevelop.Core/InstrumentationTests.cs b/main/tests/MonoDevelop.Core.Tests/MonoDevelop.Core/InstrumentationTests.cs
index 3e1a734703..6415ae6c47 100644
--- a/main/tests/MonoDevelop.Core.Tests/MonoDevelop.Core/InstrumentationTests.cs
+++ b/main/tests/MonoDevelop.Core.Tests/MonoDevelop.Core/InstrumentationTests.cs
@@ -31,6 +31,7 @@ using System.Threading;
 using Newtonsoft.Json.Linq;
 using System.IO;
 using Newtonsoft.Json;
+using System.Linq;
 
 namespace MonoDevelop.Core
 {
@@ -226,6 +227,12 @@ namespace MonoDevelop.Core
 		[Test]
 		public void SerializeCounters ()
 		{
+			InstrumentationService.Enabled = true;
+			var timer = InstrumentationService.CreateCounter<CustomCounterMetadata> ("TestCounter", "IDEGroup", id: "IDE.TestCounter");
+			timer.Inc (1, "First Trace", new CustomCounterMetadata () { SomeMeasure = 1 });
+			timer.Inc (1, "Second Trace", new CustomCounterMetadata () { SomeMeasure = 2 });
+			timer.Inc (1, "Third Trace", new CustomCounterMetadata () { SomeMeasure = 3 });
+
 			InstrumentationService.SaveJson ("serialize_counters.json");
 			using (var textReader = new StreamReader("serialize_counters.json")) {
 				using (var jsonTextReader = new JsonTextReader (textReader)) {
@@ -238,54 +245,43 @@ namespace MonoDevelop.Core
 
 					var counters = jsonRootObj ["Counters"];
 					Assert.IsNotNull (counters);
-					var runtimeCounter = counters ["Runtime initialization"];
-					var actualRuntimeInitializationCounter = InstrumentationService.GetCounter ("Runtime initialization");
-					Assert.IsNotNull (runtimeCounter);
-
-					var storeValuesToken = runtimeCounter ["StoreValues"];
-					Assert.IsNotNull (storeValuesToken);
-					Assert.That (bool.Parse(storeValuesToken.ToString ()), Is.EqualTo (actualRuntimeInitializationCounter.StoreValues));
+					var testCounter = counters ["TestCounter"];
+					var actualTestCounter = InstrumentationService.GetCounter ("TestCounter");
+					Assert.IsNotNull (actualTestCounter);
 
-					var resolutionToken = runtimeCounter ["Resolution"];
-					Assert.IsNotNull (resolutionToken);
-					Assert.That (resolutionToken.ToString (), Is.EqualTo (actualRuntimeInitializationCounter.Resolution.ToString()));
+					var storeValuesToken = testCounter ["StoreValues"];
+					Assert.That (bool.Parse(storeValuesToken.ToString ()), Is.EqualTo (actualTestCounter.StoreValues));
 
-					var totalCountToken = runtimeCounter ["TotalCount"];
+					var totalCountToken = testCounter ["TotalCount"];
 					Assert.IsNotNull (totalCountToken);
-					Assert.That (int.Parse (totalCountToken.ToString ()), Is.EqualTo (actualRuntimeInitializationCounter.TotalCount));
+					Assert.That (int.Parse (totalCountToken.ToString ()), Is.EqualTo (actualTestCounter.TotalCount));
 
-					var nameToken = runtimeCounter ["Name"];
+					var nameToken = testCounter ["Name"];
 					Assert.IsNotNull (nameToken);
-					Assert.That (nameToken.ToString(), Is.EqualTo (actualRuntimeInitializationCounter.Name));
+					Assert.That (nameToken.ToString(), Is.EqualTo (actualTestCounter.Name));
 
-					var idToken = runtimeCounter ["Id"];
+					var idToken = testCounter ["Id"];
 					Assert.IsNotNull (idToken);
-					Assert.That (idToken.ToString (), Is.EqualTo (actualRuntimeInitializationCounter.Id));
+					Assert.That (idToken.ToString (), Is.EqualTo (actualTestCounter.Id));
 
-					var countToken = runtimeCounter ["Count"];
+					var countToken = testCounter ["Count"];
 					Assert.IsNotNull (countToken);
-					Assert.That (int.Parse (countToken.ToString ()), Is.EqualTo (actualRuntimeInitializationCounter.Count));
-
-					var displayModeToken = runtimeCounter ["DisplayMode"];
-					Assert.IsNotNull (displayModeToken);
-					Assert.That ((CounterDisplayMode)Enum.Parse (typeof(CounterDisplayMode), displayModeToken.ToString ()), Is.EqualTo (actualRuntimeInitializationCounter.DisplayMode));
+					Assert.That (int.Parse (countToken.ToString ()), Is.EqualTo (actualTestCounter.Count));
 
-					var enabledToken = runtimeCounter ["Enabled"];
+					var enabledToken = testCounter ["Enabled"];
 					Assert.IsNotNull (enabledToken);
-					Assert.That (bool.Parse (enabledToken.ToString ()), Is.EqualTo (actualRuntimeInitializationCounter.Enabled));
-
-					Assert.IsNotNull (runtimeCounter ["Category"]);
-					Assert.IsNotNull (runtimeCounter ["MinSeconds"]);
-					Assert.IsNotNull (runtimeCounter ["TotalTime"]);
-					Assert.IsNotNull (runtimeCounter ["AverageTime"]);
-					Assert.IsNotNull (runtimeCounter ["MinTime"]);
-					Assert.IsNotNull (runtimeCounter ["MaxTime"]);
-					Assert.IsNotNull (runtimeCounter ["CountWithDuration"]);
-
-					Assert.IsNotNull (runtimeCounter ["values"]);
-					Assert.IsNotNull (runtimeCounter ["LastValue"]);
-					Assert.IsNotNull (runtimeCounter ["LogMessages"]);
-					Assert.IsNotNull (runtimeCounter ["Handlers"]);
+					Assert.That (bool.Parse (enabledToken.ToString ()), Is.EqualTo (actualTestCounter.Enabled));
+
+					Assert.IsNotNull (testCounter ["Category"]);
+					Assert.IsNotNull (testCounter ["AllValues"]);
+
+					foreach (var val in testCounter ["AllValues"]) {
+						var timeStamp = DateTime.Parse (val ["TimeStamp"].ToString());
+						var message = val ["Message"].ToString ();
+						var metaData = (int)val ["Metadata"] ["SomeMeasure"];
+						Assert.That (actualTestCounter.AllValues
+							.FirstOrDefault (x => x.Message == message && (int)x.Metadata["SomeMeasure"] == metaData && x.TimeStamp == timeStamp), Is.Not.Null);
+					}
 				}
 			}
 		}