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

Add metrics collection support to nftables set-only mode #292

Merged
merged 1 commit into from
May 31, 2023

Conversation

jarppiko
Copy link
Contributor

@jarppiko jarppiko commented May 29, 2023

Summary

This PR solves #291. It stops crowdsec-firewall-bouncer flooding crowdsec-firewall-bouncer.log by adding basic support for metrics collection to set-only mode.

As explained in #291 , it is not a perfect solution since it allows metrics collection only from one chain.

Description of changes

File changed: pkg/nftables/metrics.go

  1. Change collectDroppedPackets() to take chain name directly
  2. Move string concatenation (chain-hook) from it collectDroppedPackets() to collectDropped()
  3. Add if c.setOnly clause to collectDropped() to handle both the cases

Further considerations

Collecting metrics from all chains / hooks in set-only mode would require a way to tell the bouncer which chains are being used. This could be done with the following changes (not included in the PR)

  1. Change nftables_hooks config option to contain actual chain names and the remove the use of undocumented string concatenation to create nftables chain names from chain (basename) and hooks
  2. Use the same nftables_hooks config option in set-only mode to tell the bouncer from which chains to collect metrics from

Copy link
Contributor

@LaurenceJJones LaurenceJJones left a comment

Choose a reason for hiding this comment

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

lgtm tested

LaurenceJJones

This comment was marked as duplicate.

@mmetc mmetc merged commit 3d3cb8a into crowdsecurity:main May 31, 2023
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants