Skip to content

Commit

Permalink
Plumb metrics through loader and bundle helpers
Browse files Browse the repository at this point in the history
This corrects the missing time in rego_module_parse timers as we now
have metrics collecting info as we parse *.rego files from file
loaders and from bundles as they are unpacked.

It also adds in a timer for the data files that are loaded through
similar mechanisms.

Signed-off-by: Patrick East <[email protected]>
  • Loading branch information
patrick-east committed Dec 12, 2019
1 parent f01610e commit 9c85dfc
Show file tree
Hide file tree
Showing 11 changed files with 159 additions and 92 deletions.
27 changes: 24 additions & 3 deletions bundle/bundle.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (

"github.com/open-policy-agent/opa/internal/file/archive"
"github.com/open-policy-agent/opa/internal/merge"
"github.com/open-policy-agent/opa/metrics"

"github.com/pkg/errors"

Expand Down Expand Up @@ -125,6 +126,7 @@ type ModuleFile struct {
type Reader struct {
loader DirectoryLoader
includeManifestInData bool
metrics metrics.Metrics
}

// NewReader returns a new Reader which is configured for reading tarballs.
Expand All @@ -136,7 +138,8 @@ func NewReader(r io.Reader) *Reader {
// specified DirectoryLoader.
func NewCustomReader(loader DirectoryLoader) *Reader {
nr := Reader{
loader: loader,
loader: loader,
metrics: metrics.New(),
}
return &nr
}
Expand All @@ -148,6 +151,12 @@ func (r *Reader) IncludeManifestInData(includeManifestInData bool) *Reader {
return r
}

// WithMetrics sets the metrics object to be used while loading bundles
func (r *Reader) WithMetrics(m metrics.Metrics) *Reader {
r.metrics = m
return r
}

// Read returns a new Bundle loaded from the reader.
func (r *Reader) Read() (Bundle, error) {

Expand Down Expand Up @@ -177,7 +186,9 @@ func (r *Reader) Read() (Bundle, error) {
path := filepath.ToSlash(f.Path())

if strings.HasSuffix(path, RegoExt) {
r.metrics.Timer(metrics.RegoModuleParse).Start()
module, err := ast.ParseModule(path, buf.String())
r.metrics.Timer(metrics.RegoModuleParse).Stop()
if err != nil {
return bundle, err
}
Expand All @@ -194,7 +205,12 @@ func (r *Reader) Read() (Bundle, error) {

} else if filepath.Base(path) == dataFile {
var value interface{}
if err := util.NewJSONDecoder(&buf).Decode(&value); err != nil {

r.metrics.Timer(metrics.RegoDataParse).Start()
err := util.NewJSONDecoder(&buf).Decode(&value)
r.metrics.Timer(metrics.RegoDataParse).Stop()

if err != nil {
return bundle, errors.Wrapf(err, "bundle load failed on %v", path)
}

Expand All @@ -205,7 +221,12 @@ func (r *Reader) Read() (Bundle, error) {
} else if filepath.Base(path) == yamlDataFile {

var value interface{}
if err := util.Unmarshal(buf.Bytes(), &value); err != nil {

r.metrics.Timer(metrics.RegoDataParse).Start()
err := util.Unmarshal(buf.Bytes(), &value)
r.metrics.Timer(metrics.RegoDataParse).Stop()

if err != nil {
return bundle, errors.Wrapf(err, "bundle load failed on %v", path)
}

Expand Down
4 changes: 2 additions & 2 deletions cmd/check.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ func checkModules(args []string) int {

if checkParams.bundleMode {
for _, path := range args {
b, err := loader.AsBundle(path)
b, err := loader.NewFileLoader().AsBundle(path)
if err != nil {
outputErrors(err)
return 1
Expand All @@ -73,7 +73,7 @@ func checkModules(args []string) int {
Ignore: checkParams.ignore,
}

result, err := loader.Filtered(args, f.Apply)
result, err := loader.NewFileLoader().Filtered(args, f.Apply)
if err != nil {
outputErrors(err)
return 1
Expand Down
4 changes: 2 additions & 2 deletions cmd/deps.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ func deps(args []string, params depsCommandParams) error {
Ignore: params.ignore,
}

result, err := loader.Filtered(params.dataPaths.v, f.Apply)
result, err := loader.NewFileLoader().Filtered(params.dataPaths.v, f.Apply)
if err != nil {
return err
}
Expand All @@ -90,7 +90,7 @@ func deps(args []string, params depsCommandParams) error {

if len(params.bundlePaths.v) > 0 {
for _, path := range params.bundlePaths.v {
b, err := loader.AsBundle(path)
b, err := loader.NewFileLoader().AsBundle(path)
if err != nil {
return err
}
Expand Down
2 changes: 1 addition & 1 deletion internal/presentation/presentation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -297,7 +297,7 @@ func TestOutputJSONErrorStructuredLoaderErrList(t *testing.T) {
var tmpPath string
test.WithTempFS(files, func(path string) {
tmpPath = path
_, err = loader.All([]string{path})
_, err = loader.NewFileLoader().All([]string{path})
})

expected := fmt.Sprintf(`{
Expand Down
7 changes: 0 additions & 7 deletions loader/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,13 +38,6 @@ func (e *Errors) add(err error) {
}
}

func newResult() *Result {
return &Result{
Documents: map[string]interface{}{},
Modules: map[string]*RegoFile{},
}
}

type unsupportedDocumentType string

func (path unsupportedDocumentType) Error() string {
Expand Down
Loading

0 comments on commit 9c85dfc

Please sign in to comment.