Add developer guide #18

VijayanB · 2021-06-23T00:15:39Z

Signed-off-by: Vijayan Balasubramanian balasvij@amazon.com

Description

Update gem spec and DEVELOPER_GUIDE.

Issues Resolved

Check List

Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

VijayanB · 2021-06-23T00:29:15Z

Link to see this change in better readable format: https://github.com/VijayanB/logstash-output-opensearch/blob/guide/DEVELOPER_GUIDE.md

vamshin · 2021-06-23T05:16:34Z

DEVELOPER_GUIDE.md

+
+## Getting Started
+
+### Git Clone logstash-output-opensearch Repo


How about just call clone repo?

I would state it differently in the heading - if someone wants to contribute they should fork.

So, maybe "### Fork the logstash-output-opensearch Repo"

vamshin · 2021-06-23T05:17:27Z

DEVELOPER_GUIDE.md

+
+#### JRuby
+
+This plugin builds using JRuby. This means you'll need JRuby with the Bundler gem installed.


Do these commands work for all the OS?

This should. I use rvm everywhere.

Link to JRuby.org the first time it's mentioned so folks get some context.

dblock · 2021-06-23T14:39:02Z

DEVELOPER_GUIDE.md

+
+#### JRuby
+
+This plugin builds using JRuby. This means you'll need JRuby with the Bundler gem installed.


This should. I use rvm everywhere.

dblock · 2021-06-23T14:39:15Z

DEVELOPER_GUIDE.md

+rvm install jruby
+
+#Installing bundler
+jruby -S gem install bundler 


I think this should be rvm use jruby, then gem install bundler.

dblock · 2021-06-23T14:40:15Z

DEVELOPER_GUIDE.md

+
+```bash
+# Set up Environment variable for docker to pull your test environment
+export LOGSTASH_VERSION=7.13.2 # mandatory 


Can this be defaulted. I ran into logstash-plugins/logstash-output-elasticsearch#1019 which seems more annoying than anything else.

dblock · 2021-06-23T14:40:57Z

DEVELOPER_GUIDE.md

+To perform unit tests, set `INTEGRATION=false` and re-run the `docker-run.sh`.
+You don't have to set up docker again.
+
+2. Tests against OpenDistro clusters.


Do we need a "test against Elasticsearch OSS"?

There is no difference between opendistro and Elasticsearch OSS. If you still feel we need dedicated test, it can be added easily too.

stockholmux

Looks good - a few minor things.

stockholmux · 2021-06-23T15:35:58Z

DEVELOPER_GUIDE.md

+
+## Getting Started
+
+### Git Clone logstash-output-opensearch Repo


I would state it differently in the heading - if someone wants to contribute they should fork.

So, maybe "### Fork the logstash-output-opensearch Repo"

stockholmux · 2021-06-23T15:37:18Z

DEVELOPER_GUIDE.md

+
+### Git Clone logstash-output-opensearch Repo
+
+Fork [opensearch-project/logstash-output-opensearch](https://github.com/opensearch-project/logstash-output-opensearch) and clone locally, e.g. `git clone https://github.com/[your username]/logstash-output-opensearch.git`.


I think block style code is more clear and easier to identify

stockholmux · 2021-06-23T15:38:02Z

DEVELOPER_GUIDE.md

+
+#### JRuby
+
+This plugin builds using JRuby. This means you'll need JRuby with the Bundler gem installed.


Link to JRuby.org the first time it's mentioned so folks get some context.

stockholmux · 2021-06-23T15:50:48Z

DEVELOPER_GUIDE.md

+   bin/logstash -e 'output {opensearch {}}'
+   ```
+
+At this point any modifications to the plugin code will be applied to this local Logstash setup. After modifying the plugin, simply re-run Logstash.


https://jameshfisher.com/2017/02/22/dont-use-simply/

stockholmux · 2021-06-23T15:51:47Z

DEVELOPER_GUIDE.md

+    Version: 1.0.0
+    File: logstash-output-opensearch-1.0.0.gem
+    ```
+   The s.version number from your gemspec file will provide the gem version, in this case, 1.0.0.


s.version

stockholmux · 2021-06-23T15:53:58Z

DEVELOPER_GUIDE.md

+
+### Authorization
+
+Authorization to a secure OpenSearch cluster requires read permission at index level.


point to the documentation on index level permissions.

stockholmux · 2021-06-23T15:54:32Z

logstash-output-opensearch.gemspec

@@ -2,7 +2,7 @@ Gem::Specification.new do |s|
  s.name            = 'logstash-output-opensearch'
  s.version         = '1.0.0'

-  s.licenses        = ['Apache License (2.0)']
+  s.licenses        = ['Apache-2.0']
  s.summary         = "Stores logs in OpenSearch"
  s.description     = "This gem is a Logstash plugin required to be installed on top of the Logstash core pipeline using $LS_HOME/bin/logstash-plugin install gem. This gem is not a stand-alone program"
  s.authors         = ["OpenSearch"]


Should this be "OpenSearch Community"?

@dblock @vamshin what do you think?

I really don't know what it should be. It feels wrong to remove Elastic here, but maybe we should say OpenSearch Contributors?

@dblock @stockholmux what do you think of this ?
s.authors = ["Elastic", "OpenSearch Contributors"]

Since this plugin is more about OSS and Apache2.0, I feel just OpenSearch Contributors should be good enough?

I'm queasy removing "Elastic".

I would use the array as mentioned by @VijayanB.

stockholmux · 2021-06-23T15:57:31Z

DEVELOPER_GUIDE.md

+
+### Run Tests
+
+We run both unit tests and integration tests inside docker environment. 


The 'royal we' here is weird. Rephrase to be first person singular 'You should run both unit....' or similar

stockholmux · 2021-06-23T15:57:55Z

DEVELOPER_GUIDE.md

+```
+
+To perform unit tests, set `INTEGRATION=false` and re-run the `docker-run.sh`.
+You don't have to set up docker again.


big D for Docker

stockholmux · 2021-06-23T16:01:55Z

DEVELOPER_GUIDE.md

+
+## Configuration for Logstash Output OpenSearch Plugin
+
+To run the Logstash Output Opensearch plugin, simply add a configuration following the below documentation.


Simply is not a great word for this type of document.

stockholmux

Tiny nit.

stockholmux · 2021-06-23T22:37:57Z

DEVELOPER_GUIDE.md

+
+Example:
+```bash
+    git clone https://github.com/[your username]/logstash-output-opensearch.git.


this should be 'git fork' to match the above text?

This is an example for git clone. similar to https://github.com/opensearch-project/OpenSearch/blob/main/DEVELOPER_GUIDE.md#git-clone-opensearch-repo

@VijayanB Ahh - This is why I can't do these at the end of the day. My bad. LGTM

stockholmux

Good on the git clone (🤦‍♂️ on my part) and a comment on the gemspec

stockholmux · 2021-06-24T19:49:37Z

DEVELOPER_GUIDE.md

+
+Example:
+```bash
+    git clone https://github.com/[your username]/logstash-output-opensearch.git.


@VijayanB Ahh - This is why I can't do these at the end of the day. My bad. LGTM

stockholmux · 2021-06-24T19:54:09Z

logstash-output-opensearch.gemspec

@@ -2,7 +2,7 @@ Gem::Specification.new do |s|
  s.name            = 'logstash-output-opensearch'
  s.version         = '1.0.0'

-  s.licenses        = ['Apache License (2.0)']
+  s.licenses        = ['Apache-2.0']
  s.summary         = "Stores logs in OpenSearch"
  s.description     = "This gem is a Logstash plugin required to be installed on top of the Logstash core pipeline using $LS_HOME/bin/logstash-plugin install gem. This gem is not a stand-alone program"
  s.authors         = ["OpenSearch"]


I'm queasy removing "Elastic".

I would use the array as mentioned by @VijayanB.

vamshin

LGTM! Thanks

Update gem spec and DEVELOPER_GUIDE. Signed-off-by: Vijayan Balasubramanian <balasvij@amazon.com>

Add exclude pattern since it is expected. Signed-off-by: Vijayan Balasubramanian <balasvij@amazon.com>

VijayanB changed the title ~~Update guide~~ Add developer guide Jun 23, 2021

VijayanB requested review from dblock, jmazanec15 and vamshin June 23, 2021 00:16

VijayanB self-assigned this Jun 23, 2021

VijayanB added the documentation Improvements or additions to documentation label Jun 23, 2021

VijayanB linked an issue Jun 23, 2021 that may be closed by this pull request

Clean up README and other files #6

Closed

VijayanB force-pushed the guide branch from f3bdd98 to 568e824 Compare June 23, 2021 00:28

vamshin requested a review from stockholmux June 23, 2021 04:23

vamshin reviewed Jun 23, 2021

View reviewed changes

dblock reviewed Jun 23, 2021

View reviewed changes

stockholmux suggested changes Jun 23, 2021

View reviewed changes

VijayanB force-pushed the guide branch from 568e824 to 1957b32 Compare June 23, 2021 17:38

VijayanB requested review from stockholmux, vamshin and dblock June 23, 2021 17:39

stockholmux suggested changes Jun 23, 2021

View reviewed changes

dblock approved these changes Jun 24, 2021

View reviewed changes

VijayanB requested a review from stockholmux June 24, 2021 17:50

stockholmux suggested changes Jun 24, 2021

View reviewed changes

vamshin approved these changes Jun 24, 2021

View reviewed changes

VijayanB force-pushed the guide branch from 1957b32 to add6b72 Compare June 25, 2021 01:00

Update guide

10a9ae1

Update gem spec and DEVELOPER_GUIDE. Signed-off-by: Vijayan Balasubramanian <balasvij@amazon.com>

VijayanB force-pushed the guide branch from add6b72 to 10a9ae1 Compare June 25, 2021 01:04

Update link checker arguments

42f9a3b

Add exclude pattern since it is expected. Signed-off-by: Vijayan Balasubramanian <balasvij@amazon.com>

VijayanB force-pushed the guide branch from 55a5271 to 42f9a3b Compare June 25, 2021 03:27

VijayanB merged commit 7de04ba into opensearch-project:develop Jun 25, 2021

VijayanB deleted the guide branch June 25, 2021 03:41


		## Getting Started

		### Git Clone logstash-output-opensearch Repo


		#### JRuby

		This plugin builds using JRuby. This means you'll need JRuby with the Bundler gem installed.


		### Git Clone logstash-output-opensearch Repo

		Fork [opensearch-project/logstash-output-opensearch](https://github.com/opensearch-project/logstash-output-opensearch) and clone locally, e.g. `git clone https://github.com/[your username]/logstash-output-opensearch.git`.


		### Authorization

		Authorization to a secure OpenSearch cluster requires read permission at index level.


		### Run Tests

		We run both unit tests and integration tests inside docker environment.


		## Configuration for Logstash Output OpenSearch Plugin

		To run the Logstash Output Opensearch plugin, simply add a configuration following the below documentation.

Add developer guide #18

Add developer guide #18

Conversation

VijayanB commented Jun 23, 2021

Description

Issues Resolved

Check List

Uh oh!

VijayanB commented Jun 23, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

stockholmux left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

stockholmux left a comment

VijayanB Jun 23, 2021 •

edited

Loading