Refactor/disconnect reason metrics (#5859)

* DisconnectReason to EthDisconnectReason

* InitiateDisconnectReason to DisconnectReason

* Basic change of logic

* Make metrics with label

* Fix metrics

* Minor metric name change

* Cleanup

* More cleanup

* Fix test

* Unify the two dictionary type

* Addressing comment

* Fix dictionary metric not working

* Addressing comment

src/Nethermind/Nethermind.Blockchain/Synchronization/ISyncPeer.cs

@@ -31,7 +31,7 @@ public interface ISyncPeer : ITxPoolPeer, IPeerWithSatelliteProtocol
         bool IsPriority { get; set; }
         byte ProtocolVersion { get; }
         string ProtocolCode { get; }
-        void Disconnect(InitiateDisconnectReason reason, string details);
+        void Disconnect(DisconnectReason reason, string details);
         Task<BlockBody[]> GetBlockBodies(IReadOnlyList<Keccak> blockHashes, CancellationToken token);
         Task<BlockHeader[]> GetBlockHeaders(long number, int maxBlocks, int skip, CancellationToken token);
         Task<BlockHeader[]> GetBlockHeaders(Keccak startHash, int maxBlocks, int skip, CancellationToken token);

src/Nethermind/Nethermind.Core/Attributes/Metrics.cs

@@ -16,3 +16,17 @@ public sealed class CounterMetricAttribute : Attribute { }
 /// </summary>
 [AttributeUsage(AttributeTargets.Property | AttributeTargets.Field)]
 public sealed class GaugeMetricAttribute : Attribute { }
+
+/// <summary>
+/// Mark that the attribute is a dictionary whose key is used as a label of name LabelName.
+/// </summary>
+[AttributeUsage(AttributeTargets.Property | AttributeTargets.Field)]
+public sealed class KeyIsLabelAttribute : Attribute
+{
+    public string LabelName { get; }
+
+    public KeyIsLabelAttribute(string labelName)
+    {
+        LabelName = labelName;
+    }
+}

src/Nethermind/Nethermind.Core/Collections/LockableConcurrentDictionary.cs

@@ -105,6 +105,11 @@ public static class ConcurrentDictionaryExtensions
     public static ConcurrentDictionaryLock<TKey, TValue>.Lock AcquireLock<TKey, TValue>(this ConcurrentDictionary<TKey, TValue> dictionary)
         where TKey : notnull =>
         ConcurrentDictionaryLock<TKey, TValue>.Acquire(dictionary);
+
+    public static void Increment<TKey>(this ConcurrentDictionary<TKey, long> dictionary, TKey key) where TKey : notnull
+    {
+        dictionary.AddOrUpdate(key, 1, (_, value) => value + 1);
+    }
 }
 
 

src/Nethermind/Nethermind.Monitoring.Test/MetricsTests.cs

@@ -2,13 +2,16 @@
 // SPDX-License-Identifier: LGPL-3.0-only
 
 using System;
+using System.Collections;
+using System.Collections.Concurrent;
 using System.Collections.Generic;
 using System.IO;
 using System.Linq;
 using System.Reflection;
 using System.Runtime.Serialization;
 using FluentAssertions;
 using Nethermind.Core;
+using Nethermind.Core.Attributes;
 using Nethermind.Logging;
 using Nethermind.Monitoring.Config;
 using Nethermind.Monitoring.Metrics;
@@ -27,26 +30,60 @@ public static class TestMetrics
             [System.ComponentModel.Description("Another test description.")]
             [DataMember(Name = "one_two_three")]
             public static long OneTwoThreeSpecial { get; set; }
+
+            [System.ComponentModel.Description("Another test description.")]
+            [KeyIsLabel("somelabel")]
+            public static ConcurrentDictionary<SomeEnum, long> WithLabelledDictionary { get; set; } = new();
+
+            public static IDictionary<string, long> OldDictionaryMetrics { get; set; } = new ConcurrentDictionary<string, long>();
+        }
+
+        public enum SomeEnum
+        {
+            Option1,
+            Option2,
         }
 
         [Test]
-        public void Test_gauge_names()
+        public void Test_update_correct_gauge()
         {
             MetricsConfig metricsConfig = new()
             {
                 Enabled = true
             };
             MetricsController metricsController = new(metricsConfig);
             metricsController.RegisterMetrics(typeof(TestMetrics));
+
+            TestMetrics.OneTwoThree = 123;
+            TestMetrics.OneTwoThreeSpecial = 1234;
+            TestMetrics.WithLabelledDictionary[SomeEnum.Option1] = 2;
+            TestMetrics.WithLabelledDictionary[SomeEnum.Option2] = 3;
+            TestMetrics.OldDictionaryMetrics["metrics0"] = 4;
+            TestMetrics.OldDictionaryMetrics["metrics1"] = 5;
+            metricsController.UpdateMetrics(null);
+
             var gauges = metricsController._gauges;
             var keyDefault = $"{nameof(TestMetrics)}.{nameof(TestMetrics.OneTwoThree)}";
             var keySpecial = $"{nameof(TestMetrics)}.{nameof(TestMetrics.OneTwoThreeSpecial)}";
+            var keyDictionary = $"{nameof(TestMetrics)}.{nameof(TestMetrics.WithLabelledDictionary)}";
+            var keyOldDictionary0 = $"{nameof(TestMetrics.OldDictionaryMetrics)}.metrics0";
+            var keyOldDictionary1 = $"{nameof(TestMetrics.OldDictionaryMetrics)}.metrics1";
 
             Assert.Contains(keyDefault, gauges.Keys);
             Assert.Contains(keySpecial, gauges.Keys);
 
             Assert.That(gauges[keyDefault].Name, Is.EqualTo("nethermind_one_two_three"));
             Assert.That(gauges[keySpecial].Name, Is.EqualTo("one_two_three"));
+            Assert.That(gauges[keyDictionary].Name, Is.EqualTo("nethermind_with_labelled_dictionary"));
+            Assert.That(gauges[keyOldDictionary0].Name, Is.EqualTo("nethermind_metrics0"));
+            Assert.That(gauges[keyOldDictionary1].Name, Is.EqualTo("nethermind_metrics1"));
+
+            Assert.That(gauges[keyDefault].Value, Is.EqualTo(123));
+            Assert.That(gauges[keySpecial].Value, Is.EqualTo(1234));
+            Assert.That(gauges[keyDictionary].WithLabels(SomeEnum.Option1.ToString()).Value, Is.EqualTo(2));
+            Assert.That(gauges[keyDictionary].WithLabels(SomeEnum.Option2.ToString()).Value, Is.EqualTo(3));
+            Assert.That(gauges[keyOldDictionary0].Value, Is.EqualTo(4));
+            Assert.That(gauges[keyOldDictionary1].Value, Is.EqualTo(5));
         }
 
         [Test]

src/Nethermind/Nethermind.Monitoring/Metrics/MetricsController.cs

@@ -2,6 +2,7 @@
 // SPDX-License-Identifier: LGPL-3.0-only
 
 using System;
+using System.Collections;
 using System.Collections.Generic;
 using System.ComponentModel;
 using System.Diagnostics.Metrics;
@@ -23,14 +24,23 @@ public partial class MetricsController : IMetricsController
         private readonly int _intervalSeconds;
         private Timer _timer;
         private readonly Dictionary<Type, (MemberInfo, string, Func<double>)[]> _membersCache = new();
-        private readonly Dictionary<Type, (string DictName, IDictionary<string, long> Dict)> _dynamicPropCache = new();
+        private readonly Dictionary<Type, DictionaryMetricInfo[]> _dictionaryCache = new();
         private readonly HashSet<Type> _metricTypes = new();
 
         public readonly Dictionary<string, Gauge> _gauges = new();
         private readonly bool _useCounters;
 
         private readonly List<Action> _callbacks = new();
 
+        class DictionaryMetricInfo
+        {
+            internal MemberInfo MemberInfo;
+            internal string DictionaryName;
+            internal string LabelName;
+            internal string GaugeName;
+            internal IDictionary Dictionary;
+        }
+
         public void RegisterMetrics(Type type)
         {
             if (_metricTypes.Add(type) == false)
@@ -50,23 +60,29 @@ public void RegisterMetrics(Type type)
 
                 _gauges[gaugeName] = CreateMemberInfoMetricsGauge(member);
             }
+
+            foreach (DictionaryMetricInfo info in _dictionaryCache[type])
+            {
+                if (info.LabelName is null) continue; // Old behaviour creates new metric as it is created
+                _gauges[info.GaugeName] = CreateMemberInfoMetricsGauge(info.MemberInfo, info.LabelName);
+            }
         }
 
-        private static Gauge CreateMemberInfoMetricsGauge(MemberInfo member)
+        private static Gauge CreateMemberInfoMetricsGauge(MemberInfo member, params string[] labels)
         {
-            string description = member.GetCustomAttribute<DescriptionAttribute>()?.Description;
             string name = BuildGaugeName(member);
+            string description = member.GetCustomAttribute<DescriptionAttribute>()?.Description;
 
             bool haveTagAttributes = member.GetCustomAttributes<MetricsStaticDescriptionTagAttribute>().Any();
             if (!haveTagAttributes)
             {
-                return CreateGauge(name, description, _commonStaticTags);
+                return CreateGauge(name, description, _commonStaticTags, labels);
             }
 
             Dictionary<string, string> tags = new(_commonStaticTags);
             member.GetCustomAttributes<MetricsStaticDescriptionTagAttribute>().ForEach(attribute =>
                 tags.Add(attribute.Label, GetStaticMemberInfo(attribute.Informer, attribute.Label)));
-            return CreateGauge(name, description, tags);
+            return CreateGauge(name, description, tags, labels);
         }
 
         // Tags that all metrics share
@@ -138,13 +154,24 @@ static Func<double> GetValueAccessor(MemberInfo member)
                     .ToArray();
             }
 
-            if (!_dynamicPropCache.ContainsKey(type))
+            if (!_dictionaryCache.ContainsKey(type))
             {
-                var p = type.GetProperties().FirstOrDefault(p => p.PropertyType.IsAssignableTo(typeof(IDictionary<string, long>)));
-                if (p != null)
-                {
-                    _dynamicPropCache[type] = (p.Name, (IDictionary<string, long>)p.GetValue(null));
-                }
+                _dictionaryCache[type] = type.GetProperties()
+                    .Where(p =>
+                        p.PropertyType.IsGenericType &&
+                        (
+                            p.PropertyType.GetGenericTypeDefinition().IsAssignableTo(typeof(IDictionary))
+                            || p.PropertyType.GetGenericTypeDefinition().IsAssignableTo(typeof(IDictionary<,>))
+                        ))
+                    .Select(p => new DictionaryMetricInfo()
+                    {
+                        MemberInfo = p,
+                        DictionaryName = p.Name,
+                        LabelName = p.GetCustomAttribute<KeyIsLabelAttribute>()?.LabelName,
+                        GaugeName = GetGaugeNameKey(type.Name, p.Name),
+                        Dictionary = (IDictionary)p.GetValue(null)
+                    })
+                    .ToArray();
             }
         }
 
@@ -154,9 +181,9 @@ private static string BuildGaugeName(MemberInfo propertyInfo) =>
         private static string BuildGaugeName(string propertyName) =>
             $"nethermind_{GetGaugeNameRegex().Replace(propertyName, "$1_$2").ToLowerInvariant()}";
 
-        private static Gauge CreateGauge(string name, string help = null, IDictionary<string, string> labels = null) => labels is null
-            ? Prometheus.Metrics.CreateGauge(name, help ?? string.Empty)
-            : Prometheus.Metrics.WithLabels(labels).CreateGauge(name, help ?? string.Empty);
+        private static Gauge CreateGauge(string name, string help = null, IDictionary<string, string> staticLabels = null, params string[] labels) => staticLabels is null
+            ? Prometheus.Metrics.CreateGauge(name, help ?? string.Empty, labels)
+            : Prometheus.Metrics.WithLabels(staticLabels).CreateGauge(name, help ?? string.Empty, labels);
 
         public MetricsController(IMetricsConfig metricsConfig)
         {
@@ -195,28 +222,55 @@ private void UpdateMetrics(Type type)
                 ReplaceValueIfChanged(accessor(), gaugeName);
             }
 
-            if (_dynamicPropCache.TryGetValue(type, out var dict))
+            foreach (DictionaryMetricInfo info in _dictionaryCache[type])
             {
-                foreach (var kvp in dict.Dict)
+                if (info.LabelName is null)
                 {
-                    double value = Convert.ToDouble(kvp.Value);
-                    var gaugeName = GetGaugeNameKey(dict.DictName, kvp.Key);
-
-                    if (ReplaceValueIfChanged(value, gaugeName) is null)
+                    IDictionary dict = info.Dictionary;
+                    // Its fine that the key here need to call `ToString()`. Better here then in the metrics, where it might
+                    // impact the performance of whatever is updating the metrics.
+                    foreach (object keyObj in dict.Keys) // Different dictionary seems to iterate to different KV type. So need to use `Keys` here.
+                    {
+                        string keyStr = keyObj.ToString();
+                        double value = Convert.ToDouble(dict[keyObj]);
+                        string gaugeName = GetGaugeNameKey(info.DictionaryName, keyStr);
+
+                        if (ReplaceValueIfChanged(value, gaugeName) is null)
+                        {
+                            // Don't know why it does not prefix with dictionary name or class name. Not gonna change behaviour now.
+                            Gauge gauge = CreateGauge(BuildGaugeName(keyStr));
+                            _gauges[gaugeName] = gauge;
+                            gauge.Set(value);
+                        }
+                    }
+                }
+                else
+                {
+                    IDictionary dict = info.Dictionary;
+                    string gaugeName = info.GaugeName;
+                    foreach (object key in dict.Keys)
                     {
-                        Gauge gauge = CreateGauge(BuildGaugeName(kvp.Key));
-                        _gauges[gaugeName] = gauge;
-                        gauge.Set(value);
+                        double value = Convert.ToDouble(dict[key]);
+                        ReplaceValueIfChanged(value, gaugeName, key.ToString());
                     }
                 }
             }
 
-            Gauge ReplaceValueIfChanged(double value, string gaugeName)
+            Gauge ReplaceValueIfChanged(double value, string gaugeName, params string[] labels)
             {
                 if (_gauges.TryGetValue(gaugeName, out Gauge gauge))
                 {
-                    if (Math.Abs(gauge.Value - value) > double.Epsilon)
-                        gauge.Set(value);
+                    if (labels.Length > 0)
+                    {
+                        Gauge.Child ch = gauge.WithLabels(labels);
+                        if (Math.Abs(ch.Value - value) > double.Epsilon)
+                            ch.Set(value);
+                    }
+                    else
+                    {
+                        if (Math.Abs(gauge.Value - value) > double.Epsilon)
+                            gauge.Set(value);
+                    }
                 }
 
                 return gauge;