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

Prometheus encoder added similar to hostd #1068

Closed
wants to merge 7 commits into from
Closed
Show file tree
Hide file tree
Changes from all 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
33 changes: 31 additions & 2 deletions autopilot/autopilot.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
"go.sia.tech/renterd/api"
"go.sia.tech/renterd/build"
"go.sia.tech/renterd/hostdb"
"go.sia.tech/renterd/internal/prometheus"
"go.sia.tech/renterd/object"
"go.sia.tech/renterd/wallet"
"go.sia.tech/renterd/webhooks"
Expand Down Expand Up @@ -175,6 +176,34 @@
})
}

func (ap *Autopilot) writeResponse(c jape.Context, code int, resp any) {
Copy link
Member

Choose a reason for hiding this comment

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

Can you move the content of this to a function in a new file api/utils.go like so

func WriteResponse(c jape.Context, code int, resp any) error

and then call that function in here, check its error, and log it? Right now there is a lot of duplicate code.

Copy link
Member

Choose a reason for hiding this comment

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

code is unused by the way

Copy link
Contributor Author

Choose a reason for hiding this comment

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

hey I think @n8maninger wrote this method not sure what it does I just use it

var responseFormat string
if err := c.DecodeForm("response", &responseFormat); err != nil {
return
}

if resp != nil {
switch responseFormat {
case "prometheus":
v, ok := resp.(prometheus.Marshaller)
if !ok {
err := fmt.Errorf("response does not implement prometheus.Marshaller %T", resp)
c.Error(err, http.StatusInternalServerError)
ap.logger.Error("response does not implement prometheus.Marshaller", zap.Stack("stack"), zap.Error(err))
return
}

enc := prometheus.NewEncoder(c.ResponseWriter)
if err := enc.Append(v); err != nil {
ap.logger.Error("failed to marshal prometheus response", zap.Error(err))
return
Copy link
Member

Choose a reason for hiding this comment

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

You return here without calling c.Error.

}
default:
c.Encode(resp)
}
}
}

func (ap *Autopilot) configHandlerPOST(jc jape.Context) {
ctx := jc.Request.Context()

Expand Down Expand Up @@ -691,7 +720,7 @@
jc.Encode(host)
}

func (ap *Autopilot) stateHandlerGET(jc jape.Context) {

Check failure on line 723 in autopilot/autopilot.go

View workflow job for this annotation

GitHub Actions / test (ubuntu-latest, 1.21)

GET routes should write a response object

Check failure on line 723 in autopilot/autopilot.go

View workflow job for this annotation

GitHub Actions / test (ubuntu-latest, 1.21)

Analyzer warning in autopilot

GET routes should write a response object
pruning, pLastStart := ap.c.Status()
migrating, mLastStart := ap.m.Status()
scanning, sLastStart := ap.s.Status()
Expand All @@ -701,7 +730,7 @@
return
}

jc.Encode(api.AutopilotStateResponse{
ap.writeResponse(jc, http.StatusOK, AutopilotStateResp(api.AutopilotStateResponse{
Copy link
Member

Choose a reason for hiding this comment

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

This won't work with our static code analysis tool jape. It expects jc.Encode to be called in the handler right now. So without updating that this won't make it through the CI.

Copy link
Member

Choose a reason for hiding this comment

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

We should seriously consider extending jape to support this or maybe try and write some kind of middleware for it. It won't work otherwise. Maybe we can make jape.Context an interface and then we can swap it (using a middleware) with one that's capable of handling prometheus response types. E.g. test the interface in Encode on some kind of interface that implements toPrometheus and handle it accordingly

Configured: err == nil,
Migrating: migrating,
MigratingLastStart: api.TimeRFC3339(mLastStart),
Expand All @@ -719,7 +748,7 @@
OS: runtime.GOOS,
BuildTime: api.TimeRFC3339(build.BuildTime()),
},
})
}))
}

func (ap *Autopilot) hostsHandlerPOST(jc jape.Context) {
Expand Down
66 changes: 66 additions & 0 deletions autopilot/prometheus.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
package autopilot

import (
"go.sia.tech/renterd/api"
"go.sia.tech/renterd/internal/prometheus"
)

type AutopilotStateResp api.AutopilotStateResponse

func (asr AutopilotStateResp) PrometheusMetric() (metrics []prometheus.Metric) {
Copy link
Member

Choose a reason for hiding this comment

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

I'd implement PrometheusMetric or maybe ToPrometheus on the actual return type in the api package within a single api/prometheus.go. That way we don't need 3 of these prometheus.go files with duplicated types.

If a return type doesn't exist yet because it's directly returning a types.Currency or something it's fine to create a new one in the api package.

Copy link
Member

Choose a reason for hiding this comment

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

+1

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@n8maninger wrote this I can'ti would run it by him before I make any changes to it.

labels := map[string]any{
"network": asr.Network,
"version": asr.Version,
"commit": asr.Commit,
"os": asr.OS,
"buildTime": asr.BuildTime.String(),
}
// labels := fmt.Sprintf(`network="%s", version="%s", commit="%s", os="%s", buildTime="%s"`,
// build.NetworkName(), build.Version(), build.Commit(), runtime.GOOS, api.TimeRFC3339(build.BuildTime()))
metrics = append(metrics, prometheus.Metric{
Name: "renterd_autopilot_state_uptime",
Labels: labels,
Value: float64(asr.UptimeMS),
})
metrics = append(metrics, prometheus.Metric{
Name: "renterd_autopilot_state_configured",
Labels: labels,
Value: func() float64 {
if asr.Configured {
return 1
}
return 0
}(),
})
metrics = append(metrics, prometheus.Metric{
Name: "renterd_autopilot_state_migrating",
Labels: labels,
Value: func() float64 {
if asr.Migrating {
return 1
}
return 0
}(),
})
metrics = append(metrics, prometheus.Metric{
Name: "renterd_autopilot_state_pruning",
Labels: labels,
Value: func() float64 {
if asr.Pruning {
return 1
}
return 0
}(),
})
metrics = append(metrics, prometheus.Metric{
Name: "renterd_autopilot_state_scanning",
Labels: labels,
Value: func() float64 {
if asr.Scanning {
return 1
}
return 0
}(),
})
return
}
Loading
Loading