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

DEV-43744-alert-eval-api #1

Merged
merged 12 commits into from
Apr 3, 2024
4 changes: 3 additions & 1 deletion pkg/api/alerting.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (

"github.com/grafana/grafana/pkg/api/dtos"
"github.com/grafana/grafana/pkg/api/response"
"github.com/grafana/grafana/pkg/models" // LOGZ.IO GRAFANA CHANGE :: DEV-17927 - add LogzIoHeaders
Copy link
Collaborator

Choose a reason for hiding this comment

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

In the comment, I think better to use the reference to the new tickets. Also note in my PR I used a slightly different comment, so better be consistent (you can change mine if you wish).

Copy link
Author

Choose a reason for hiding this comment

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

done

"github.com/grafana/grafana/pkg/services/alerting"
alertmodels "github.com/grafana/grafana/pkg/services/alerting/models"
contextmodel "github.com/grafana/grafana/pkg/services/contexthandler/model"
Expand Down Expand Up @@ -179,7 +180,8 @@ func (hs *HTTPServer) AlertTest(c *contextmodel.ReqContext) response.Response {
return response.Error(400, "The dashboard needs to be saved at least once before you can test an alert rule", nil)
}

res, err := hs.AlertEngine.AlertTest(c.SignedInUser.GetOrgID(), dto.Dashboard, dto.PanelId, c.SignedInUser)
// LOGZ.IO GRAFANA CHANGE :: DEV-17927 - add LogzIoHeaders
res, err := hs.AlertEngine.AlertTest(c.SignedInUser.GetOrgID(), dto.Dashboard, dto.PanelId, c.SignedInUser, &models.LogzIoHeaders{RequestHeaders: c.Req.Header})
Copy link
Collaborator

Choose a reason for hiding this comment

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

why are LogzIoHeaders required for test alert flow?

Copy link
Author

Choose a reason for hiding this comment

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

it is technically part of the logzio datasources support, but because a lot of the implementation for the logz evaluation includes this i put it in this PR.

i don't know if at that point it's worth the time of taking out of this PR for the next , because the next PR anyway will be based on that and making these changes can add up a day of work i don't see any value for it 😅
in hindsight i would make the split if i knew about it from the beginning 😓

if err != nil {
var validationErr alerting.ValidationError
if errors.As(err, &validationErr) {
Expand Down
38 changes: 38 additions & 0 deletions pkg/models/headers.logzio.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
// LOGZ.IO GRAFANA CHANGE :: DEV-17927 use LogzIoHeaders obj to pass on headers
package models

import (
"net/http"
)

const LogzioHeadersCtxKey string = "logzioHeaders"
const LogzioRequestIdHeaderName string = "x-request-id"

type LogzIoHeaders struct {
RequestHeaders http.Header
}

var logzioHeadersWhitelist = []string{
"x-auth-token",
"x-api-token",
"user-context",
LogzioRequestIdHeaderName,
"cookie",
"x-logz-csrf-token",
"x-logz-csrf-token-v2",
}

func (logzioHeaders *LogzIoHeaders) GetDatasourceQueryHeaders(grafanaGeneratedHeaders http.Header) http.Header {
datasourceRequestHeaders := grafanaGeneratedHeaders.Clone()
logzioGrafanaRequestHeaders := logzioHeaders.RequestHeaders

for _, whitelistedHeader := range logzioHeadersWhitelist {
if requestHeader := logzioGrafanaRequestHeaders.Get(whitelistedHeader); requestHeader != "" {
datasourceRequestHeaders.Set(whitelistedHeader, requestHeader)
}
}

return datasourceRequestHeaders
}

// LOGZ.IO GRAFANA CHANGE :: end
8 changes: 8 additions & 0 deletions pkg/services/alerting/conditions/query.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (

"github.com/grafana/grafana/pkg/components/null"
"github.com/grafana/grafana/pkg/components/simplejson"
"github.com/grafana/grafana/pkg/infra/log" // LOGZ.IO GRAFANA CHANGE :: (ALERTS) DEV-16492 Support external alert evaluation
"github.com/grafana/grafana/pkg/services/alerting"
"github.com/grafana/grafana/pkg/services/datasources"
ngalertmodels "github.com/grafana/grafana/pkg/services/ngalert/models"
Expand All @@ -19,6 +20,13 @@ import (
"github.com/grafana/grafana/pkg/tsdb/prometheus"
)

// LOGZ.IO GRAFANA CHANGE :: (ALERTS) DEV-16492 Support external alert evaluation
var (
queryLog = log.New("alerting.conditions.query")
)

// LOGZ.IO GRAFANA CHANGE :: end

func init() {
alerting.RegisterCondition("query", func(model *simplejson.Json, index int) (alerting.Condition, error) {
return newQueryCondition(model, index)
Expand Down
4 changes: 2 additions & 2 deletions pkg/services/alerting/eval_context.go
Original file line number Diff line number Diff line change
Expand Up @@ -154,8 +154,8 @@ func (c *EvalContext) GetNewState() alertmodels.AlertStateType {
return ns
}

since := time.Since(c.Rule.LastStateChange)
if c.PrevAlertState == alertmodels.AlertStatePending && since > c.Rule.For {
since := c.StartTime.Sub(c.Rule.LastStateChange) // LOGZ.IO GRAFANA CHANGE :: DEV-17927 - change time to StartTime
if c.PrevAlertState == alertmodels.AlertStatePending && since >= c.Rule.For { // LOGZ.IO GRAFANA CHANGE :: DEV-18900 - fix For check to be inclusive
return alertmodels.AlertStateAlerting
}

Expand Down
2 changes: 2 additions & 0 deletions pkg/services/alerting/rule.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"time"

"github.com/grafana/grafana/pkg/components/simplejson"
pkgmodels "github.com/grafana/grafana/pkg/models" // LOGZ.IO GRAFANA CHANGE :: DEV-17927 - add LogzIoHeaders
"github.com/grafana/grafana/pkg/services/alerting/models"
"github.com/grafana/grafana/pkg/services/tag"
)
Expand Down Expand Up @@ -55,6 +56,7 @@ type Rule struct {
Conditions []Condition
Notifications []string
AlertRuleTags []*tag.Tag
LogzIoHeaders *pkgmodels.LogzIoHeaders // LOGZ.IO GRAFANA CHANGE :: DEV-17927 - add LogzIoHeaders

StateChanges int64
}
Expand Down
5 changes: 4 additions & 1 deletion pkg/services/alerting/test_rule.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,14 @@ import (
"fmt"

"github.com/grafana/grafana/pkg/components/simplejson"
"github.com/grafana/grafana/pkg/models" // LOGZ.IO GRAFANA CHANGE :: DEV-17927 - add LogzIoHeaders
"github.com/grafana/grafana/pkg/services/annotations/annotationstest"
"github.com/grafana/grafana/pkg/services/dashboards"
"github.com/grafana/grafana/pkg/services/user"
)

// AlertTest makes a test alert.
func (e *AlertEngine) AlertTest(orgID int64, dashboard *simplejson.Json, panelID int64, user *user.SignedInUser) (*EvalContext, error) {
func (e *AlertEngine) AlertTest(orgID int64, dashboard *simplejson.Json, panelID int64, user *user.SignedInUser, LogzIoHeaders *models.LogzIoHeaders) (*EvalContext, error) { // LOGZ.IO GRAFANA CHANGE :: DEV-17927 - add LogzIoHeaders
dash := dashboards.NewDashboardFromJson(dashboard)
dashInfo := DashAlertInfo{
User: user,
Expand All @@ -32,6 +33,8 @@ func (e *AlertEngine) AlertTest(orgID int64, dashboard *simplejson.Json, panelID
return nil, err
}

rule.LogzIoHeaders = LogzIoHeaders // LOGZ.IO GRAFANA CHANGE :: DEV-17927 - add LogzIoHeaders

handler := NewEvalHandler(e.DataService)

context := NewEvalContext(context.Background(), rule, fakeRequestValidator{}, e.AlertStore, nil, e.datasourceService, annotationstest.NewFakeAnnotationsRepo())
Expand Down
13 changes: 13 additions & 0 deletions pkg/services/ngalert/api/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package api

import (
"context"
"github.com/benbjohnson/clock" // LOGZ.IO GRAFANA CHANGE :: DEV-30169,DEV-30170,DEV-30275: add logzio alerting endpoints
"net/url"
"time"

Expand Down Expand Up @@ -152,6 +153,18 @@ func (api *API) RegisterAPIEndpoints(m *metrics.API) {
hist: api.Historian,
}), m)

// LOGZ.IO GRAFANA CHANGE :: DEV-30169,DEV-30170,DEV-30275: add logzio alerting endpoints
api.RegisterLogzioAlertingApiEndpoints(NewLogzioAlertingApi(
NewLogzioAlertingService(proxy,
api.Cfg,
api.EvaluatorFactory,
clock.New(),
api.MultiOrgAlertmanager,
logger,
),
), m)
// LOGZ.IO GRAFANA CHANGE :: end

// Inject upgrade endpoints if legacy alerting is enabled and the feature flag is enabled.
if !api.Cfg.UnifiedAlerting.IsEnabled() && api.FeatureManager.IsEnabledGlobally(featuremgmt.FlagAlertingPreviewUpgrade) {
api.RegisterUpgradeApiEndpoints(NewUpgradeApi(NewUpgradeSrc(
Expand Down
80 changes: 80 additions & 0 deletions pkg/services/ngalert/api/api_logzio_alerting.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
package api

// LOGZ.IO GRAFANA CHANGE :: DEV-30169,DEV-30170: add endpoints to evaluate and process alerts
import (
"github.com/grafana/grafana/pkg/api/response"
"github.com/grafana/grafana/pkg/api/routing"
contextmodel "github.com/grafana/grafana/pkg/services/contexthandler/model"
apimodels "github.com/grafana/grafana/pkg/services/ngalert/api/tooling/definitions"
"github.com/grafana/grafana/pkg/services/ngalert/metrics"
"github.com/grafana/grafana/pkg/web"
"net/http"
)

type LogzioAlertingApi struct {
service *LogzioAlertingService
}

type RunAlertMigrationForOrg struct {
OrgId int64 `json:"orgId"`
EmailNotifications []AlertMigrationEmailNotification `json:"emailNotifications"`
}

type ClearOrgAlertMigration struct {
OrgId int64 `json:"orgId"`
}

type AlertMigrationEmailNotification struct {
EmailAddress string `json:"address"`
ChannelUid string `json:"channelUid"`
}

// NewLogzioAlertingApi creates a new LogzioAlertingApi instance
func NewLogzioAlertingApi(service *LogzioAlertingService) *LogzioAlertingApi {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Didn't we say its redundant? (leave only the service)

Copy link
Author

Choose a reason for hiding this comment

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

this is the structure of all APIs in this project. there is a class of "XXXApi" that has the route to the endpoint, and the function to call the service class that contains the main logic.
i think what we said is that maybe the LogzioAlertingService can be redundant and we can put the logic directly in this class. but actually i think this structure has some sense in it.. also i'm pretty sure we'll want it also for later when more logic/apis will be added (or at least it gives the infra for it).

return &LogzioAlertingApi{
service: service,
}
}

func (api *LogzioAlertingApi) RouteEvaluateAlert(ctx *contextmodel.ReqContext) response.Response {
body := apimodels.AlertEvaluationRequest{}
if err := web.Bind(ctx.Req, &body); err != nil {
return response.Error(http.StatusBadRequest, "bad request data", err)
}

return api.service.RouteEvaluateAlert(ctx, body)
}

//func (api *LogzioAlertingApi) RouteProcessAlert(ctx *contextmodel.ReqContext) response.Response {
// body := apimodels.AlertProcessRequest{}
// if err := web.Bind(ctx.Req, &body); err != nil {
// return response.Error(http.StatusBadRequest, "bad request data", err)
// }
//
// return api.service.RouteProcessAlert(*ctx.Req, body)
//}

func (api *API) RegisterLogzioAlertingApiEndpoints(srv *LogzioAlertingApi, m *metrics.API) {
api.RouteRegister.Group("", func(group routing.RouteRegister) {
group.Post(
toMacaronPath("/internal/alert/api/v1/eval"),
metrics.Instrument(
http.MethodPost,
"/internal/alert/api/v1/eval",
srv.RouteEvaluateAlert,
m,
),
)
//group.Post(
// toMacaronPath("/internal/alert/api/v1/process"),
// metrics.Instrument(
// http.MethodPost,
// "/internal/alert/api/v1/process",
// srv.RouteProcessAlert,
// m,
// ),
//)
})
}

// LOGZ.IO GRAFANA CHANGE :: end
Loading