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

[Access] Fixed REST API metrics #375

Merged
merged 9 commits into from
Jun 1, 2023
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
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
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ require (
github.com/logrusorgru/aurora v2.0.3+incompatible
github.com/onflow/cadence v0.38.1
github.com/onflow/flow-archive v1.3.4-0.20230503192214-9e81e82d4dcc
github.com/onflow/flow-go v0.30.1-0.20230503175154-ebc4be2a52af
github.com/onflow/flow-go v0.30.1-0.20230526113926-956645d30ac8
github.com/onflow/flow-go-sdk v0.40.0
github.com/onflow/flow-go/crypto v0.24.7
github.com/onflow/flow-nft/lib/go/contracts v0.0.0-20220727161549-d59b1e547ac4
Expand Down
5 changes: 5 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -696,8 +696,13 @@ github.com/onflow/flow-core-contracts/lib/go/templates v1.2.3 h1:X25A1dNajNUtE+K
github.com/onflow/flow-core-contracts/lib/go/templates v1.2.3/go.mod h1:dqAUVWwg+NlOhsuBHex7bEWmsUjsiExzhe/+t4xNH6A=
github.com/onflow/flow-ft/lib/go/contracts v0.7.0 h1:XEKE6qJUw3luhsYmIOteXP53gtxNxrwTohgxJXCYqBE=
github.com/onflow/flow-ft/lib/go/contracts v0.7.0/go.mod h1:kTMFIySzEJJeupk+7EmXs0EJ6CBWY/MV9fv9iYQk+RU=
<<<<<<< HEAD
peterargue marked this conversation as resolved.
Show resolved Hide resolved
github.com/onflow/flow-go v0.30.1-0.20230517045958-378baae2f7b0 h1:jh1rdLCsGdo5sd9V5LC5TAm9qr0RRkjhfwvzdK47lFU=
github.com/onflow/flow-go v0.30.1-0.20230517045958-378baae2f7b0/go.mod h1:uEwS/eX5sMMkDy7cGxzF5IAeALmWSo/b40g14n9XRLQ=
=======
github.com/onflow/flow-go v0.30.1-0.20230503175154-ebc4be2a52af h1:A2zB5ptOkcir4X7FkSLMLZEgHZgs5vmQQVZCXfY+2bA=
github.com/onflow/flow-go v0.30.1-0.20230503175154-ebc4be2a52af/go.mod h1:uEwS/eX5sMMkDy7cGxzF5IAeALmWSo/b40g14n9XRLQ=
>>>>>>> origin/master
github.com/onflow/flow-go-sdk v0.24.0/go.mod h1:IoptMLPyFXWvyd9yYA6/4EmSeeozl6nJoIv4FaEMg74=
github.com/onflow/flow-go-sdk v0.40.0 h1:s8uwoyTquN8tjdXpqGmNkXTjf79yUII8JExc5QEl4Xw=
github.com/onflow/flow-go-sdk v0.40.0/go.mod h1:34dxXk9Hp/bQw6Zy6+H44Xo0kQU+aJyQoqdDxq00rJM=
Expand Down
6 changes: 4 additions & 2 deletions server/rest.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import (
"github.com/onflow/flow-emulator/server/backend"
"github.com/onflow/flow-go/engine/access/rest"
"github.com/onflow/flow-go/model/flow"
"github.com/onflow/flow-go/module/metrics"
"github.com/rs/zerolog"
)

Expand Down Expand Up @@ -71,13 +72,14 @@ func (r *RestServer) Stop() {
}

func NewRestServer(logger *zerolog.Logger, be *backend.Backend, chain flow.Chain, host string, port int, debug bool) (*RestServer, error) {

debugLogger := zerolog.Logger{}
if debug {
debugLogger = zerolog.New(os.Stdout)
}

srv, err := rest.NewServer(backend.NewAdapter(be), fmt.Sprintf("%s:3333", host), debugLogger, chain)
restCollector := metrics.NewNoopCollector()
Copy link
Contributor

@peterargue peterargue May 17, 2023

Choose a reason for hiding this comment

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

sounds like @bluesign would these turned on by default

Suggested change
restCollector := metrics.NewNoopCollector()
restCollector := metrics.NewRestCollector(metricsProm.Config{Prefix: "access_rest_api"})

also needs to import metricsProm

Copy link
Contributor

Choose a reason for hiding this comment

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

not necessarily by default, but support an option to enable it

Copy link
Contributor

@peterargue peterargue May 17, 2023

Choose a reason for hiding this comment

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

that's fair, but fwiw it is enabled by default for grpc

grpcprometheus.Register(grpcServer)

sounds like we need a followup issue to make metrics configurable for both

Copy link
Contributor

Choose a reason for hiding this comment

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

We can separate this in another PR yeah.


srv, err := rest.NewServer(backend.NewAdapter(be), fmt.Sprintf("%s:3333", host), debugLogger, chain, restCollector)
if err != nil {
return nil, err
}
Expand Down