Version Metrics updated (#4860) #4870
Conversation
Implemented a system for resolving #4860 needs
Added exceptions to make sure that only initialised fields are used for metrics
src/Nethermind/Nethermind.Monitoring/Metrics/MetricsController.cs
Outdated
Show resolved
Hide resolved
| configuration.StaticLabels = tagValues; | ||
|
|
||
| string description = propertyInfo.GetCustomAttribute<DescriptionAttribute>()?.Description; | ||
| _gauges[gaugeName] = CreateGauge(BuildGaugeName(propertyInfo.Name), description, configuration); |
There was a problem hiding this comment.
Naming: Get method creates something inside, can we keep it that it returns it higher where it is inserted to dictionary?
There was a problem hiding this comment.
Thank you. Yeah, it is primarily a builder - it creates a Gauge for metrics, I will rename it properly.
There was a problem hiding this comment.
hm.. would still make this return CreateGauge(BuildGaugeName(propertyInfo.Name), description, configuration); and set _gauges[gaugeName] = CreateMemberInfoMectricsGauge() in the call method to be more readable with less side effects.
src/Nethermind/Nethermind.Monitoring/Metrics/MetricsController.cs
Outdated
Show resolved
Hide resolved
src/Nethermind/Nethermind.Monitoring/Metrics/MetricsController.cs
Outdated
Show resolved
Hide resolved
src/Nethermind/Nethermind.Monitoring/Metrics/MetricsController.cs
Outdated
Show resolved
Hide resolved
1) throw concrete exceptions 2) Naming 3) Unused removal 4) Function simplified 5) standarizing is all over the solution
|
@LukaszRozmej added fixes following your review. Please take a look |
OlegJakushkin
force-pushed
the
fix/issue-4860
branch
from
347f812
to
4400fd2
Compare
| configuration.StaticLabels = tagValues; | ||
|
|
||
| string description = propertyInfo.GetCustomAttribute<DescriptionAttribute>()?.Description; | ||
| _gauges[gaugeName] = CreateGauge(BuildGaugeName(propertyInfo.Name), description, configuration); |
There was a problem hiding this comment.
hm.. would still make this return CreateGauge(BuildGaugeName(propertyInfo.Name), description, configuration); and set _gauges[gaugeName] = CreateMemberInfoMectricsGauge() in the call method to be more readable with less side effects.
|
Not sure why you did |
Implemented a system for resolving #4860 needs
Resolves #4860
Added a system that allows for Static Metrics labelling.
Now versioning looks like this:
Changes:
Types of changes
Testing
Cosmetic data publishing change for external metric tools. Code covered by previously existing tests
In case you checked yes, did you write tests??
Tested on local Prometheus