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

Elasticsearch sink refactor #1010

Merged
merged 7 commits into from
Mar 13, 2023

Conversation

huseyinbabal
Copy link
Contributor

@huseyinbabal huseyinbabal commented Mar 7, 2023

Description

Changes proposed in this pull request:

  • Elasticsearch sink is updated to accept new contract api.Message
  • Introduced rawData param to send original data to sinks
  • Collector is updated to enable plugins for also sinks

Testing

Build plugins

PLUGIN_DOWNLOAD_URL_BASE_PATH=http://localhost:8080/plugin-dist make gen-plugins-index

Run plugin server

npx serve --listen 8080

Run Botkube with the following config

export BOTKUBE_CONFIG_PATHS=/tmp/botkube.yml
export BOTKUBE_SETTINGS_KUBECONFIG=${USER}/.kube/config

go run cmd/botkube/main.go
## /tmp/botkube.yml
communications:
  'default-group':
    elasticsearch:
      enabled: true
      server: 'elastic_url'
      username: user
      password: pass
      indices:
        'default':
          name: botkube
          type: kubernetes
          shards: 1
          replicas: 1
          bindings:
            sources: 
              - k8s-events
              - k8s-updates


settings:
  clusterName: gke-playground
  configWatcher: true
  log:
    level: debug
  lifecycleServer:
    deployment:
      name: botkube
      namespace: botkube
    port: "2113"

configWatcher:
  enabled: false
  initialSyncTimeout: 0

sources:
  'k8s-events':
    displayName: "K8s recommendations"
    'botkube/kubernetes':
      enabled: true
      config:
        log:
          level: debug
        recommendations:
          pod:
            noLatestImageTag: true
            labelsSet: true
          ingress:
            backendServiceValid: false
            tlsSecretValid: false
        namespaces:
          include:
            - botkube
        event:
          types:
            - create
            - update

  'k8s-updates':
    displayName: "K8s ConfigMaps updates"
    'botkube/kubernetes':
      enabled: true
      config:
        log:
          level: debug
        namespaces:
          include:
            - default
        event:
          types:
            - create
            - update
            - delete
        resources:
          - type: v1/configmaps
            namespaces:
              include:
                - botkube
            event: # overrides top level `event` entry
              types:
                - update
            updateSetting:
              includeDiff: true
              fields:
                - data

plugins:
  repositories:
    botkube:
      url: http://localhost:8080/plugins-index.yaml

Create a pod to see a notification

kubectl run nginx_es --image=nginx:latest -n botkube

Related issue(s)

Fixes #998

@huseyinbabal huseyinbabal requested a review from a team March 7, 2023 12:08
@huseyinbabal huseyinbabal requested a review from PrasadG193 as a code owner March 7, 2023 12:08
Copy link
Collaborator

@pkosiec pkosiec left a 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!

pkg/sink/elasticsearch.go Show resolved Hide resolved
@pkosiec pkosiec removed the request for review from josefkarasek March 9, 2023 10:17
@pkosiec pkosiec self-assigned this Mar 9, 2023
@huseyinbabal huseyinbabal force-pushed the elasticsearch-sink-refactor branch from 5982c67 to 80de28b Compare March 13, 2023 12:34
Copy link
Collaborator

@pkosiec pkosiec left a 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!

@@ -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.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
// SendEvent is no-op.
// SendEvent sends an event to a configured server.

@@ -196,8 +196,21 @@ func (e *Elasticsearch) SendMessage(ctx context.Context, event event.Event, even
}

Copy link
Collaborator

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 {
Copy link
Collaborator

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!

Copy link
Contributor Author

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

@huseyinbabal huseyinbabal merged commit 5a8f26d into kubeshop:main Mar 13, 2023
@huseyinbabal huseyinbabal deleted the elasticsearch-sink-refactor branch March 13, 2023 20:04
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.

Refactor Elasticsearch sink
2 participants