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

[Behavioral Analytics] Analytics pipeline to the index template registry #96104

Merged

Conversation

joemcelroy
Copy link
Member

@joemcelroy joemcelroy commented May 15, 2023

With the changes added in #95782, we are now able to declare and install the behavioral analytics pipeline required by the behavioral analytics Index template.

This PR unblocks behavioral analytics + we can enable the YAML tests which was blocked as the index template could not be installed. The reason is the above PR change only allows index templates that have a pipeline, can be installed if the referring pipeline has been installed.

This impacts where the pipeline now eagerly installed (before was lazily installed https://github.com/elastic/elasticsearch/pull/95775/files) and therefore the geoip processor we depend on will eagerly load the geoip database, increasing the storage footprint of elasticsearch for all customers. See more information here https://github.com/elastic/elasticsearch-benchmarks/issues/1498#issuecomment-1525499333

We will address this optimisation in a future PR.

@joemcelroy joemcelroy added :EnterpriseSearch/Application Enterprise Search Team:Enterprise Search Meta label for Enterprise Search team labels May 15, 2023
Copy link
Contributor

@jimczi jimczi left a comment

Choose a reason for hiding this comment

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

Thanks @joemcelroy ! The change looks good. The AnalyticsTemplateRegistryTests need some changes too, are you looking at it too?

@joemcelroy
Copy link
Member Author

Thanks @joemcelroy ! The change looks good. The AnalyticsTemplateRegistryTests need some changes too, are you looking at it too?

Yep! Looks like down to the unexpected pipeline put call to install the declared BA pipeline. Will address it now

Copy link
Contributor

@jimczi jimczi left a comment

Choose a reason for hiding this comment

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

LGTM to unblock but we'll need to rewrite the tests for ingest pipelines in
AnalyticsTemplateRegistryTests?

@joemcelroy
Copy link
Member Author

LGTM to unblock but we'll need to rewrite the tests for ingest pipelines in AnalyticsTemplateRegistryTests?

Added one test to validate that the ingest pipeline is installed but agree could be better coverage added. Will them after this PR later on this week.

@joemcelroy joemcelroy marked this pull request as ready for review May 15, 2023 14:17
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/ent-search-eng (Team:Enterprise Search)

@jimczi jimczi added the >test Issues or PRs that are addressing/adding tests label May 15, 2023
@joemcelroy joemcelroy merged commit 832987e into elastic:main May 15, 2023
@joemcelroy joemcelroy deleted the ba-install-pipeline-via-index-template branch May 15, 2023 16:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:EnterpriseSearch/Application Enterprise Search Team:Enterprise Search Meta label for Enterprise Search team >test Issues or PRs that are addressing/adding tests v8.9.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants