Skip to content

Commit

Permalink
unexport StatsEntries, lock on gatherMetrics
Browse files Browse the repository at this point in the history
Signed-off-by: Alex Pana <8968914+acpana@users.noreply.github.com>
  • Loading branch information
acpana committed Feb 1, 2023
1 parent 4cdb922 commit ad4e3b7
Show file tree
Hide file tree
Showing 4 changed files with 34 additions and 15 deletions.
3 changes: 1 addition & 2 deletions constraint/pkg/client/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -588,8 +588,7 @@ func (c *Client) review(ctx context.Context, target string, constraints []*unstr

var statsEntries []*instrumentation.StatsEntry
if localDriver, ok := c.driver.(*local.Driver); ok {
statsEntries = make([]*instrumentation.StatsEntry, len(localDriver.StatsEntries))
copy(statsEntries, localDriver.StatsEntries)
statsEntries = localDriver.StatsEntries()
}

return &types.Response{
Expand Down
2 changes: 1 addition & 1 deletion constraint/pkg/client/drivers/local/args.go
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ func Externs(externs ...string) Arg {
func GatherMetrics() Arg {
return func(driver *Driver) error {
driver.gatherMetrics = true
driver.StatsEntries = make([]*instrumentation.StatsEntry, 0)
driver.statsEntries = make([]*instrumentation.StatsEntry, 0)

return nil
}
Expand Down
38 changes: 29 additions & 9 deletions constraint/pkg/client/drivers/local/driver.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,9 +77,24 @@ type Driver struct {
// gatherMetrics controls whether the driver attempts to gather any metrics around its API calls
gatherMetrics bool

// StatsEntries returns the current statistics observed since the last API call. At present
// StatsEntries are only gathered on the Query() API.
StatsEntries []*instrumentation.StatsEntry
// statsEntries returns the current statistics observed since the last API call. At present
// statsEntries are only gathered on the Query() API.
statsEntries []*instrumentation.StatsEntry
}

// StatsEntries returns any stats associated with the driver API if GatherMetrics
// driver arg was passed in. The only current endpoinut supported is Query().
func (d *Driver) StatsEntries() []*instrumentation.StatsEntry {
if d.gatherMetrics {
d.mtx.Lock()
defer d.mtx.Unlock()

cpy := make([]*instrumentation.StatsEntry, len(d.statsEntries))
copy(cpy, d.statsEntries)
return cpy
}

return nil
}

// AddTemplate adds templ to Driver. Normalizes modules into usable forms for
Expand Down Expand Up @@ -225,8 +240,16 @@ func (d *Driver) eval(ctx context.Context, compiler *ast.Compiler, target string
}

func (d *Driver) Query(ctx context.Context, target string, constraints []*unstructured.Unstructured, review interface{}, opts ...drivers.QueryOpt) ([]*types.Result, *string, error) {
// reset stats entries every time
d.StatsEntries = make([]*instrumentation.StatsEntry, 0)
if d.gatherMetrics {
d.mtx.Lock()
defer d.mtx.Unlock()

// reset stats entries every time
d.statsEntries = make([]*instrumentation.StatsEntry, 0)
} else {
d.mtx.RLock()
defer d.mtx.RUnlock()
}

if len(constraints) == 0 {
return nil, nil, nil
Expand All @@ -247,9 +270,6 @@ func (d *Driver) Query(ctx context.Context, target string, constraints []*unstru
return nil, nil, err
}

d.mtx.RLock()
defer d.mtx.RUnlock()

for kind, kindConstraints := range constraintsByKind {
evalStartTime := time.Now()
compiler := d.compilers.getCompiler(target, kind)
Expand Down Expand Up @@ -322,7 +342,7 @@ func (d *Driver) Query(ctx context.Context, target string, constraints []*unstru
},
}

d.StatsEntries = append(d.StatsEntries, statsEntry)
d.statsEntries = append(d.statsEntries, statsEntry)
}
}
}
Expand Down
6 changes: 3 additions & 3 deletions constraint/pkg/client/drivers/local/driver_unit_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,7 @@ func TestDriver_Query_StatsEntries(t *testing.T) {

// then expext two stats entries that correspond to the
// two constraints that were passed in to the Query call
ses := d.StatsEntries
ses := d.statsEntries
if len(ses) != 2 {
t.Fatalf("got %d errors on normal query; want 2", len(ses))
}
Expand Down Expand Up @@ -283,7 +283,7 @@ func TestDriver_Query_StatsEntries(t *testing.T) {

// then expect a new StatsEntry entirely, with no
// leaks of the previous data in it.
ses2 := d.StatsEntries
ses2 := d.statsEntries
if reflect.DeepEqual(ses, ses2) {
t.Fatalf("got the same slice on a new Query call")
}
Expand Down Expand Up @@ -340,7 +340,7 @@ func TestDriver_Query_StatsEntries_nonViolating(t *testing.T) {
}

// then we want to see a stats entry for the non violating constraint
ses := d.StatsEntries
ses := d.statsEntries
if len(ses) != 1 {
t.Fatalf("got %d errors on normal query; want 1", len(ses))
}
Expand Down

0 comments on commit ad4e3b7

Please sign in to comment.