search

jaegertracing / jaeger-lib

A collection of shared infrastructure libraries used by different components of Jaeger.
https://github.com/uber/jaeger
Apache License 2.0
67 stars 35 forks source link

Add metric description to factory API #61

Closed objectiser closed 5 years ago

objectiser commented 5 years ago

Resolves #60

Signed-off-by: Gary Brown gary@brownuk.com

codecov[bot] commented 5 years ago

Codecov Report

Merging #61 into master will not change coverage. The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff          @@
##           master    #61   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files          25     25           
  Lines         694    711   +17     
=====================================
+ Hits          694    711   +17
Impacted Files Coverage Δ
metrics/tally/factory.go 100% <100%> (ø) :arrow_up:
metrics/metricstest/local.go 100% <100%> (ø) :arrow_up:
metrics/factory.go 100% <100%> (ø) :arrow_up:
metrics/adapters/tagless.go 100% <100%> (ø) :arrow_up:
metrics/adapters/factory.go 100% <100%> (ø) :arrow_up:
metrics/go-kit/factory.go 100% <100%> (ø) :arrow_up:
metrics/prometheus/factory.go 100% <100%> (ø) :arrow_up:
metrics/multi/multi.go 100% <100%> (ø) :arrow_up:
metrics/expvar/factory.go 100% <100%> (ø) :arrow_up:
metrics/metrics.go 100% <100%> (ø) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 1fc5c31...2dde290. Read the comment docs.

yurishkuro commented 5 years ago

I just mentioned in #62 - maybe we should consider using structs as arguments instead of explicit args.

objectiser commented 5 years ago

@yurishkuro Updated. Let me know if you would prefer different struct names, and whether would prefer no embedded type.

objectiser commented 5 years ago

@yurishkuro Updates applied. Did you mean the reflection here?

yurishkuro commented 5 years ago

@objectiser one minor thing was nagging at me - maybe we should use the name Help instead of Description? It will make the syntax more concise. I can do that change on top of yours.

objectiser commented 5 years ago

@yurishkuro SGTM - updated.

objectiser commented 5 years ago

@yurishkuro Changes applied.

yurishkuro commented 5 years ago

Nice!