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

Doc: Working with Logstash Plugins on ECK #7702

Merged
merged 18 commits into from
Apr 25, 2024
Merged

Conversation

karenzone
Copy link
Contributor

@karenzone karenzone commented Apr 8, 2024

@botelastic botelastic bot added the triage label Apr 8, 2024
@thbkrkr thbkrkr added >docs Documentation :logstash labels Apr 8, 2024
@botelastic botelastic bot removed the triage label Apr 8, 2024
@karenzone karenzone changed the title 3023 ls plugins Doc: Working with Logstash Plugins on ECK Apr 8, 2024
@karenzone karenzone marked this pull request as draft April 8, 2024 16:14
Copy link
Member

@robbavey robbavey left a comment

Choose a reason for hiding this comment

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

This is great stuff! I've added some thoughts and comments

Copy link
Contributor

@kaisecheng kaisecheng left a comment

Choose a reason for hiding this comment

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

The docs look mature, really awesome.

When your pipeline uses `logstash-integration-logstash`, add `keepalive=>false` to the {logstash-ref}/plugins-outputs-logstash.html[logstash-output] definition to ensure that load balancing works correctly rather than keeping affinity to the same pod.

{logstash-ref}/plugins-filters-elastic_integration.html[Elastic_integration filter plugin]::
The elastic_integration filter allows the use of `ElasticsearchRef` and environment variables.
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion: add a link to environment variables to point user how to use it. https://www.elastic.co/guide/en/cloud-on-k8s/master/k8s-logstash-configuration.html#k8s-logstash-pipelines-es

Copy link
Contributor Author

Choose a reason for hiding this comment

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

To make this work, I added a new heading called "elasticsearchRefs for establishing a secured connection". I'm linking to the new section from the elastic_integration filter section.

@karenzone note to self: Restructure to make this flow better in the next iteration

Examples of these plugins include {logstash-ref}/plugins-inputs-kafka.html[`logstash-input-kafka`], {logstash-ref}/plugins-inputs-azure_event_hubs.html[`logstash-input-azure_event_hubs`], and {logstash-ref}/plugins-inputs-kinesis.html[`logstash-input-kinesis`].

[id="{p}-logstash-working-with-plugin-considerations"]
=== Plugin-specific considerations
Copy link
Contributor

Choose a reason for hiding this comment

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

Would you also consider adding elasticsearch-output plugin in this session? I know it is duplicated to https://www.elastic.co/guide/en/cloud-on-k8s/master/k8s-logstash-configuration.html#k8s-logstash-pipelines-es
As I see elastic_integration-filter has a touch on setting role and permission, it seems the same category for es-ouput to add role and permission setup.

"cluster": ["monitor", "manage_ilm", "read_ilm", "manage_logstash_pipelines", "manage_index_templates", "cluster:admin/ingest/pipeline/get",],
  "indices": [
    {
      "names": [ "logstash", "logstash-*", "ecs-logstash", "ecs-logstash-*", "logs-*", "metrics-*", "synthetics-*", "traces-*" ],
      "privileges": ["manage", "write", "create_index", "read", "view_index_metadata"]
    }

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Please see if I implemented this as you pictured it.

Copy link
Contributor

Choose a reason for hiding this comment

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

btw, the source code in the original note is missing ] at the end and having an extra , after "cluster:admin/ingest/pipeline/get" which will cause syntax error :)

This is the correct yaml

"cluster": ["monitor", "manage_ilm", "read_ilm", "manage_logstash_pipelines", "manage_index_templates", "cluster:admin/ingest/pipeline/get"],
"indices": [
  {
    "names": [ "logstash", "logstash-*", "ecs-logstash", "ecs-logstash-*", "logs-*", "metrics-*", "synthetics-*", "traces-*" ],
    "privileges": ["manage", "write", "create_index", "read", "view_index_metadata"]
  }
]

karenzone and others added 2 commits April 15, 2024 15:01
Co-authored-by: kaisecheng <69120390+kaisecheng@users.noreply.github.com>
Co-authored-by: kaisecheng <69120390+kaisecheng@users.noreply.github.com>
@karenzone
Copy link
Contributor Author

@robbavey, please LMKWYT and if we can move to "Ready for Review" status.

Copy link
Member

@robbavey robbavey left a comment

Choose a reason for hiding this comment

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

I think we're really close!

Co-authored-by: Rob Bavey <rob.bavey@elastic.co>
@karenzone karenzone marked this pull request as ready for review April 18, 2024 23:56
@karenzone karenzone requested a review from robbavey April 18, 2024 23:56
@karenzone
Copy link
Contributor Author

I'll resync base branch again when we're done making changes.

Co-authored-by: Rob Bavey <rob.bavey@elastic.co>
Copy link
Member

@robbavey robbavey left a comment

Choose a reason for hiding this comment

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

LGTM

@karenzone
Copy link
Contributor Author

karenzone commented Apr 23, 2024

@robbavey, thank you for your work and approval on this.
As for next steps... Should I request someone from the ECK team to review? And if so, who should that be?

@robbavey
Copy link
Member

@karenzone Yes, we should get a review from the ECK team:

@pebrc @thbkrkr @barkbay This documentation PR, to give more information on how to work with plugins to Logstash on ECK users, is ready for review.

Copy link
Collaborator

@pebrc pebrc left a comment

Choose a reason for hiding this comment

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

LGTM

karenzone and others added 2 commits April 24, 2024 18:24
Co-authored-by: Peter Brachwitz <peter.brachwitz@gmail.com>
@pebrc pebrc merged commit e9681fe into elastic:main Apr 25, 2024
5 checks passed
@karenzone karenzone deleted the 3023-ls-plugins branch April 25, 2024 12:26
@karenzone
Copy link
Contributor Author

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants