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 event limit for filter event definition #16035

Merged
merged 18 commits into from Aug 18, 2023
Merged

add event limit for filter event definition #16035

merged 18 commits into from Aug 18, 2023

Conversation

AntonEbel
Copy link
Contributor

@AntonEbel AntonEbel commented Jul 25, 2023

This PR introduces an event limit for the "Filter and Aggregation" event definition. However, only if the user selects "Filter has results".

How this feature behaves:

  • Existing event definitions are not migrated and the event limit 0 is shown in the UI.
  • When the event limit is reached, the remaining messages are discarded and a notification is shown that the limit has been reached.
  • the validation of the event limit works as follows:
    • the event limit must be greater than 0 (except for existing event definitions).
    • if for an existing event definition the event limit has been set, there is no way back and the user must set a value greater than 0 on the next update.
    • The event limit must be smaller than event_definition_max_event_limit

Description

Motivation and Context

Graylog2/graylog-plugin-enterprise#5382

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Refactoring (non-breaking change)
  • Breaking change (fix or feature that would cause existing functionality to change)

# Conflicts:
#	graylog2-server/src/main/java/org/graylog/events/processor/aggregation/AggregationEventProcessor.java
#	graylog2-server/src/main/java/org/graylog/events/processor/aggregation/AggregationEventProcessorConfig.java
@AntonEbel AntonEbel self-assigned this Jul 27, 2023
@AntonEbel AntonEbel marked this pull request as ready for review August 1, 2023 12:19
@AntonEbel AntonEbel requested a review from a team August 1, 2023 16:13
@moesterheld moesterheld self-requested a review August 2, 2023 08:57
@mpfz0r mpfz0r self-assigned this Aug 16, 2023
Copy link
Member

@mpfz0r mpfz0r left a comment

Choose a reason for hiding this comment

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

a few comments, but it's almost over the finish line :)

Copy link
Contributor

@moesterheld moesterheld left a comment

Choose a reason for hiding this comment

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

Approving, as my comments have been addressed.

@AntonEbel AntonEbel requested a review from mpfz0r August 18, 2023 07:52
Copy link
Member

@mpfz0r mpfz0r left a comment

Choose a reason for hiding this comment

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

Works and LGTM 👍

@mpfz0r mpfz0r merged commit 73ac961 into master Aug 18, 2023
5 checks passed
@mpfz0r mpfz0r deleted the event-limit branch August 18, 2023 09:07
mpfz0r added a commit that referenced this pull request Aug 24, 2023
With #16035 we try to enforce a new default event limit,
without breaking backwards compatibility.

The validations are making an exemption if an existing
event definition is being edited which doesn't have an event limit yet
(limit of 0)

Duplicating event definitions was done in the frontend, and then
creating a new one.
This makes it impossible to differentiate between copy and create in the
backend.

 - Create a new `/events/definitions/{definitionId}/duplicate` endpoint.
   Which creates a copy without performing a validation.

 - Also correctly set the state for unscheduled event defintions.
   This got missed with #15558

Refs #16035
mpfz0r added a commit that referenced this pull request Aug 30, 2023
* Allow duplicating event definitions with a limit of 0

With #16035 we try to enforce a new default event limit,
without breaking backwards compatibility.

The validations are making an exemption if an existing
event definition is being edited which doesn't have an event limit yet
(limit of 0)

Duplicating event definitions was done in the frontend, and then
creating a new one.
This makes it impossible to differentiate between copy and create in the
backend.

 - Create a new `/events/definitions/{definitionId}/duplicate` endpoint.
   Which creates a copy without performing a validation.

 - Also correctly set the state for unscheduled event defintions.
   This got missed with #15558

Refs #16035

* Add changelog

* Move code to EventDefinitionHandler and add test
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants