Skip to content
Open
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
109 changes: 109 additions & 0 deletions internal/managementrouter/alerts_get.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
package managementrouter

import (
"context"
"encoding/json"
"net/http"

"github.com/openshift/monitoring-plugin/pkg/k8s"
)

type GetAlertsResponse struct {
Data GetAlertsResponseData `json:"data"`
Warnings []string `json:"warnings,omitempty"`
}

type GetAlertsResponseData struct {
Alerts []k8s.PrometheusAlert `json:"alerts"`
}

func (hr *httpRouter) GetAlerts(w http.ResponseWriter, req *http.Request) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

With the layers of abstractions it can be difficult to follow the actual functions being used in this PR due to many functions sharing the same name. Since there are 3 separate GetAlerts functions you can't grep/search for the name and the abstaction through interfaces means you can't use LSP's actions to "go to definition" or "find all references". I know it makes the individual functions not as clear to their exact purpose but could we swap to different function names so that we can grep/search through the codebase and only see the exact function being searched for. Same for some other the functions like alertinghealth

state, labels, err := parseStateAndLabels(req.URL.Query())
if err != nil {
writeError(w, http.StatusBadRequest, err.Error())
return
}
ctx := req.Context()

alerts, err := hr.managementClient.GetAlerts(ctx, k8s.GetAlertsRequest{
Labels: labels,
State: state,
})
if err != nil {
handleError(w, err)
return
}

w.Header().Set("Content-Type", "application/json")
w.Header().Set("Cache-Control", "no-store")
w.WriteHeader(http.StatusOK)
if err := json.NewEncoder(w).Encode(GetAlertsResponse{
Data: GetAlertsResponseData{
Alerts: alerts,
},
Warnings: hr.alertWarnings(ctx),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

At it's root, alertWarnings is used to fetch the /health endpoint of platform and user workload prometheus's, then surface error in connecting to the /health endpoint to users.

The purpose behind this is that within hr.managementClient.GetAlerts we don't want to immediately return an error if one of the endpoints fails to return correctly, so instead we log.warn and proceed forward with processing any return from the other.

I think this approach is overcomplicated and actually doesn't surface the users issues. Requests could be denied for reasons other than the /health endpoint being down which the users should know about. Really what we want is failures of each endpoint within GetAlerts and to send them back as warnings. So we should just create a slice of warnings in GetAlerts and return them back, then surface those warnings to the user (maybe after some formatting if need be)

}); err != nil {
log.WithError(err).Warn("failed to encode alerts response")
}
}

func (hr *httpRouter) alertWarnings(ctx context.Context) []string {
health, ok := hr.alertingHealth(ctx)
if !ok {
return nil
}

warnings := []string{}
if health.UserWorkloadEnabled && health.UserWorkload != nil {
warnings = append(warnings, buildRouteWarnings(health.UserWorkload.Prometheus, k8s.UserWorkloadRouteName, "user workload Prometheus")...)
warnings = append(warnings, buildRouteWarnings(health.UserWorkload.Alertmanager, k8s.UserWorkloadAlertmanagerRouteName, "user workload Alertmanager")...)
}

return warnings
}

//nolint:unused // used by the rules listing handler in a subsequent branch
func (hr *httpRouter) rulesWarnings(ctx context.Context) []string {
health, ok := hr.alertingHealth(ctx)
if !ok {
return nil
}

if health.UserWorkloadEnabled && health.UserWorkload != nil {
return buildRouteWarnings(health.UserWorkload.Prometheus, k8s.UserWorkloadRouteName, "user workload Prometheus")
}

return nil
}

func (hr *httpRouter) alertingHealth(ctx context.Context) (k8s.AlertingHealth, bool) {
if hr.managementClient == nil {
return k8s.AlertingHealth{}, false
}

health, err := hr.managementClient.GetAlertingHealth(ctx)
if err != nil {
log.WithError(err).Warn("alerting health unavailable")
return k8s.AlertingHealth{}, false
}

return health, true
}

func buildRouteWarnings(route k8s.AlertingRouteHealth, expectedName string, friendlyName string) []string {
if route.Name != "" && route.Name != expectedName {
return nil
}
if route.FallbackReachable {
return nil
}

switch route.Status {
case k8s.RouteNotFound:
return []string{friendlyName + " route is missing"}
case k8s.RouteUnreachable:
return []string{friendlyName + " route is unreachable"}
default:
return nil
}
}
Loading