Add metric description to factory API #61
Conversation
Signed-off-by: Gary Brown <gary@brownuk.com>
Codecov Report
@@ Coverage Diff @@
## master #61 +/- ##
=====================================
Coverage 100% 100%
=====================================
Files 25 25
Lines 694 711 +17
=====================================
+ Hits 694 711 +17
Continue to review full report at Codecov.
|
|
I just mentioned in #62 - maybe we should consider using structs as arguments instead of explicit args. |
Signed-off-by: Gary Brown <gary@brownuk.com>
objectiser
force-pushed
the
metricsdesc
branch
from
160a047 to
72aa500
Compare
|
@yurishkuro Updated. Let me know if you would prefer different struct names, and whether would prefer no embedded type. |
metrics/factory.go
Outdated
| @@ -14,22 +14,40 @@ | |||
|
|
|||
| package metrics | |||
|
|
|||
| // MetricScope defines the name and tags map associated with a metric | |||
| type MetricScope struct { | |||
There was a problem hiding this comment.
I think this is a bit overkill, makes calls to Counter() etc a lot uglier.
metrics/factory.go
Outdated
| } | ||
|
|
||
| // MetricInfo defines the information associated with a metric | ||
| type MetricInfo struct { |
There was a problem hiding this comment.
I'd move Name/Tags here. Also can it be renamed to Options? So that the syntax doesn't stutter:
c := factory.Counter(metrics.Options{Name: .., Description: ...})
…de the Options. Signed-off-by: Gary Brown <gary@brownuk.com>
|
@yurishkuro Updates applied. Did you mean the reflection here? |
|
@objectiser one minor thing was nagging at me - maybe we should use the name |
Signed-off-by: Gary Brown <gary@brownuk.com>
|
@yurishkuro SGTM - updated. |
| } | ||
| func (nullFactory) Gauge(options Options) Gauge { | ||
| return NullGauge | ||
| } |
There was a problem hiding this comment.
Autoformatting by vscode - but seems to accept manually putting it back to single line.
metrics/factory.go
Outdated
| @@ -14,22 +14,41 @@ | |||
|
|
|||
| package metrics | |||
|
|
|||
| // Scope defines the name and tags map associated with a metric | |||
| type Scope struct { | |||
There was a problem hiding this comment.
Maybe something like NSOptions instead? Scope sounds like we’re introducing another concept
Signed-off-by: Gary Brown <gary@brownuk.com>
|
@yurishkuro Changes applied. |
|
Nice! |
Resolves #60
Signed-off-by: Gary Brown gary@brownuk.com