Skip to content

Commit

Permalink
chore: change returned error from list to error (#5805)
Browse files Browse the repository at this point in the history
  • Loading branch information
srikanthccv authored Aug 30, 2024
1 parent 6b09657 commit 16738ea
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 51 deletions.
35 changes: 22 additions & 13 deletions pkg/query-service/rules/api_params.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"github.com/pkg/errors"
"go.signoz.io/signoz/pkg/query-service/model"
v3 "go.signoz.io/signoz/pkg/query-service/model/v3"
"go.uber.org/multierr"

"go.signoz.io/signoz/pkg/query-service/utils/times"
"go.signoz.io/signoz/pkg/query-service/utils/timestamp"
Expand All @@ -32,6 +33,12 @@ const (
RuleDataKindYaml RuleDataKind = "yaml"
)

var (
ErrFailedToParseJSON = errors.New("failed to parse json")
ErrFailedToParseYAML = errors.New("failed to parse yaml")
ErrInvalidDataType = errors.New("invalid data type")
)

// this file contains api request and responses to be
// served over http

Expand Down Expand Up @@ -72,31 +79,31 @@ type PostableRule struct {
OldYaml string `json:"yaml,omitempty"`
}

func ParsePostableRule(content []byte) (*PostableRule, []error) {
func ParsePostableRule(content []byte) (*PostableRule, error) {
return parsePostableRule(content, "json")
}

func parsePostableRule(content []byte, kind string) (*PostableRule, []error) {
func parsePostableRule(content []byte, kind RuleDataKind) (*PostableRule, error) {
return parseIntoRule(PostableRule{}, content, kind)
}

// parseIntoRule loads the content (data) into PostableRule and also
// validates the end result
func parseIntoRule(initRule PostableRule, content []byte, kind string) (*PostableRule, []error) {
func parseIntoRule(initRule PostableRule, content []byte, kind RuleDataKind) (*PostableRule, error) {

rule := &initRule

var err error
if kind == "json" {
if kind == RuleDataKindJson {
if err = json.Unmarshal(content, rule); err != nil {
return nil, []error{fmt.Errorf("failed to load json")}
return nil, ErrFailedToParseJSON
}
} else if kind == "yaml" {
} else if kind == RuleDataKindYaml {
if err = yaml.Unmarshal(content, rule); err != nil {
return nil, []error{fmt.Errorf("failed to load yaml")}
return nil, ErrFailedToParseYAML
}
} else {
return nil, []error{fmt.Errorf("invalid data type")}
return nil, ErrInvalidDataType
}

if rule.RuleCondition == nil && rule.Expr != "" {
Expand Down Expand Up @@ -138,11 +145,11 @@ func parseIntoRule(initRule PostableRule, content []byte, kind string) (*Postabl
}
}

if errs := rule.Validate(); len(errs) > 0 {
return nil, errs
if err := rule.Validate(); err != nil {
return nil, err
}

return rule, []error{}
return rule, nil
}

func isValidLabelName(ln string) bool {
Expand All @@ -161,7 +168,9 @@ func isValidLabelValue(v string) bool {
return utf8.ValidString(v)
}

func (r *PostableRule) Validate() (errs []error) {
func (r *PostableRule) Validate() error {

var errs []error

if r.RuleCondition == nil {
errs = append(errs, errors.Errorf("rule condition is required"))
Expand Down Expand Up @@ -200,7 +209,7 @@ func (r *PostableRule) Validate() (errs []error) {
}

errs = append(errs, testTemplateParsing(r)...)
return errs
return multierr.Combine(errs...)
}

func testTemplateParsing(rl *PostableRule) (errs []error) {
Expand Down
56 changes: 18 additions & 38 deletions pkg/query-service/rules/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -160,32 +160,20 @@ func (m *Manager) initiate() error {

for _, rec := range storedRules {
taskName := fmt.Sprintf("%d-groupname", rec.Id)
parsedRule, errs := ParsePostableRule([]byte(rec.Data))
parsedRule, err := ParsePostableRule([]byte(rec.Data))

if len(errs) > 0 {
if errs[0].Error() == "failed to load json" {
if err != nil {
if errors.Is(err, ErrFailedToParseJSON) {
zap.L().Info("failed to load rule in json format, trying yaml now:", zap.String("name", taskName))

// see if rule is stored in yaml format
parsedRule, errs = parsePostableRule([]byte(rec.Data), "yaml")
parsedRule, err = parsePostableRule([]byte(rec.Data), RuleDataKindYaml)

if parsedRule == nil {
if err != nil {
zap.L().Error("failed to parse and initialize yaml rule", zap.String("name", taskName), zap.Error(err))
// just one rule is being parsed so expect just one error
loadErrors = append(loadErrors, errs[0])
loadErrors = append(loadErrors, err)
continue
} else {
// rule stored in yaml, so migrate it to json
zap.L().Info("migrating rule from JSON to yaml", zap.String("name", taskName))
ruleJSON, err := json.Marshal(parsedRule)
if err == nil {
taskName, _, err := m.ruleDB.EditRuleTx(context.Background(), string(ruleJSON), fmt.Sprintf("%d", rec.Id))
if err != nil {
zap.L().Error("failed to migrate rule", zap.String("name", taskName), zap.Error(err))
} else {
zap.L().Info("migrated rule from yaml to json", zap.String("name", taskName))
}
}
}
} else {
zap.L().Error("failed to parse and initialize rule", zap.String("name", taskName), zap.Error(err))
Expand Down Expand Up @@ -236,12 +224,10 @@ func (m *Manager) Stop() {
// datastore and also updates the rule executor
func (m *Manager) EditRule(ctx context.Context, ruleStr string, id string) error {

parsedRule, errs := ParsePostableRule([]byte(ruleStr))
parsedRule, err := ParsePostableRule([]byte(ruleStr))

if len(errs) > 0 {
zap.L().Error("failed to parse rules", zap.Errors("errors", errs))
// just one rule is being parsed so expect just one error
return errs[0]
if err != nil {
return err
}

taskName, _, err := m.ruleDB.EditRuleTx(ctx, ruleStr, id)
Expand Down Expand Up @@ -337,12 +323,10 @@ func (m *Manager) deleteTask(taskName string) {
// CreateRule stores rule def into db and also
// starts an executor for the rule
func (m *Manager) CreateRule(ctx context.Context, ruleStr string) (*GettableRule, error) {
parsedRule, errs := ParsePostableRule([]byte(ruleStr))
parsedRule, err := ParsePostableRule([]byte(ruleStr))

if len(errs) > 0 {
zap.L().Error("failed to parse rules", zap.Errors("errors", errs))
// just one rule is being parsed so expect just one error
return nil, errs[0]
if err != nil {
return nil, err
}

lastInsertId, tx, err := m.ruleDB.CreateRuleTx(ctx, ruleStr)
Expand Down Expand Up @@ -701,11 +685,9 @@ func (m *Manager) PatchRule(ctx context.Context, ruleStr string, ruleId string)
}

// patchedRule is combo of stored rule and patch received in the request
patchedRule, errs := parseIntoRule(storedRule, []byte(ruleStr), "json")
if len(errs) > 0 {
zap.L().Error("failed to parse rules", zap.Errors("errors", errs))
// just one rule is being parsed so expect just one error
return nil, errs[0]
patchedRule, err := parseIntoRule(storedRule, []byte(ruleStr), "json")
if err != nil {
return nil, err
}

// deploy or un-deploy task according to patched (new) rule state
Expand Down Expand Up @@ -753,11 +735,10 @@ func (m *Manager) PatchRule(ctx context.Context, ruleStr string, ruleId string)
// sends a test notification. returns alert count and error (if any)
func (m *Manager) TestNotification(ctx context.Context, ruleStr string) (int, *model.ApiError) {

parsedRule, errs := ParsePostableRule([]byte(ruleStr))
parsedRule, err := ParsePostableRule([]byte(ruleStr))

if len(errs) > 0 {
zap.L().Error("failed to parse rule from request", zap.Errors("errors", errs))
return 0, newApiErrorBadData(errs[0])
if err != nil {
return 0, newApiErrorBadData(err)
}

var alertname = parsedRule.AlertName
Expand All @@ -771,7 +752,6 @@ func (m *Manager) TestNotification(ctx context.Context, ruleStr string) (int, *m
parsedRule.AlertName = fmt.Sprintf("%s%s", alertname, TestAlertPostFix)

var rule Rule
var err error

if parsedRule.RuleType == RuleTypeThreshold {

Expand Down

0 comments on commit 16738ea

Please sign in to comment.