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

78024: move transform out of dataset #78216

Merged
merged 12 commits into from
Sep 29, 2020

Conversation

nnamdifrankie
Copy link
Contributor

@nnamdifrankie nnamdifrankie commented Sep 22, 2020

Summary

#78024

  • move transform install from dataset to package root under elasticsearch.
  • remove datastream configuration for metadata current, it is no longer a datastream.

Documentation

  • The transform is not related to a dataset and should be placed under the ElasticSearch directory which is the same level as the dataset. Along with other ElasticSearch assets like IndexTemplate. A transform under the dataset directory will be treated as a transform file under the dataset directory. In the future the package specification may be implemented to restrict it to the ElasticSearch directory.

  • A transform creates the destination index if it does not exist. With this version it is assumed that the lifespan of destination index is tied to the lifespan of the transform if it is managed by Ingest Manager. This means that when the transform is removed from the package the destination index is also deleted as a result. This cycle also occurs during upgrade of a package, and it is expected that transform will start from checkpoint zero. In the future this behavior may be handled by the transform service but will remain a key feature of the system.

  • Given that the index is created by the transform the best way to apply index mapping and settings to the index is to include an index template in the package that has an index pattern that matches the destination index of the transform. It is possible to have an existing index template with a pattern that matches the destination index for the template, but it is recommended to create new templates and patterns in order to avoid unexpected indexing and search outcomes on the destination index. In addition, if you remove the transform it is important to also remove the index template, as the index will be removed by Ingest Manager. Since the index template is a separate assets it has to be removed from the package to trigger its removal.

  • For the transform to start the source index must exist, if you are using a concrete index. If you are using a datastream then the source index has to be a wildcard pattern for the transform to be started successfully.

Checklist

Delete any items that are not applicable to this PR.

@nnamdifrankie nnamdifrankie added release_note:skip Skip the PR/issue when compiling release notes v7.10.0 labels Sep 22, 2020
@@ -8,7 +8,7 @@ export const eventsIndexPattern = 'logs-endpoint.events.*';
export const alertsIndexPattern = 'logs-endpoint.alerts-*';
export const metadataIndexPattern = 'metrics-endpoint.metadata-*';
export const metadataCurrentIndexPattern = 'metrics-endpoint.metadata_current-*';
Copy link
Member

Choose a reason for hiding this comment

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

As this will not follow the index pattern, this should change. I'm starting to wonder if we should even put the data under metrics prefix or outside? If we put it under metrics-* it can't contain to -.

@nnamdifrankie nnamdifrankie marked this pull request as ready for review September 24, 2020 14:09
@nnamdifrankie nnamdifrankie requested a review from a team as a code owner September 24, 2020 14:09
@nnamdifrankie nnamdifrankie requested a review from a team September 24, 2020 14:09
@nnamdifrankie nnamdifrankie requested a review from a team as a code owner September 24, 2020 14:09
@botelastic botelastic bot added the Team:Fleet Team label for Observability Data Collection Fleet team label Sep 24, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/ingest-management (Team:Ingest Management)

@@ -88,21 +85,21 @@ export default ({ getPageObjects, getService }: FtrProviderContext) => {
});

it('finds data after load and polling', async () => {
await esArchiver.load('endpoint/metadata/destination_index', { useCreate: true });
await pageObjects.endpoint.waitForTableToHaveData('endpointListTable', 1100);
await esArchiver.load('endpoint/metadata/api_feature', { useCreate: true });
Copy link
Contributor

Choose a reason for hiding this comment

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

these tests are skipped right now, so it's OK for now, but I'd rather we not wait for the transform in the integration tests because of the long wait times.

IMO, the UI integration tests can just test that we can call the AP when there's data there and the UI renders correctly

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well that should be tested in unit tests, the integration test should be integration. And ingesting into metadata current may not be a good test but I get the point. To get the test to pass again I had to change a few line that were removed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also the old setup will fail since we remove the matadata current stream.

path: string;
dataset: Dataset;
}

export const installTransformForDataset = async (
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe this should be renamed to remove ForDataset?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes will remove

@nnamdifrankie
Copy link
Contributor Author

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

async chunks size

id value diff baseline
securitySolution 10.2MB -16.0B 10.2MB

History

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

@nnamdifrankie nnamdifrankie merged commit 9e9a48b into elastic:master Sep 29, 2020
@nnamdifrankie nnamdifrankie deleted the 78024_fix_transform_error branch September 29, 2020 14:33
nnamdifrankie added a commit to nnamdifrankie/kibana that referenced this pull request Sep 29, 2020
* 78024: move transform out of dataset

* Change index prefix

* 78024: fix tests, remove vestiges

* 78024: remove index defined in the transform when transform is removed.

* 78024: clean up

* 78024: fix build

* 78024: add comment

* 78024: remove test I added

* 78024: more removal, will add in next PR

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
# Conflicts:
#	x-pack/plugins/ingest_manager/server/services/epm/packages/install.ts
nnamdifrankie added a commit that referenced this pull request Sep 29, 2020
[7.x] 78024: move transform out of dataset (#78216)
phillipb added a commit to phillipb/kibana that referenced this pull request Sep 29, 2020
…-to-timeline

* 'master' of github.com:elastic/kibana: (22 commits)
  update apm index pattern (elastic#78732)
  78024: move transform out of dataset (elastic#78216)
  [QA][Code Coverage] Upload the coverage static site before ingestion (elastic#78695)
  [Discover] Make _source field not clickable (elastic#78698)
  [Fleet] Rename Ingest Manager => Fleet, Fleet => Agents in the UI (elastic#78685)
  [APM] Review feedback from distribution + transaction metrics (elastic#78752)
  [Ingest pipelines] Add ability to stop pipeline simulation  (elastic#78183)
  [CSM] Fix core vital legend background (elastic#78273)
  [Usage Collection] [schema] Support spreads + `canvas` definition (elastic#78481)
  fix lodash imports (elastic#78456)
  [Maps] Add layer type preview icons (elastic#78650)
  [APM] Use transaction metrics for distribution charts (elastic#78484)
  [Uptime] Ml anomaly alert edit (elastic#76909)
  [ML] Limit exposing shared static code through ml/public/index.ts. (elastic#77745)
  making expression debug info serializable (elastic#78727)
  fix lodahs imports in app-arch code (elastic#78582)
  Make Field a React.lazy export (elastic#78483)
  [Security Solution] Improves detections tests (elastic#77295)
  [TSVB] Different field format on different series is ignored (elastic#78138)
  RFC: Improve saved object migrations (elastic#66056)
  ...
@nnamdifrankie
Copy link
Contributor Author

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release_note:skip Skip the PR/issue when compiling release notes Team:Fleet Team label for Observability Data Collection Fleet team v7.10.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants