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

[whitelist] metrics improvement #2517

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
bb72fe9
Improve whitelist parsing
LaurenceJJones Aug 10, 2023
54e4589
Merge branch 'master' into s02_whitelist_improvement
LaurenceJJones Aug 22, 2023
cc6adc0
Split whitelist check into a function tied to whitelist, also since w…
LaurenceJJones Aug 22, 2023
4c1af35
No point passing clog as an argument since it is just a pointer to no…
LaurenceJJones Aug 22, 2023
4e3c17b
We should break instead of returning false, false as it may have been…
LaurenceJJones Aug 22, 2023
a797407
reimplement early return if expr errors
LaurenceJJones Aug 22, 2023
7278081
Fix lint and dont need to parse ip back to string just loop over sources
LaurenceJJones Aug 22, 2023
96158ea
Log error with node logger as it provides context
LaurenceJJones Aug 22, 2023
9018275
Move getsource to a function cleanup some code
LaurenceJJones Aug 22, 2023
9def62c
Change func name
LaurenceJJones Aug 22, 2023
ac091a5
Split out compile to a function so we can use in tests. Add a bunch o…
LaurenceJJones Aug 23, 2023
8c1e509
spell correction
LaurenceJJones Aug 23, 2023
571fe94
Use node logger so it has context
LaurenceJJones Aug 23, 2023
89432c9
alternative solution
buixor Aug 23, 2023
8323091
quick fixes
buixor Aug 23, 2023
1904e30
Merge pull request #1 from crowdsecurity/alt_s02_whitelist_improvement
LaurenceJJones Aug 23, 2023
1b389c2
Merge branch 'master' into s02_whitelist_improvement
LaurenceJJones Aug 23, 2023
e1d7926
Use containswls
LaurenceJJones Aug 23, 2023
89c752e
Change whitelist test to use parseipsource and only events
LaurenceJJones Aug 23, 2023
a01b1b8
Merge branch 'master' into s02_whitelist_improvement
LaurenceJJones Sep 19, 2023
a4ba41b
Make it simpler
LaurenceJJones Sep 19, 2023
a9d8f91
Postoverflow tests, some basic ones to make sure it works
LaurenceJJones Sep 19, 2023
f7455c0
Use official pkg
LaurenceJJones Sep 19, 2023
4e6f3e5
Merge branch 'master' into s02_whitelist_improvement
LaurenceJJones Sep 20, 2023
b2f6347
Merge branch 'master' into s02_whitelist_improvement
LaurenceJJones Oct 2, 2023
9ecd231
Merge branch 'master' into s02_whitelist_improvement
LaurenceJJones Oct 3, 2023
fa6b829
Split whitelist metrics into a seperate prometheus collectors, first …
LaurenceJJones Oct 4, 2023
df642ab
Merge branch 'master' into whitelist_nmetrics
LaurenceJJones Oct 4, 2023
22b00a6
Merge branch 'master' into whitelist_nmetrics
LaurenceJJones Oct 16, 2023
7d04f82
fix merge
LaurenceJJones Oct 17, 2023
5d56527
Merge branch 'master' into whitelist_nmetrics
LaurenceJJones Oct 17, 2023
94de6de
Merge branch 'master' into whitelist_nmetrics
LaurenceJJones Oct 26, 2023
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
19 changes: 16 additions & 3 deletions cmd/crowdsec-cli/metrics.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@
}{}
acquis_stats := map[string]map[string]int{}
parsers_stats := map[string]map[string]int{}
whitelist_stats := map[string]map[string]int{}
buckets_stats := map[string]map[string]int{}
lapi_stats := map[string]map[string]int{}
lapi_machine_stats := map[string]map[string]map[string]int{}
Expand Down Expand Up @@ -173,6 +174,18 @@
parsers_stats[name] = make(map[string]int)
}
parsers_stats[name]["unparsed"] += ival
case "cs_node_hits_wl_ok_total":
subName := fmt.Sprintf("%s:[%s]", name, metric.Labels["expression"])
if _, ok := whitelist_stats[subName]; !ok {
whitelist_stats[subName] = make(map[string]int)
}
whitelist_stats[subName]["wl"] += ival
case "cs_node_hits_wl_ko_total":
subName := fmt.Sprintf("%s:[%s]", name, metric.Labels["expression"])
if _, ok := whitelist_stats[subName]; !ok {
whitelist_stats[subName] = make(map[string]int)
}
whitelist_stats[subName]["nowl"] += ival

Check warning on line 188 in cmd/crowdsec-cli/metrics.go

View check run for this annotation

Codecov / codecov/patch

cmd/crowdsec-cli/metrics.go#L177-L188

Added lines #L177 - L188 were not covered by tests
case "cs_lapi_route_requests_total":
if _, ok := lapi_stats[route]; !ok {
lapi_stats[route] = make(map[string]int)
Expand Down Expand Up @@ -237,6 +250,7 @@
acquisStatsTable(out, acquis_stats)
bucketStatsTable(out, buckets_stats)
parserStatsTable(out, parsers_stats)
whitelistStatsTable(out, whitelist_stats)
lapiStatsTable(out, lapi_stats)
lapiMachineStatsTable(out, lapi_machine_stats)
lapiBouncerStatsTable(out, lapi_bouncer_stats)
Expand All @@ -252,6 +266,7 @@
stats["acquisition"] = acquis_stats
stats["buckets"] = buckets_stats
stats["parsers"] = parsers_stats
stats["whitelist"] = whitelist_stats

Check warning on line 269 in cmd/crowdsec-cli/metrics.go

View check run for this annotation

Codecov / codecov/patch

cmd/crowdsec-cli/metrics.go#L269

Added line #L269 was not covered by tests
stats["lapi"] = lapi_stats
stats["lapi_machine"] = lapi_machine_stats
stats["lapi_bouncer"] = lapi_bouncer_stats
Expand Down Expand Up @@ -282,7 +297,6 @@

var noUnit bool


func runMetrics(cmd *cobra.Command, args []string) error {
if err := csConfig.LoadPrometheus(); err != nil {
return fmt.Errorf("failed to load prometheus config: %w", err)
Expand Down Expand Up @@ -311,15 +325,14 @@
return nil
}


func NewMetricsCmd() *cobra.Command {
cmdMetrics := &cobra.Command{
Use: "metrics",
Short: "Display crowdsec prometheus metrics.",
Long: `Fetch metrics from the prometheus server and display them in a human-friendly way`,
Args: cobra.ExactArgs(0),
DisableAutoGenTag: true,
RunE: runMetrics,
RunE: runMetrics,
}
cmdMetrics.PersistentFlags().StringVarP(&prometheusURL, "url", "u", "", "Prometheus url (http://<ip>:<port>/metrics)")
cmdMetrics.PersistentFlags().BoolVar(&noUnit, "no-unit", false, "Show the real number instead of formatted with units")
Expand Down
14 changes: 14 additions & 0 deletions cmd/crowdsec-cli/metrics_table.go
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,20 @@
}
}

func whitelistStatsTable(out io.Writer, stats map[string]map[string]int) {
t := newTable(out)
t.SetRowLines(false)
t.SetHeaders("Whitelist", "WL", "NoWL")
t.SetAlignment(table.AlignLeft, table.AlignLeft, table.AlignLeft, table.AlignLeft)
keys := []string{"wl", "nowl"}
if numRows, err := metricsToTable(t, stats, keys); err != nil {
log.Warningf("while collecting metrics stats: %s", err)

Check warning on line 139 in cmd/crowdsec-cli/metrics_table.go

View check run for this annotation

Codecov / codecov/patch

cmd/crowdsec-cli/metrics_table.go#L139

Added line #L139 was not covered by tests
} else if numRows > 0 {
renderTableTitle(out, "\nWhitelist Metrics:")
t.Render()
}

Check warning on line 143 in cmd/crowdsec-cli/metrics_table.go

View check run for this annotation

Codecov / codecov/patch

cmd/crowdsec-cli/metrics_table.go#L141-L143

Added lines #L141 - L143 were not covered by tests
}

func stashStatsTable(out io.Writer, stats map[string]struct {
Type string
Count int
Expand Down
2 changes: 1 addition & 1 deletion cmd/crowdsec/metrics.go
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ func registerPrometheus(config *csconfig.PrometheusCfg) {
} else {
log.Infof("Loading prometheus collectors")
prometheus.MustRegister(globalParserHits, globalParserHitsOk, globalParserHitsKo,
parser.NodesHits, parser.NodesHitsOk, parser.NodesHitsKo,
parser.NodesHits, parser.NodesHitsOk, parser.NodesHitsKo, parser.NodesHitsWl, parser.NodesHitsWlOk, parser.NodesHitsWlKo,
globalCsInfo, globalParsingHistogram, globalPourHistogram,
v1.LapiRouteHits, v1.LapiMachineHits, v1.LapiBouncerHits, v1.LapiNilDecisions, v1.LapiNonNilDecisions, v1.LapiResponseTime,
leaky.BucketsPour, leaky.BucketsUnderflow, leaky.BucketsCanceled, leaky.BucketsInstantiation, leaky.BucketsOverflow, leaky.BucketsCurrentCount,
Expand Down
26 changes: 11 additions & 15 deletions pkg/parser/node.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import (
"github.com/antonmedv/expr"
"github.com/antonmedv/expr/vm"
"github.com/davecgh/go-spew/spew"
"github.com/prometheus/client_golang/prometheus"
log "github.com/sirupsen/logrus"
yaml "gopkg.in/yaml.v2"

Expand Down Expand Up @@ -168,22 +167,23 @@ func (n *Node) process(p *types.Event, ctx UnixParserCtx, expressionEnv map[stri
NodeState = true
}

if n.Name != "" {
NodesHits.With(prometheus.Labels{"source": p.Line.Src, "type": p.Line.Module, "name": n.Name}).Inc()
}
n.IncPromHits(p)
exprErr := error(nil)
isWhitelisted := n.CheckIPsWL(p.ParseIPSources())
isWhitelisted := n.CheckIPsWL(p)
if !isWhitelisted {
isWhitelisted, exprErr = n.CheckExprWL(cachedExprEnv)
isWhitelisted, exprErr = n.CheckExprWL(cachedExprEnv, p)
}
if exprErr != nil {
// Previous code returned nil if there was an error, so we keep this behavior
return false, nil //nolint:nilerr
}

if isWhitelisted && !p.Whitelisted {
p.Whitelisted = true
p.WhitelistReason = n.Whitelist.Reason

if isWhitelisted {
if !p.Whitelisted {
p.Whitelisted = true
p.WhitelistReason = n.Whitelist.Reason
}
/*huglily wipe the ban order if the event is whitelisted and it's an overflow */
if p.Type == types.OVFLW { /*don't do this at home kids */
ips := []string{}
Expand Down Expand Up @@ -331,16 +331,12 @@ func (n *Node) process(p *types.Event, ctx UnixParserCtx, expressionEnv map[stri

//grok or leafs failed, don't process statics
if !NodeState {
if n.Name != "" {
NodesHitsKo.With(prometheus.Labels{"source": p.Line.Src, "type": p.Line.Module, "name": n.Name}).Inc()
}
n.IncKoNodeHits(p)
clog.Debugf("Event leaving node : ko")
return NodeState, nil
}

if n.Name != "" {
NodesHitsOk.With(prometheus.Labels{"source": p.Line.Src, "type": p.Line.Module, "name": n.Name}).Inc()
}
n.IncOkNodeHits(p)

/*
This is to apply statics when the node either was whitelisted, or is not a whitelist (it has no expr/ips wl)
Expand Down
96 changes: 96 additions & 0 deletions pkg/parser/node_metrics.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
package parser

import (
"github.com/crowdsecurity/crowdsec/pkg/types"
"github.com/prometheus/client_golang/prometheus"
)

var NodesHits = prometheus.NewCounterVec(
prometheus.CounterOpts{
Name: "cs_node_hits_total",
Help: "Total events entered node.",
},
[]string{"source", "type", "name"},
)

var NodesHitsOk = prometheus.NewCounterVec(
prometheus.CounterOpts{
Name: "cs_node_hits_ok_total",
Help: "Total events successfully exited node.",
},
[]string{"source", "type", "name"},
)

var NodesHitsKo = prometheus.NewCounterVec(
prometheus.CounterOpts{
Name: "cs_node_hits_ko_total",
Help: "Total events unsuccessfully exited node.",
},
[]string{"source", "type", "name"},
)

var NodesHitsWl = prometheus.NewCounterVec(
prometheus.CounterOpts{
Name: "cs_node_hits_wl_total",
Help: "Total events entered whitelists.",
},
[]string{"name", "source", "type"},
)

var NodesHitsWlOk = prometheus.NewCounterVec(
prometheus.CounterOpts{
Name: "cs_node_hits_wl_ok_total",
Help: "Total events successfully exited node.",
},
[]string{"name", "expression", "source", "type"},
)

var NodesHitsWlKo = prometheus.NewCounterVec(
prometheus.CounterOpts{
Name: "cs_node_hits_wl_ko_total",
Help: "Total events unsuccessfully exited node.",
},
[]string{"name", "expression", "source", "type"},
)

// Increase the number of hits for a node excluding whitelists
func (n *Node) IncPromHits(p *types.Event) {
if n.Name == "" {
return
}

Check warning on line 60 in pkg/parser/node_metrics.go

View check run for this annotation

Codecov / codecov/patch

pkg/parser/node_metrics.go#L59-L60

Added lines #L59 - L60 were not covered by tests
if !n.ContainsWLs() {
NodesHits.With(prometheus.Labels{"source": p.Line.Src, "type": p.Line.Module, "name": n.Name}).Inc()
}
}

// Increase the number of ok hits for a node excluding whitelists
func (n *Node) IncOkNodeHits(p *types.Event) {
if n.Name == "" || n.ContainsWLs() {
return
}
NodesHitsOk.With(prometheus.Labels{"source": p.Line.Src, "type": p.Line.Module, "name": n.Name}).Inc()
}

// Increase the number of ko hits for a node excluding whitelists
func (n *Node) IncKoNodeHits(p *types.Event) {
if n.Name == "" || n.ContainsWLs() {
return
}

Check warning on line 78 in pkg/parser/node_metrics.go

View check run for this annotation

Codecov / codecov/patch

pkg/parser/node_metrics.go#L77-L78

Added lines #L77 - L78 were not covered by tests
NodesHitsKo.With(prometheus.Labels{"source": p.Line.Src, "type": p.Line.Module, "name": n.Name}).Inc()
}

// Increase the number of ok hits for a whitelist node
func (n *Node) IncOkWLHits(p *types.Event, expr string) {
if n.Name == "" || !n.ContainsWLs() {
return
}
NodesHitsWlOk.With(prometheus.Labels{"source": p.Line.Src, "type": p.Line.Module, "name": n.Name, "expression": expr}).Inc()
}

// Increase the number of ko hits for a whitelist node
func (n *Node) IncKoWLHits(p *types.Event, expr string) {
if n.Name == "" || !n.ContainsWLs() {
return
}
NodesHitsWlKo.With(prometheus.Labels{"source": p.Line.Src, "type": p.Line.Module, "name": n.Name, "expression": expr}).Inc()
}
25 changes: 0 additions & 25 deletions pkg/parser/runtime.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ import (

"github.com/antonmedv/expr"
"github.com/mohae/deepcopy"
"github.com/prometheus/client_golang/prometheus"
log "github.com/sirupsen/logrus"

"github.com/crowdsecurity/crowdsec/pkg/types"
Expand Down Expand Up @@ -194,30 +193,6 @@ func (n *Node) ProcessStatics(statics []ExtraField, event *types.Event) error {
return nil
}

var NodesHits = prometheus.NewCounterVec(
prometheus.CounterOpts{
Name: "cs_node_hits_total",
Help: "Total events entered node.",
},
[]string{"source", "type", "name"},
)

var NodesHitsOk = prometheus.NewCounterVec(
prometheus.CounterOpts{
Name: "cs_node_hits_ok_total",
Help: "Total events successfully exited node.",
},
[]string{"source", "type", "name"},
)

var NodesHitsKo = prometheus.NewCounterVec(
prometheus.CounterOpts{
Name: "cs_node_hits_ko_total",
Help: "Total events unsuccessfully exited node.",
},
[]string{"source", "type", "name"},
)

func stageidx(stage string, stages []string) int {
for i, v := range stages {
if stage == v {
Expand Down
16 changes: 12 additions & 4 deletions pkg/parser/whitelist.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,36 +38,40 @@ func (n *Node) ContainsIPLists() bool {
return len(n.Whitelist.B_Ips) > 0 || len(n.Whitelist.B_Cidrs) > 0
}

func (n *Node) CheckIPsWL(srcs []net.IP) bool {
func (n *Node) CheckIPsWL(p *types.Event) bool {
isWhitelisted := false
if !n.ContainsIPLists() {
return isWhitelisted
}
for _, src := range srcs {
for _, src := range p.ParseIPSources() {
if isWhitelisted {
break
}
for _, v := range n.Whitelist.B_Ips {
if v.Equal(src) {
n.Logger.Debugf("Event from [%s] is whitelisted by IP (%s), reason [%s]", src, v, n.Whitelist.Reason)
isWhitelisted = true
n.IncOkWLHits(p, v.String())
break
}
n.IncKoWLHits(p, v.String())
n.Logger.Tracef("whitelist: %s is not eq [%s]", src, v)
}
for _, v := range n.Whitelist.B_Cidrs {
if v.Contains(src) {
n.Logger.Debugf("Event from [%s] is whitelisted by CIDR (%s), reason [%s]", src, v, n.Whitelist.Reason)
isWhitelisted = true
n.IncOkWLHits(p, v.String())
break
}
n.IncKoWLHits(p, v.String())
n.Logger.Tracef("whitelist: %s not in [%s]", src, v)
}
}
return isWhitelisted
}

func (n *Node) CheckExprWL(cachedExprEnv map[string]interface{}) (bool, error) {
func (n *Node) CheckExprWL(cachedExprEnv map[string]interface{}, p *types.Event) (bool, error) {
isWhitelisted := false

if !n.ContainsExprLists() {
Expand All @@ -79,7 +83,8 @@ func (n *Node) CheckExprWL(cachedExprEnv map[string]interface{}) (bool, error) {
if isWhitelisted {
break
}
output, err := expr.Run(e.Filter, cachedExprEnv)

output, err := expr.Run(e.Filter, cachedExprEnv)
if err != nil {
n.Logger.Warningf("failed to run whitelist expr : %v", err)
n.Logger.Debug("Event leaving node : ko")
Expand All @@ -93,6 +98,9 @@ func (n *Node) CheckExprWL(cachedExprEnv map[string]interface{}) (bool, error) {
if out {
n.Logger.Debugf("Event is whitelisted by expr, reason [%s]", n.Whitelist.Reason)
isWhitelisted = true
n.IncOkWLHits(p, fmt.Sprintf("%d", eidx))
} else {
n.IncKoWLHits(p, fmt.Sprintf("%d", eidx))
}
default:
n.Logger.Errorf("unexpected type %t (%v) while running '%s'", output, output, n.Whitelist.Exprs[eidx])
Expand Down
4 changes: 2 additions & 2 deletions pkg/parser/whitelist_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -289,9 +289,9 @@ func TestWhitelistCheck(t *testing.T) {
var err error
node.Whitelist = tt.whitelist
node.CompileWLs()
isWhitelisted := node.CheckIPsWL(tt.event.ParseIPSources())
isWhitelisted := node.CheckIPsWL(tt.event)
if !isWhitelisted {
isWhitelisted, err = node.CheckExprWL(map[string]interface{}{"evt": tt.event})
isWhitelisted, err = node.CheckExprWL(map[string]interface{}{"evt": tt.event}, tt.event)
}
require.NoError(t, err)
require.Equal(t, tt.expected, isWhitelisted)
Expand Down
Loading