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

[WIP] Introduce dynamic index patterns #10450

Closed
wants to merge 6 commits into from

Conversation

ruflin
Copy link
Member

@ruflin ruflin commented Jan 31, 2019

With the introduction of migration.enabled config option the index pattern must generated on the fly to skip migration alias if needed.

Currently this PR generates the index pattern as a file in the Kibana directory and then it reads in the data as before. This so far seems to work but as not ideal. The fieldsYML data should be loaded directly from the go fields and the file should never be generated on disk.

TODO

  • Stop generation of index pattern on make update
  • Make sure packaging works as expected withut index pattern
  • Remove directory
  • Introduce command to export index pattern -> needed?

@ruflin ruflin added in progress Pull request is currently in progress. Team:Integrations Label for the Integrations team v7.0.0-beta1 labels Jan 31, 2019
@ruflin ruflin self-assigned this Jan 31, 2019
@ruflin ruflin requested a review from a team as a code owner January 31, 2019 08:20
With the introduction of `migration.enabled` config option the index pattern must generated on the fly to skip migration alias if needed.

Currently this PR generates the index pattern as a file in the Kibana directory and then it reads in the data as before. This so far seems to work but as not ideal. The fieldsYML data should be loaded directly from the go fields and the file should never be generated on disk.

TODO

* Stop generation of index pattern on make update
* Make sure packaging works as expected withut index pattern
* Remove directory
* Introduce command to export index pattern -> needed?
remove kibana index pattern export step as not needed anymore, also remove it from makefile

strangely if now the index pattern is generated it reports duplicates. Needs investigation.
// Generate index pattern
version, _ := common.NewVersion("7.0.0") // TODO: dynamic version
//beatVersion, _ := common.NewVersion("7.0.0") // TODO: dynamic version
//indexP, _ := esConfig.String("index", 0)
Copy link
Member Author

Choose a reason for hiding this comment

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

@urso I think this also ties into your current work around indexing changes. Where should the config which index pattern should be used go for Kibana setup part?

@@ -81,24 +83,11 @@ func getKibanaClient(ctx context.Context, cfg *common.Config, retryCfg *Retry, r
return client, nil
}

func (loader KibanaLoader) ImportIndex(file string) error {
func (loader KibanaLoader) ImportIndex() error {

Choose a reason for hiding this comment

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

exported method KibanaLoader.ImportIndex should have comment or be unexported

}

func NewKibanaLoader(ctx context.Context, cfg *common.Config, dashboardsConfig *Config, hostname string, msgOutputter MessageOutputter) (*KibanaLoader, error) {
func NewKibanaLoader(ctx context.Context, cfg *common.Config, dashboardsConfig *Config, hostname string, msgOutputter MessageOutputter, fields common.MapStr) (*KibanaLoader, error) {

Choose a reason for hiding this comment

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

exported function NewKibanaLoader should have comment or be unexported

}

func NewImporter(version common.Version, cfg *Config, loader Loader) (*Importer, error) {
func NewImporter(version common.Version, cfg *Config, loader *KibanaLoader) (*Importer, error) {

Choose a reason for hiding this comment

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

exported function NewImporter should have comment or be unexported

@@ -20,6 +20,8 @@ The list below covers the major changes between 7.0.0-alpha2 and master only.

==== Breaking changes

- Remove support for loading dashboards into Elastic Stack version 5.x. {pull}10451[10451]
Copy link
Member Author

Choose a reason for hiding this comment

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

Taken over for 10451 as it was missing there.

@@ -312,7 +304,7 @@ func (imp Importer) ImportKibanaDir(dir string) error {

check := []string{}
if !imp.cfg.OnlyDashboards {
check = append(check, "index-pattern")
imp.loader.ImportIndex()
Copy link
Member Author

Choose a reason for hiding this comment

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

This breaks current behaviour in that it makes it impossible to load an index pattern from a file and requires the one from Beats.

@simitt
Copy link
Contributor

simitt commented Jan 31, 2019

@ruflin APM does not support loading the Kibana index pattern via the Server, but it is loaded directly via Kibana. Since the whole ES template and mapping logic is in the APM Server we still rely on the index pattern being created with make update so we can sync it and ensure we bundle the latest index pattern with Kibana.
For this requirements, please ensure to still provide an option to generate and export the Kibana index pattern. In case you are aiming for sending the index pattern to ES during ingestion time (which I understand the dynamic part implies) please also provide an option to disable this.

@ruflin
Copy link
Member Author

ruflin commented Feb 1, 2019

Closing this in favor of #10478 as I had to take a bit a different approach because the generation has to stay around but can be an exporter and also the old loading mechanism for zip file has to stay around for now.

@ruflin ruflin closed this Feb 1, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in progress Pull request is currently in progress. Team:Integrations Label for the Integrations team v7.0.0-beta1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants