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

clear min and max on every ExportView #1182

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

clear min and max on every ExportView #1182

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
1380fae
clearRows on every ExportView
marwan-at-work Oct 30, 2019
15a8df5
do not reset everything
marwan-at-work Nov 14, 2019
2be56c0
reset on config
marwan-at-work Nov 15, 2019
80c9ce6
Revert "reset on config"
marwan-at-work Nov 16, 2019
878d62c
reset only min/max
marwan-at-work Nov 16, 2019
7fae139
sort imports
marwan-at-work Nov 19, 2019
92ea5dc
pr comments + add tests
marwan-at-work Nov 19, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ require (
golang.org/x/net v0.0.0-20190620200207-3b0461eec859
golang.org/x/sys v0.0.0-20190502145724-3ef323f4f1fd // indirect
golang.org/x/text v0.3.2 // indirect
google.golang.org/appengine v1.4.0 // indirect
google.golang.org/genproto v0.0.0-20190425155659-357c62f0e4bb // indirect
google.golang.org/grpc v1.20.1
)
Expand Down
1 change: 1 addition & 0 deletions go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ google.golang.org/grpc v1.19.0/go.mod h1:mqu4LbDTu4XGKhr4mRzUsmM4RtVoemTSY81AxZi
google.golang.org/grpc v1.19.0/go.mod h1:mqu4LbDTu4XGKhr4mRzUsmM4RtVoemTSY81AxZiDr8c=
google.golang.org/grpc v1.20.1 h1:Hz2g2wirWK7H0qIIhGIqRGTuMwTE8HEKFnDZZ7lm9NU=
google.golang.org/grpc v1.20.1/go.mod h1:10oTOabMzJvdu6/UiuZezV6QK5dSlG84ov/aaiqXj38=
gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM=
gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
gopkg.in/yaml.v2 v2.2.2 h1:ZCJp+EgiOT7lHqUV2J862kp8Qj64Jo6az82+3Td9dZw=
gopkg.in/yaml.v2 v2.2.2/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=
Expand Down
11 changes: 11 additions & 0 deletions stats/view/collector.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
package view

import (
"math"
"sort"
"time"

Expand Down Expand Up @@ -56,6 +57,16 @@ func (c *collector) clearRows() {
c.signatures = make(map[string]AggregationData)
}

func (c *collector) resetValues() {
for _, ad := range c.signatures {
switch ad := ad.(type) {
case *DistributionData:
ad.Min = math.MaxFloat64
ad.Max = math.SmallestNonzeroFloat64
}
}
}

// encodeWithKeys encodes the map by using values
// only associated with the keys provided.
func encodeWithKeys(m *tag.Map, keys []tag.Key) []byte {
Expand Down
6 changes: 6 additions & 0 deletions stats/view/view.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,12 @@ func (v *viewInternal) clearRows() {
v.collector.clearRows()
}

// resetValues resets values that need to be
// completely isolated for each bucket
func (v *viewInternal) resetValues() {
v.collector.resetValues()
}

func (v *viewInternal) collectedRows() []*Row {
return v.collector.collectedRows(v.view.TagKeys)
}
Expand Down
1 change: 1 addition & 0 deletions stats/view/worker.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -233,6 +233,7 @@ func (w *worker) reportView(v *viewInternal, now time.Time) {
e.ExportView(viewData)
}
exportersMu.Unlock()
v.resetValues()
Copy link
Contributor

Choose a reason for hiding this comment

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

  1. This should be also called from w.Read() method. This is for new mechanism (ExportMetrics)
  2. Apart from that reportUsage() should not go through all the views unless there is an exporter registered (used for old mechanism ExportView())
    If user configures both ways or multiple exporter using new mechanism then values of min/max is unpredictable.
  3. Min/Max doc should be updated to reflect the behavior.
  4. Add tests for min/max.

Copy link
Author

Choose a reason for hiding this comment

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

@rghetia done, let me know if the last commit is what you were looking for 👍

Copy link
Contributor

Choose a reason for hiding this comment

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

test is good.
Item 2 above is not taken care of.

There should be new option in ReadAndExport method. If the option to reset min/max is specified then only do the reset.

Similarly, for old approach (export viewdata) there should be a new method view.ResetMinMaxOnExport() - It is rather a long name but I don't have a good suggestion. Alternatively, provide reset functionality only using new approach with ReadAndExport().

}

func (w *worker) reportUsage(now time.Time) {
Expand Down