Skip to content
This repository was archived by the owner on Aug 17, 2025. It is now read-only.

Conversation

yurishkuro
Copy link
Member

@yurishkuro yurishkuro commented Feb 9, 2017

This implements the pattern I was asking for in go-kit/kit#378, simplified for our needs.

@coveralls
Copy link

coveralls commented Feb 9, 2017

Coverage Status

Coverage remained the same at 100.0% when pulling a1f91f4 on gokit-expvar-metrics into 014e46f on master.

@coveralls
Copy link

coveralls commented Feb 9, 2017

Coverage Status

Coverage remained the same at 100.0% when pulling 553b708 on gokit-expvar-metrics into 014e46f on master.

@coveralls
Copy link

coveralls commented Feb 9, 2017

Coverage Status

Coverage remained the same at 100.0% when pulling b4ed911 on gokit-expvar-metrics into 014e46f on master.

glide.yaml Outdated
import:
- package: github.com/codahale/hdrhistogram
- package: github.com/go-kit/kit
subpackages:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it generated this without any subpackages?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorry, I tried to add them manually, but glide complained. Removed.

return f.scope + "_" + name
}

func (f factory) Counter(name string, tags map[string]string) metrics.Counter {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does go-kit metrics not have a way to deal with tags?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oops, you're correct, I got focused on expvar implementation that does not support tags

return xkit.NewGauge(kit.NewGauge(f.subScope(name)))
}

// Namespace is a no-op for expvar, since i
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

eh?

@CLAassistant
Copy link

CLAassistant commented Feb 9, 2017

CLA assistant check
All committers have signed the CLA.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 3300ca6 on gokit-expvar-metrics into 014e46f on master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 3300ca6 on gokit-expvar-metrics into 014e46f on master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 90bfb6f on gokit-expvar-metrics into 014e46f on master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling e84e928 on gokit-expvar-metrics into 014e46f on master.

Copy link

@badiib badiib left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@yurishkuro yurishkuro merged commit 391c4d8 into master Feb 9, 2017
@yurishkuro yurishkuro deleted the gokit-expvar-metrics branch August 22, 2017 18:06
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants