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

[Ingest] Enable adding system monitoring (datasource) during Agent Config create #59957

Conversation

paul-tavares
Copy link
Contributor

@paul-tavares paul-tavares commented Mar 11, 2020

❗️DO NOT MERGE❗️ until #59376 is merged. Change this PR's base to master once it is.

Summary

Resolves #59564

Enables the "Collect system metrics" switch on the Agent Configuration Create flyout. When checked (default), a Datasource will be created from the System package and added to the newly created Agent Configuration

agent_config_create_-sys-monitoring-2020 03 11

Checklist

Manually tested:

  • Throwing an exception while attempting to create the System datasource will still result in a successful Agent Creation result
  • When switch in checked in UI, validated Datasource is created and added to Agent Config
  • When switch is un-checked in UI, validated Datasource is not created.

skh and others added 30 commits November 14, 2019 11:45
* Add directory structure for server/lib.

* 'tests' seems to be more common than 'test'

* Make CI happy
* [EPM] Add basic documentation directory

Having the doc directory around allows us to easily add docs from here on to document how EPM works.

To run the docs build, use the following command from the kibana directory:

```
../docs/build_docs --doc docs/epm/index.asciidoc --open
```

The above assumes that docs (https://github.com/elastic/docs) are checked out in the same directory as Kibana.

With this change, the EPM docs build is not included yet in the overall docs build. For this adjustments to https://github.com/elastic/docs/blob/master/conf.yaml must be made.
This PR adds the very basic index template we will use for the packages. It contains all the basic settings and some examples. The examples will be remove as soon as we have an actual implementation with packages but for now is convenient to see if it is a valid package.

This code is put into the lib directory as it does not tie directly into any handlers.

It also adds an functional tests for loading a template. This means we have a way to check if a template is valid in Elasticsearch. Based on this we can check in the future all our generated templates for validity with Elasticsearch.

To run the functional test, go to the Kibana x-pack directory. Start the first command:

```
node scripts/functional_tests_server.js --config test/epm_api_integration/config.ts
```

Keep the above running and switch to an other Terminal. Now run:

```
node scripts/functional_test_runner.js --config x-pack/test/epm_api_integration/config.ts
```
* Add directory structure for server/lib.

* 'tests' seems to be more common than 'test'

* Make CI happy

* Implement pipeline rewriting.

* Add more testcases

* For posterity (comment change)

* Allow beats-style template delimiters

* Be more succinct

* Document better
See elastic#50609 (comment)

Discussed in Slack and agree to revert for now. Can track down the issues & restore later
…astic#50609)

* cleanup assets, filter assets for those currently supported

* removed unused type

* fix type

* add comment for better type

* change type name to be more descriptive
* hardcode image width for ie11

* eslint

* improve comment

* add maxWidth
* plugin.start now does reactdom.render vs returning react element

export plugin function from public/index

* Move setClient call from plugin.start to plugin.setup

* Use `useUiSetting$` from `useKibana` hooks
Can't use useKibana outside a React component.

Reverting to prior approach since it's still NP. Can revisit context usage in a followup PR
* Support basic "click button -> show spinner -> installed" install flow

* Remove incorrect comments. Add TS return types to data functions.
* Use NP feature_catalogue.register

* Use type from NP plugin
* Update (all remaining?) references from integrations_manager to EPM

* Update path in i18n file
* change min max version to single range value

* add elastic stack icon and change text

* remove badge from version and use code font

* remove euistyled

* add back version component lost in merge

* remove euiStyled

* remove old file
John Schulz and others added 16 commits March 10, 2020 16:20
* Make app setup loading state prettier

* Add review step of datasource wizard

* Change name to ID in agent config datasources field

* Fix types

* Add stored datasource to agent datasource unit tests

* Adjustment of registry typings and which registry copy fields to show to sync with elastic/package-registry#242

* Fix `multi` vars not populating with array: elastic#59724

* Account for if a stream is enabled by default from registry package definition: elastic#59724

* Adjust tests to account for last two commits

* Fix review page back link

* Fix d'oh typo
…#59783)

- Syncs Agent Config Details header to design
- Includes sub navigation tabs connected to route URL
- Agent Config List Create data source row action enabled
…st-59564-agent-config-create-with-sys-metrics
…st-59564-agent-config-create-with-sys-metrics
@paul-tavares paul-tavares added Feature:EPM Fleet team's Elastic Package Manager (aka Integrations) project Feature:Fleet Fleet team's agent central management project labels Mar 11, 2020
@paul-tavares paul-tavares self-assigned this Mar 11, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/ingest-management (Feature:EPM)

@@ -193,3 +222,35 @@ export const getFullAgentConfig: RequestHandler<TypeOf<
});
}
};

const buildSystemDatasource = async (
Copy link
Contributor Author

Choose a reason for hiding this comment

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

It feels like this function belongs in one of the services as a method - maybe Datasources?
Let me know if its best for it to live elsewhere.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think this logic could be split up into two services.

  1. A new service inside x-pack/plugins/ingest_manager/common/services/package_to_config.ts that builds a complete new datasource from package info:
export const packageToConfigDatasource = (
  packageInfo: PackageInfo,
  configId: string,
  outputId: string,
  datasourceName?: string
): NewDatasource => {
      return {
        name: datasourceName || `${packageInfo.name}-1`,
        package: {
          name: packageInfo.name,
          title: packageInfo.title,
          version: packageInfo.version,
        },
        enabled: true,
        config_id: configId,
        output_id: outputId,
        inputs: packageToConfigDatasourceInputs(packageInfo),
      };
}
  1. A new datasourceService that retrieves the information needed to use that service inside x-pack/plugins/ingest_manager/server/services/datasource.ts:

(almost identical to what you currently have here, just removing sys and changing return line to use the generic service)

const createDatasourceFromPackage = async (
  soClient: SavedObjectsClientContract,
  pkgName: string,
): Promise<NewDatasource | undefined> => {
  const pkgInstall = await findInstalledPackageByName({
    savedObjectsClient: soClient,
    pkgName,
  });
  if (pkgInstall) {
    const [pkgInfo, defaultOutputId] = await Promise.all([
      getPackageInfo({
        savedObjectsClient: soClient,
        pkgkey: `${pkgInstall.name}-${pkgInstall.version}`,
      }),
      outputService.getDefaultOutputId(soClient),
    ]);
    if (pkgInfo) {
      return packageToConfigDatasource(pkgInfo, '', defaultOutputId);
    }
  }
};

What do you think? I can see the generic service inside /common being useful elsewhere too, hence my not putting everything inside datasourceService. It also makes it easier to write unit tests for.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks for the feedback @jen-huang . This makes sense to me and I agree that that the common service to top-level /common/services will be helpful in a few places (maybe even for Endpoint as we will need to create datasources for Policy). I have also noticed that we generate the package key manually in several places - maybe a service to handle that as well so that the generation of the value can be centralized?

@jen-huang jen-huang self-requested a review March 12, 2020 20:30
Copy link
Contributor

@jen-huang jen-huang left a comment

Choose a reason for hiding this comment

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

I have a few suggestions - let me know if they make sense.

await agentConfigService.assignDatasources(soClient, agentConfig.id, [sysDatasource.id]);
}
}

const body: CreateAgentConfigResponse = { item: agentConfig, success: true };
Copy link
Contributor

Choose a reason for hiding this comment

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

the agentConfig that is returned here does not contain the most up-to-date information for the newly created config (aka it won't have the system datasource inside). I think we should retrieve this agent config again after creating system datasource.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done :) - thanks

@@ -193,3 +222,35 @@ export const getFullAgentConfig: RequestHandler<TypeOf<
});
}
};

const buildSystemDatasource = async (
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this logic could be split up into two services.

  1. A new service inside x-pack/plugins/ingest_manager/common/services/package_to_config.ts that builds a complete new datasource from package info:
export const packageToConfigDatasource = (
  packageInfo: PackageInfo,
  configId: string,
  outputId: string,
  datasourceName?: string
): NewDatasource => {
      return {
        name: datasourceName || `${packageInfo.name}-1`,
        package: {
          name: packageInfo.name,
          title: packageInfo.title,
          version: packageInfo.version,
        },
        enabled: true,
        config_id: configId,
        output_id: outputId,
        inputs: packageToConfigDatasourceInputs(packageInfo),
      };
}
  1. A new datasourceService that retrieves the information needed to use that service inside x-pack/plugins/ingest_manager/server/services/datasource.ts:

(almost identical to what you currently have here, just removing sys and changing return line to use the generic service)

const createDatasourceFromPackage = async (
  soClient: SavedObjectsClientContract,
  pkgName: string,
): Promise<NewDatasource | undefined> => {
  const pkgInstall = await findInstalledPackageByName({
    savedObjectsClient: soClient,
    pkgName,
  });
  if (pkgInstall) {
    const [pkgInfo, defaultOutputId] = await Promise.all([
      getPackageInfo({
        savedObjectsClient: soClient,
        pkgkey: `${pkgInstall.name}-${pkgInstall.version}`,
      }),
      outputService.getDefaultOutputId(soClient),
    ]);
    if (pkgInfo) {
      return packageToConfigDatasource(pkgInfo, '', defaultOutputId);
    }
  }
};

What do you think? I can see the generic service inside /common being useful elsewhere too, hence my not putting everything inside datasourceService. It also makes it easier to write unit tests for.

@paul-tavares paul-tavares changed the base branch from feature-ingest to master March 13, 2020 15:08
@paul-tavares
Copy link
Contributor Author

Closing this. New PR opened against master - #60111

@paul-tavares paul-tavares deleted the task/ingest-59564-agent-config-create-with-sys-metrics branch March 13, 2020 15:15
@kibanamachine
Copy link
Contributor

💚 Build Succeeded

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@jen-huang jen-huang added the Team:Fleet Team label for Observability Data Collection Fleet team label Mar 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:EPM Fleet team's Elastic Package Manager (aka Integrations) project Feature:Fleet Fleet team's agent central management project Team:Fleet Team label for Observability Data Collection Fleet team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Ingest] Add ability to Collect System Metrics when creating an Agent Config