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

Integrations developer guide #769

Merged
merged 18 commits into from
Sep 7, 2021

Conversation

bmorelli25
Copy link
Member

@bmorelli25 bmorelli25 commented Jun 2, 2021

Summary

The majority of this content was written by others. Some of the docs have been copy/pasted as-is (testing docs), while other docs have been changed considerably (build instructions). This PR won't be the final PR for the integrations developer guide, but I'd like to get this merged soon-ish so that others can start contributing as well.

Build locally

gh pr checkout 769
$GIT_HOME/docs/build_docs --doc $GIT_HOME/observability-docs/docs/en/integrations/index.asciidoc --resource $GIT_HOME/package-spec/versions --chunk 1 --open

Update preview

  1. Update conf.yaml:
   package-spec:         https://github.com/elastic/package-spec.git

          - title:      Integrations Developer Guide
            prefix:     en/integrations-developer
            current:    master
            branches:   [ master ]
            live:       [ master ]
            index:      docs/en/integrations/index.asciidoc
            chunk:      1
            tags:       Integrations/Developer
            subject:    Integrations
            sources:
              -
                repo:   observability-docs
                path:   docs/en
              -
                repo:   docs
                path:   shared/versions/stack/{version}.asciidoc
              -
                repo:   docs
                path:   shared/attributes.asciidoc
              -
                repo:   package-spec
                path:   versions
  1. Run the build_docs --all command:
$GIT_HOME/docs/build_docs --all \
    --target_repo git@github.com:elastic/built-docs.git \
    --target_branch integration-dev-guide \
    --push \
    --sub_dir observability-docs:master:$GIT_HOME/observability-docs

Related issues

Closes #583
Closes #984

Related PRs

Add to doc build: elastic/docs#2210

@apmmachine
Copy link
Contributor

apmmachine commented Jun 2, 2021

A documentation preview will be available soon:

@bmorelli25 bmorelli25 changed the title Test PR, please ignore WIP: Integrations developer guide Jun 4, 2021
@sorantis sorantis self-requested a review June 7, 2021 16:59
@bmorelli25
Copy link
Member Author

Thank you for all the resources, @sorantis!

@bmorelli25 bmorelli25 changed the title WIP: Integrations developer guide Integrations developer guide Jun 30, 2021
@bmorelli25 bmorelli25 self-assigned this Jun 30, 2021
@bmorelli25 bmorelli25 requested a review from a team June 30, 2021 21:56
@bmorelli25 bmorelli25 marked this pull request as ready for review June 30, 2021 21:56
Copy link
Contributor

@mtojek mtojek left a comment

Choose a reason for hiding this comment

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

Nice progress on the package docs!

Few points to consider:

elastic-package format - what is it, when to use it, which files will be formatted

elastic-package check - ultimate check before pushing it to the repository. If the "check" is happy, we know that the code is built correctly, formatted properly and it's aligned with the spec (linting passed).

You could also provide some guidance for designing forms in Kibana (UI and UX), cover recommendation for names, labels, descriptions. I tried to put them in: https://github.com/elastic/integrations/blob/master/docs/tips_for_building_integrations.md#manifest-files-1

root folder, and the command will only operate on the contents of that package.

// *************************
// The following is copied directly from
Copy link
Contributor

Choose a reason for hiding this comment

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

Hi @bmorelli25 ,

I see that you copied content from elastic-package READMEs. Do you have a plan for updating it in the future? We used to modify README files from time to time, so these .asciidocs will become outdated really soon.

Copy link
Member Author

Choose a reason for hiding this comment

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

Hi @mtojek! Sorry for the delay. A long PTO break and other projects have gotten in the way. Returning to this now.

Do you have a plan for updating it in the future?

Concrete plan, no. Eventually, we'd like the only documentation for these commands to exist in the Integrations developer guide. We discussed documenting these commands in a yaml file in the elastic-package repository with a script that can be used to extract and build to mdx. But that's still a pipe dream at this point.

[[system-kubernetes]]
=== Kubernetes service deployer

The Kubernetes service deployer requires the `_dev/deploy/k8s` directory to be present. It can include additional `*.yaml` files to deploy
Copy link
Contributor

Choose a reason for hiding this comment

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

Same concern about service deployers. Does it mean that we have to apply changes in both places to keep them up-to-date?

Copy link
Member Author

Choose a reason for hiding this comment

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

It's a good point. I'll bring this up in our weekly tomorrow. There might just be too many manual copy/paste steps required to move forward with this PR at this point.

Copy link
Member Author

@bmorelli25 bmorelli25 Jul 21, 2021

Choose a reason for hiding this comment

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

In the end, I may just have to update things manually for a release or two until we figure out the automation side of things


_Context: global_

Use this command to add, remove, and manage multiple config profiles.
Copy link
Contributor

Choose a reason for hiding this comment

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

It's also a good candidate for an example. Keep in mind this is a feature for advanced users who want to modify the default setup.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'd probably need help on an example here. I'm not sure I understand what the command is used for.

Copy link
Contributor

@EamonnTP EamonnTP left a comment

Choose a reason for hiding this comment

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

@bmorelli25 Looks great so far. It's quite lengthy, so I only managed to review a few of the topics - just minor suggestions. I'll review the rest of the topics tomorrow.

Co-authored-by: EamonnTP <Eamonn.Smith@elastic.co>
Copy link
Contributor

@EamonnTP EamonnTP left a comment

Choose a reason for hiding this comment

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

Second round! Just minor suggestions. Otherwise, it looks great!

Co-authored-by: EamonnTP <Eamonn.Smith@elastic.co>
@bmorelli25
Copy link
Member Author

This is ready for final review. I'll merge on Tuesday. Thanks to everyone for their help.

@bmorelli25 bmorelli25 merged commit 7828b7c into elastic:master Sep 7, 2021
@bmorelli25 bmorelli25 deleted the integration-dev-guide branch September 7, 2021 18:24
bmorelli25 added a commit to bmorelli25/observability-docs that referenced this pull request Sep 7, 2021
Co-authored-by: sorantis <dimitri.mazmanov@elastic.co>
Co-authored-by: mtojek <marcin.tojek@elastic.co>
Co-authored-by: EamonnTP <Eamonn.Smith@elastic.co>
bmorelli25 added a commit to bmorelli25/observability-docs that referenced this pull request Sep 7, 2021
Co-authored-by: sorantis <dimitri.mazmanov@elastic.co>
Co-authored-by: mtojek <marcin.tojek@elastic.co>
Co-authored-by: EamonnTP <Eamonn.Smith@elastic.co>
bmorelli25 added a commit that referenced this pull request Sep 7, 2021
Co-authored-by: sorantis <dimitri.mazmanov@elastic.co>
Co-authored-by: mtojek <marcin.tojek@elastic.co>
Co-authored-by: EamonnTP <Eamonn.Smith@elastic.co>

Co-authored-by: sorantis <dimitri.mazmanov@elastic.co>
Co-authored-by: mtojek <marcin.tojek@elastic.co>
Co-authored-by: EamonnTP <Eamonn.Smith@elastic.co>
bmorelli25 added a commit that referenced this pull request Sep 7, 2021
Co-authored-by: sorantis <dimitri.mazmanov@elastic.co>
Co-authored-by: mtojek <marcin.tojek@elastic.co>
Co-authored-by: EamonnTP <Eamonn.Smith@elastic.co>

Co-authored-by: sorantis <dimitri.mazmanov@elastic.co>
Co-authored-by: mtojek <marcin.tojek@elastic.co>
Co-authored-by: EamonnTP <Eamonn.Smith@elastic.co>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants