Skip to content
This repository has been archived by the owner on Jul 31, 2023. It is now read-only.

Commit

Permalink
Hide Measure fields. (#528)
Browse files Browse the repository at this point in the history
We don't want to expose Measurement fields or allow direct construction
of Measurement instances because we want to keep open the possibility
of adding new Measurement representations and also encourage users to
use the .M() method.

Also saw that we make some performance promises about Measure that are
not really true. Looks like there was the beginnings of an implementation
of the fast path for skipping recording but it was never fully hooked
up.
  • Loading branch information
Ramon Nogueira committed Mar 8, 2018
1 parent c863f21 commit 495804e
Show file tree
Hide file tree
Showing 6 changed files with 22 additions and 19 deletions.
16 changes: 13 additions & 3 deletions stats/measure.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ type measure struct {
name string
description string
unit string
views int32
}

// Name returns the name of the measure.
Expand All @@ -69,6 +68,7 @@ var (
errMeasureNameTooLong = fmt.Errorf("measure name cannot be longer than %v", internal.MaxNameLength)
)

// FindMeasure finds the Measure instance, if any, associated with the given name.
func FindMeasure(name string) Measure {
mu.RLock()
m := measures[name]
Expand All @@ -91,8 +91,18 @@ func register(m Measure) (Measure, error) {
// provides methods to create measurements of their kind. For example, Int64Measure
// provides M to convert an int64 into a measurement.
type Measurement struct {
Value float64
Measure Measure
v float64
m Measure
}

// Value returns the value of the Measurement as a float64.
func (m Measurement) Value() float64 {
return m.v
}

// Measure returns the Measure from which this Measurement was created.
func (m Measurement) Measure() Measure {
return m.m
}

func checkName(name string) error {
Expand Down
2 changes: 1 addition & 1 deletion stats/measure_float64.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ type Float64Measure struct {
// M creates a new float64 measurement.
// Use Record to record measurements.
func (m *Float64Measure) M(v float64) Measurement {
return Measurement{Measure: m, Value: v}
return Measurement{m: m, v: v}
}

// Float64 creates a new measure of type Float64Measure. It returns
Expand Down
2 changes: 1 addition & 1 deletion stats/measure_int64.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ type Int64Measure struct {
// M creates a new int64 measurement.
// Use Record to record measurements.
func (m *Int64Measure) M(v int64) Measurement {
return Measurement{Measure: m, Value: float64(v)}
return Measurement{m: m, v: float64(v)}
}

// Int64 creates a new measure of type Int64Measure. It returns an
Expand Down
5 changes: 5 additions & 0 deletions stats/measure_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,11 @@ func TestCheckMeasureName(t *testing.T) {
view: "my.org/measures/\007",
wantErr: true,
},
{
name: "no emoji for you!",
view: "💩",
wantErr: true,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
Expand Down
12 changes: 0 additions & 12 deletions stats/record.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,20 +24,8 @@ import (

// Record records one or multiple measurements with the same tags at once.
// If there are any tags in the context, measurements will be tagged with them.
//
// Record returns fast and recorded measurements will have little overhead
// if there are no views assicaiated with them.
func Record(ctx context.Context, ms ...Measurement) {
if internal.DefaultRecorder != nil {
var record bool
for _, m := range ms {
if m.Measure != nil {
record = true
}
}
if !record {
return
}
internal.DefaultRecorder(tag.FromContext(ctx), ms)
}
}
4 changes: 2 additions & 2 deletions stats/view/worker_commands.go
Original file line number Diff line number Diff line change
Expand Up @@ -135,9 +135,9 @@ type recordReq struct {

func (cmd *recordReq) handleCommand(w *worker) {
for _, m := range cmd.ms {
ref := w.getMeasureRef(m.Measure.Name())
ref := w.getMeasureRef(m.Measure().Name())
for v := range ref.views {
v.addSample(cmd.tm, m.Value)
v.addSample(cmd.tm, m.Value())
}
}
}
Expand Down

0 comments on commit 495804e

Please sign in to comment.