-
Notifications
You must be signed in to change notification settings - Fork 292
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
Elasticsearch sink refactor #1010
Elasticsearch sink refactor #1010
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please see the review of #1008. Thanks!
5982c67
to
80de28b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM but please apply comments before merge. Thanks!
pkg/sink/webhook.go
Outdated
@@ -50,7 +50,7 @@ func NewWebhook(log logrus.FieldLogger, c config.Webhook, reporter AnalyticsRepo | |||
return whNotifier, nil | |||
} | |||
|
|||
// SendMessage is no-op. | |||
// SendEvent is no-op. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
// SendEvent is no-op. | |
// SendEvent sends an event to a configured server. |
pkg/sink/elasticsearch.go
Outdated
@@ -196,8 +196,21 @@ func (e *Elasticsearch) SendMessage(ctx context.Context, event event.Event, even | |||
} | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is SendMessage
used anywhere? I don't think so, so please remove it. Thanks!
e.log.Debugf(">> Sending to Elasticsearch: %+v", rawData) | ||
|
||
errs := multierror.New() | ||
for _, indexCfg := range e.indices { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This logic is invalid - we send events to all indexes and sources
are not used. We should do
if !sliceutil.Intersect(indexCfg.Bindings.Sources, eventSources) {
continue
}
instead.
Basically please use logic from SendMessage
in SendEvent
, and remove SendMessage
. Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, I removed SendMessage now, and each es flush index is for one source, so I removed source part
Description
Changes proposed in this pull request:
api.Message
Testing
Build plugins
Run plugin server
Run Botkube with the following config
Create a pod to see a notification
Related issue(s)
Fixes #998