Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pull out ...WithExemplar methods into separate interfaces #708

Merged
merged 1 commit into from Jan 27, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 7 additions & 3 deletions examples/random/main.go
Expand Up @@ -90,9 +90,13 @@ func main() {
for {
v := (rand.NormFloat64() * *normDomain) + *normMean
rpcDurations.WithLabelValues("normal").Observe(v)
rpcDurationsHistogram.ObserveWithExemplar(
// Demonstrate exemplar support with a dummy ID. This would be
// something like a trace ID in a real application.
// Demonstrate exemplar support with a dummy ID. This
// would be something like a trace ID in a real
// application. Note the necessary type assertion. We
// already know that rpcDurationsHistogram implements
// the ExemplarObserver interface and thus don't need to
// check the outcome of the tipe assertion.
rpcDurationsHistogram.(prometheus.ExemplarObserver).ObserveWithExemplar(
Copy link
Member

@bwplotka bwplotka Jan 27, 2020

Choose a reason for hiding this comment

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

I would maybe in this example check if it's actually implementing this? So:

if e, ok := rpcDurationsHistogram.(prometheus.ExemplarObserver); ok {
  e..ObserveWithExemplar( ...
}

Copy link
Member

@bwplotka bwplotka Jan 27, 2020

Choose a reason for hiding this comment

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

This is because encouraging avoiding panics in user's code is nice... But one can argue that we directly call our library constructor, so up to you here, not a blocker (:

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, it's all in the same file. I have added a comment now that explains why we don't check the type assertion.

Copy link
Member Author

Choose a reason for hiding this comment

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

And I'll also fix the "tipo". :o(

Copy link
Member

Choose a reason for hiding this comment

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

Thanks!

v, prometheus.Labels{"dummyID": fmt.Sprint(rand.Intn(100000))},
)
time.Sleep(time.Duration(75*oscillationFactor()) * time.Millisecond)
Expand Down
22 changes: 15 additions & 7 deletions prometheus/counter.go
Expand Up @@ -41,13 +41,18 @@ type Counter interface {
// Add adds the given value to the counter. It panics if the value is <
// 0.
Add(float64)
// AddWithExemplar works like Add but also replaces the currently saved
// exemplar (if any) with a new one, created from the provided value,
// the current time as timestamp, and the provided labels. Empty Labels
// will lead to a valid (label-less) exemplar. But if Labels is nil, the
// current exemplar is left in place. This method panics if the value is
// < 0, if any of the provided labels are invalid, or if the provided
// labels contain more than 64 runes in total.
}

// ExemplarAdder is implemented by Counters that offer the option of adding a
// value to the Counter together with an exemplar. Its AddWithExemplar method
// works like the Add method of the Counter interface but also replaces the
// currently saved exemplar (if any) with a new one, created from the provided
// value, the current time as timestamp, and the provided labels. Empty Labels
// will lead to a valid (label-less) exemplar. But if Labels is nil, the current
// exemplar is left in place. AddWithExemplar panics if the value is < 0, if any
// of the provided labels are invalid, or if the provided labels contain more
// than 64 runes in total.
type ExemplarAdder interface {
AddWithExemplar(value float64, exemplar Labels)
}

Expand All @@ -56,6 +61,9 @@ type CounterOpts Opts

// NewCounter creates a new Counter based on the provided CounterOpts.
//
// The returned implementation also implements ExemplarAdder. It is safe to
// perform the corresponding type assertion.
//
// The returned implementation tracks the counter value in two separate
// variables, a float64 and a uint64. The latter is used to track calls of the
// Inc method and calls of the Add method with a value that can be represented
Expand Down
13 changes: 4 additions & 9 deletions prometheus/histogram.go
Expand Up @@ -48,15 +48,6 @@ type Histogram interface {

// Observe adds a single observation to the histogram.
Observe(float64)
// ObserveWithExemplar works like Observe but also replaces the
// currently saved exemplar for the relevant bucket (possibly none) with
// a new one, created from the provided value, the current time as
// timestamp, and the provided Labels. Empty Labels will lead to a valid
// (label-less) exemplar. But if Labels is nil, the current exemplar in
// the relevant bucket is left in place. This method panics if any of
// the provided labels are invalid or if the provided labels contain
// more than 64 runes in total.
ObserveWithExemplar(value float64, exemplar Labels)
}

// bucketLabel is used for the label that defines the upper bound of a
Expand Down Expand Up @@ -161,6 +152,10 @@ type HistogramOpts struct {

// NewHistogram creates a new Histogram based on the provided HistogramOpts. It
// panics if the buckets in HistogramOpts are not in strictly increasing order.
//
// The returned implementation also implements ExemplarObserver. It is safe to
// perform the corresponding type assertion. Exemplars are tracked separately
// for each bucket.
func NewHistogram(opts HistogramOpts) Histogram {
return newHistogram(
NewDesc(
Expand Down
2 changes: 1 addition & 1 deletion prometheus/histogram_test.go
Expand Up @@ -189,7 +189,7 @@ func TestHistogramConcurrency(t *testing.T) {
if n%2 == 0 {
sum.Observe(v)
} else {
sum.ObserveWithExemplar(v, Labels{"foo": "bar"})
sum.(ExemplarObserver).ObserveWithExemplar(v, Labels{"foo": "bar"})
}
}
end.Done()
Expand Down
12 changes: 12 additions & 0 deletions prometheus/observer.go
Expand Up @@ -50,3 +50,15 @@ type ObserverVec interface {

Collector
}

// ExemplarObserver is implemented by Observers that offer the option of
// observing a value together with an exemplar. Its ObserveWithExemplar method
// works like the Observe method of an Observer but also replaces the currently
// saved exemplar (if any) with a new one, created from the provided value, the
// current time as timestamp, and the provided Labels. Empty Labels will lead to
// a valid (label-less) exemplar. But if Labels is nil, the current exemplar is
// left in place. ObserveWithExemplar panics if any of the provided labels are
// invalid or if the provided labels contain more than 64 runes in total.
type ExemplarObserver interface {
ObserveWithExemplar(value float64, exemplar Labels)
}