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.

Sign up for GitHub

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

chore: add composite query validation for rule #6966

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

chore: add composite query validation for rule #6966

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
64 changes: 54 additions & 10 deletions pkg/query-service/model/v3/v3.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -549,11 +549,31 @@ func (c *CompositeQuery) Validate() error {
return fmt.Errorf("composite query is required")
}

if c.BuilderQueries == nil && c.ClickHouseQueries == nil && c.PromQueries == nil {
return fmt.Errorf("composite query must contain at least one query type")
if err := c.PanelType.Validate(); err != nil {
return fmt.Errorf("panel type is invalid: %w", err)
}

if err := c.QueryType.Validate(); err != nil {
return fmt.Errorf("query type is invalid: %w", err)
}

if c.QueryType == QueryTypeBuilder {
if c.BuilderQueries == nil {
return fmt.Errorf("at least one builder query must be provided")
}

// there should be at least one enabled query
enabled := false
for _, query := range c.BuilderQueries {
if !query.Disabled {
enabled = true
break
}
}
if !enabled {
return fmt.Errorf("at least one builder query must be enabled")
}

for name, query := range c.BuilderQueries {
if err := query.Validate(c.PanelType); err != nil {
return fmt.Errorf("builder query %s is invalid: %w", name, err)
Expand All @@ -562,6 +582,22 @@ func (c *CompositeQuery) Validate() error {
}

if c.QueryType == QueryTypeClickHouseSQL {
if c.ClickHouseQueries == nil {
return fmt.Errorf("at least one clickhouse query must be provided")
}

// there should be at least one enabled query
enabled := false
for _, query := range c.ClickHouseQueries {
if !query.Disabled {
enabled = true
break
}
}
if !enabled {
return fmt.Errorf("at least one clickhouse query must be enabled")
}

for name, query := range c.ClickHouseQueries {
if err := query.Validate(); err != nil {
return fmt.Errorf("clickhouse query %s is invalid: %w", name, err)
Expand All @@ -570,21 +606,29 @@ func (c *CompositeQuery) Validate() error {
}

if c.QueryType == QueryTypePromQL {
if c.PromQueries == nil {
return fmt.Errorf("at least one prom query must be provided")
}

// there should be at least one enabled query
enabled := false
for _, query := range c.PromQueries {
if !query.Disabled {
enabled = true
break
}
}
if !enabled {
return fmt.Errorf("at least one prom query must be enabled")
}

for name, query := range c.PromQueries {
if err := query.Validate(); err != nil {
return fmt.Errorf("prom query %s is invalid: %w", name, err)
}
}
}

if err := c.PanelType.Validate(); err != nil {
return fmt.Errorf("panel type is invalid: %w", err)
}

if err := c.QueryType.Validate(); err != nil {
return fmt.Errorf("query type is invalid: %w", err)
}

return nil
}

Expand Down
97 changes: 88 additions & 9 deletions pkg/query-service/rules/alerting.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,44 @@ const (
ValueOutsideBounds CompareOp = "7"
)

var (
supportedCompareOps = []CompareOp{CompareOpNone, ValueIsAbove, ValueIsBelow, ValueIsEq, ValueIsNotEq, ValueAboveOrEq, ValueBelowOrEq, ValueOutsideBounds}
)

func (co CompareOp) String() string {
switch co {
case CompareOpNone:
return "None: Enum value 0"
case ValueIsAbove:
return "ValueIsAbove: Enum value 1"
case ValueIsBelow:
return "ValueIsBelow: Enum value 2"
case ValueIsEq:
return "ValueIsEq: Enum value 3"
case ValueIsNotEq:
return "ValueIsNotEq: Enum value 4"
case ValueAboveOrEq:
return "ValueAboveOrEq: Enum value 5"
case ValueBelowOrEq:
return "ValueBelowOrEq: Enum value 6"
case ValueOutsideBounds:
return "ValueOutsideBounds: Enum value 7"
}
return "Unknown: Enum value"
}

func (co CompareOp) Validate() error {
var msg string
for _, op := range supportedCompareOps {
msg += op.String() + ", "
}
switch co {
case ValueIsAbove, ValueIsBelow, ValueIsEq, ValueIsNotEq, ValueAboveOrEq, ValueBelowOrEq, ValueOutsideBounds:
return nil
}
return fmt.Errorf("invalid compare op: %s supported ops are %s", co, msg)
}

type MatchType string

const (
Expand All @@ -105,6 +143,40 @@ const (
Last MatchType = "5"
)

var (
supportedMatchTypes = []MatchType{MatchTypeNone, AtleastOnce, AllTheTimes, OnAverage, InTotal, Last}
)

func (mt MatchType) String() string {
switch mt {
case MatchTypeNone:
return "None: Enum value 0"
case AtleastOnce:
return "AtleastOnce: Enum value 1"
case AllTheTimes:
return "AllTheTimes: Enum value 2"
case OnAverage:
return "OnAverage: Enum value 3"
case InTotal:
return "InTotal: Enum value 4"
case Last:
return "Last: Enum value 5"
}
return "Unknown: Enum value"
}

func (mt MatchType) Validate() error {
var msg string
for _, op := range supportedMatchTypes {
msg += op.String() + ", "
}
switch mt {
case MatchTypeNone, AtleastOnce, AllTheTimes, OnAverage, InTotal, Last:
return nil
}
return fmt.Errorf("invalid match type: %s supported ops are %s", mt, msg)
}

type RuleCondition struct {
CompositeQuery *v3.CompositeQuery `json:"compositeQuery,omitempty" yaml:"compositeQuery,omitempty"`
CompareOp CompareOp `yaml:"op,omitempty" json:"op,omitempty"`
Expand Down Expand Up @@ -161,27 +233,34 @@ func (rc *RuleCondition) GetSelectedQueryName() string {
return ""
}

func (rc *RuleCondition) IsValid() bool {
func (rc *RuleCondition) Validate() error {

if rc.CompositeQuery == nil {
return false
return ErrCompositeQueryRequired
}

if rc.QueryType() == v3.QueryTypeBuilder {
if rc.Target == nil {
return false
return ErrTargetRequired
}
if rc.CompareOp == "" {
return false
return ErrCompareOpRequired
}
if err := rc.CompareOp.Validate(); err != nil {
return err
}
if rc.MatchType == "" {
return ErrMatchTypeRequired
}
if err := rc.MatchType.Validate(); err != nil {
return err
}
}
if rc.QueryType() == v3.QueryTypePromQL {

if len(rc.CompositeQuery.PromQueries) == 0 {
return false
}
if err := rc.CompositeQuery.Validate(); err != nil {
return err
}
return true
return nil
}

// QueryType is a short hand method to get query type
Expand Down
83 changes: 16 additions & 67 deletions pkg/query-service/rules/api_params.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"go.signoz.io/signoz/pkg/query-service/model"
v3 "go.signoz.io/signoz/pkg/query-service/model/v3"
"go.uber.org/multierr"
"go.uber.org/zap"

"go.signoz.io/signoz/pkg/query-service/utils/times"
"go.signoz.io/signoz/pkg/query-service/utils/timestamp"
Expand All @@ -34,14 +35,16 @@ const (
)

var (
ErrFailedToParseJSON = errors.New("failed to parse json")
ErrFailedToParseYAML = errors.New("failed to parse yaml")
ErrInvalidDataType = errors.New("invalid data type")
ErrFailedToParseJSON = errors.New("failed to parse json")
ErrFailedToParseYAML = errors.New("failed to parse yaml")
ErrInvalidDataType = errors.New("invalid data type")
ErrRuleConditionRequired = errors.New("rule condition is required")
ErrCompositeQueryRequired = errors.New("composite query is required")
ErrCompareOpRequired = errors.New("compare op is required")
ErrTargetRequired = errors.New("target is required")
ErrMatchTypeRequired = errors.New("match type is required")
)

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

// PostableRule is used to create alerting rule from HTTP api
type PostableRule struct {
AlertName string `yaml:"alert,omitempty" json:"alert,omitempty"`
Expand All @@ -65,8 +68,8 @@ type PostableRule struct {
Version string `json:"version,omitempty"`

// legacy
Expr string `yaml:"expr,omitempty" json:"expr,omitempty"`
OldYaml string `json:"yaml,omitempty"`
// TODO(srikanthccv): remove this if there are no legacy rules
Expr string `yaml:"expr,omitempty" json:"expr,omitempty"`
}

func ParsePostableRule(content []byte) (*PostableRule, error) {
Expand Down Expand Up @@ -97,6 +100,8 @@ func parseIntoRule(initRule PostableRule, content []byte, kind RuleDataKind) (*P
}

if rule.RuleCondition == nil && rule.Expr != "" {
// there should be no legacy rules but just in case
zap.L().Info("legacy rule detected", zap.Any("rule", rule))
// account for legacy rules
rule.RuleType = RuleTypeProm
rule.EvalWindow = Duration(5 * time.Minute)
Expand Down Expand Up @@ -160,72 +165,16 @@ func isValidLabelValue(v string) bool {
return utf8.ValidString(v)
}

func isAllQueriesDisabled(compositeQuery *v3.CompositeQuery) bool {
if compositeQuery == nil {
return false
}
if compositeQuery.BuilderQueries == nil && compositeQuery.PromQueries == nil && compositeQuery.ClickHouseQueries == nil {
return false
}
switch compositeQuery.QueryType {
case v3.QueryTypeBuilder:
if len(compositeQuery.BuilderQueries) == 0 {
return false
}
for _, query := range compositeQuery.BuilderQueries {
if !query.Disabled {
return false
}
}
case v3.QueryTypePromQL:
if len(compositeQuery.PromQueries) == 0 {
return false
}
for _, query := range compositeQuery.PromQueries {
if !query.Disabled {
return false
}
}
case v3.QueryTypeClickHouseSQL:
if len(compositeQuery.ClickHouseQueries) == 0 {
return false
}
for _, query := range compositeQuery.ClickHouseQueries {
if !query.Disabled {
return false
}
}
}
return true
}

func (r *PostableRule) Validate() error {

var errs []error

if r.RuleCondition == nil {
// will get panic if we try to access CompositeQuery, so return here
return errors.Errorf("rule condition is required")
} else {
if r.RuleCondition.CompositeQuery == nil {
errs = append(errs, errors.Errorf("composite metric query is required"))
}
}

if isAllQueriesDisabled(r.RuleCondition.CompositeQuery) {
errs = append(errs, errors.Errorf("all queries are disabled in rule condition"))
return ErrRuleConditionRequired
}

if r.RuleType == RuleTypeThreshold {
if r.RuleCondition.Target == nil {
errs = append(errs, errors.Errorf("rule condition missing the threshold"))
}
if r.RuleCondition.CompareOp == "" {
errs = append(errs, errors.Errorf("rule condition missing the compare op"))
}
if r.RuleCondition.MatchType == "" {
errs = append(errs, errors.Errorf("rule condition missing the match option"))
}
if err := r.RuleCondition.Validate(); err != nil {
return err
}

for k, v := range r.Labels {
Expand Down
Loading
Loading