testutils/promrated: add prom query to collect pubkey to cluster information #1562

LukeHackett12 · 2022-12-14T19:00:02Z

Adding in prometheus query to get all the necessary cluster information and associated private keys.

category: misc
ticket: #1540

codecov · 2022-12-14T19:08:28Z

Codecov Report

Base: 54.28% // Head: 54.10% // Decreases project coverage by -0.18% ⚠️

Coverage data is based on head (71aa332) compared to base (b3fe553).
Patch coverage: 38.66% of modified lines in pull request are covered.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1562      +/-   ##
==========================================
- Coverage   54.28%   54.10%   -0.19%     
==========================================
  Files         153      156       +3     
  Lines       19436    19579     +143     
==========================================
+ Hits        10551    10593      +42     
- Misses       7455     7549      +94     
- Partials     1430     1437       +7

Impacted Files	Coverage Δ
testutil/promrated/promrated.go	`0.00% <0.00%> (ø)`
testutil/promrated/prometheus.go	`42.02% <53.70%> (ø)`
cmd/bootnode/p2p.go	`38.06% <0.00%> (-3.85%)`	⬇️
app/vmock.go	`73.05% <0.00%> (-1.04%)`	⬇️
cmd/bootnode/bootnode.go	`56.31% <0.00%> (-0.43%)`	⬇️
cmd/bootnode/metrics.go	`33.33% <0.00%> (ø)`
core/qbft/qbft.go	`83.82% <0.00%> (+0.42%)`	⬆️
core/priority/prioritiser.go	`61.69% <0.00%> (+2.82%)`	⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

corverroos · 2022-12-15T08:12:50Z

testutil/promrated/prometheus.go

 )

+const (
+	promEndpoint = "https://vm.monitoring.gcp.obol.tech"
+	promQuery    = "group%20by%20%28cluster_name%2C%20ccluster_hash%2C%20cluster_peer%2C%20cluster_network%2C%20pubkey_full%29%20%28core_scheduler_validator_balance_gwei%29"


write normal text, http libraries should handle escaping

corverroos · 2022-12-15T08:13:17Z

testutil/promrated/prometheus.go

 )

+const (
+	promEndpoint = "https://vm.monitoring.gcp.obol.tech"


best avoid globals, make this a flag with a default value

corverroos · 2022-12-15T08:13:48Z

testutil/promrated/prometheus.go

+	promQuery    = "group%20by%20%28cluster_name%2C%20ccluster_hash%2C%20cluster_peer%2C%20cluster_network%2C%20pubkey_full%29%20%28core_scheduler_validator_balance_gwei%29"
+)
+
+type ClusterInfo struct {


always unexport names by default.

corverroos · 2022-12-15T08:15:07Z

testutil/promrated/prometheus.go

+		return nil, err
+	}
+
+	if res.StatusCode == 200 {


avoid nesting https://dave.cheney.net/practical-go/presentations/gophercon-israel.html#_return_early_rather_than_nesting_deeply

corverroos · 2022-12-15T08:17:46Z

testutil/promrated/prometheus.go

+
+func getPromClusters(ctx context.Context, promAuth string) (*http.Response, error) {
+	client := new(http.Client)
+	url := fmt.Sprintf("%s/query?query=%s", promEndpoint, promQuery)


use the url package to build the url (avoid unstructured untyped logic)

corverroos · 2022-12-15T08:18:58Z

testutil/promrated/prometheus.go

+
+	req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil)
+	if err != nil {
+		log.Error(ctx, "HTTP Request malformed for prom query", err)


don't log and return errors, rather just return errors and log at the root (the loop or processor that is doing the scheduling).

corverroos · 2022-12-15T08:25:27Z

testutil/promrated/prometheus.go

+	return res, nil
+}
+
+func readPromJSON(ctx context.Context, res *http.Response) (*PromResponse, error) {


this is two function calls and pretty common go code, can inline. Also you need to close the request body.

corverroos · 2022-12-15T08:26:17Z

testutil/promrated/prometheus.go

+	return nil, errors.New("error processing prom metrics", z.Str("url", res.Request.RequestURI))
+}
+
+func getPromClusters(ctx context.Context, promAuth string) (*http.Response, error) {


returning a http response isn't a good abstraction, rather colocate http wrangling stuff and return proper business logic types

corverroos · 2022-12-15T08:28:30Z

testutil/promrated/prometheus.go

@@ -41,6 +81,73 @@ func serveMonitoring(addr string) error {
 	return errors.Wrap(server.ListenAndServe(), "failed to serve prometheus metrics")
 }

+func getPubkeyToClusterInfo(ctx context.Context, promAuth string) (map[string]prometheus.Labels, error) {


getPubkeyToClusterInfo name can improve, what about getClusters or getClustersFromProm

also add godoc please

rather return a proper type instead of map of strings. Also remember that clusters contain multiple validators. Suggest:

type Validator struct { PubkeyHex string ClusterName string ClusterHash string ClusterPeer string }

getPubkeyToClusterInfo name can improve, what about getClusters or getClustersFromProm

Yeah this naming is what I was used to for map types, can change it

corverroos · 2022-12-15T08:30:36Z

testutil/promrated/prometheus.go

+	Metric PromMetric `json:"metric"`
+}
+
+type PromData struct {


no need for all these types, you can just define a single nested type since it is only used for unmarshalling. Also, only include fields that you are actually using.

type promResponse struct { Data struct { Result []struct{ ... } `json:result` } `json:"data"` }

corverroos · 2022-12-15T08:31:13Z

testutil/promrated/prometheus.go

+}
+
+type PromMetric struct {
+	*ClusterInfo


avoid pointers

Add prom query for cluster

58bb9f6

LukeHackett12 added 2 commits December 14, 2022 20:17

wrap errors and handle http

38b06f9

fix port and linting

ba9117e

corverroos reviewed Dec 15, 2022

View reviewed changes

corverroos suggested changes Dec 15, 2022

View reviewed changes

LukeHackett12 and others added 3 commits December 15, 2022 14:24

refactor and address PR comments

30a6126

fix linting

c5adcbe

cleanup

ea4da30

corverroos approved these changes Dec 15, 2022

View reviewed changes

consistent naming

71aa332

LukeHackett12 added the merge when ready Indicates bulldozer bot may merge when all checks pass label Dec 15, 2022

obol-bulldozer bot merged commit 532b2be into main Dec 15, 2022

obol-bulldozer bot deleted the luke/prom-metrics branch December 15, 2022 14:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

testutils/promrated: add prom query to collect pubkey to cluster information #1562

testutils/promrated: add prom query to collect pubkey to cluster information #1562

LukeHackett12 commented Dec 14, 2022

codecov bot commented Dec 14, 2022 •

edited

Loading

corverroos Dec 15, 2022

corverroos Dec 15, 2022

corverroos Dec 15, 2022

corverroos Dec 15, 2022

corverroos Dec 15, 2022

corverroos Dec 15, 2022

corverroos Dec 15, 2022

corverroos Dec 15, 2022

corverroos Dec 15, 2022

corverroos Dec 15, 2022

corverroos Dec 15, 2022

LukeHackett12 Dec 15, 2022

corverroos Dec 15, 2022

corverroos Dec 15, 2022

testutils/promrated: add prom query to collect pubkey to cluster information #1562

testutils/promrated: add prom query to collect pubkey to cluster information #1562

Conversation

LukeHackett12 commented Dec 14, 2022

codecov bot commented Dec 14, 2022 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Dec 14, 2022 •

edited

Loading