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

Loading external fields.yml files #11199

Closed

Conversation

kvch
Copy link
Contributor

@kvch kvch commented Mar 11, 2019

A new option external_fields is added to setup.template to let users specify their own fields.yml files in addition to the default fields or the one configured in setup.template.fields.

# List of external fields.yml files to load.
#setup.template.external_fields:
#- /path/to/external/fields.yml

Previously, fields collection from fields and configuration was done in multiple places in the codebase. This lead to inconsistencies when loading the template when loading dashboards. This PR moves the process to a single place. It still contains a few hacks, as I do not want to introduce any breaking change in the configuration.

The ideal configuration would be the following:

setup.fields.file: /path/to/fields.yml
setup.fields.append:
- name: field_name
  type: field_type
setup.fields.external_fields:
- /path/to/external/fields.yml

This could be used when loading the template and dashboards.

I compromised to add the new option to setup.template, as fields settings are required by the template. I introduced a new asset.Supporter which collect fields from the configuration in setup.template. This supporter is able to pass the correct fields list to both template loading and dashboard loading. I moved adding extra fields in append_fields from template loading, so these fields are not missing when dashboards are loaded.

Loading JSON templates correctly when loading template and dashboards is not in the scope of this PR.

New fields.yml loading process

Loading of fields is changed to the following process.

  1. Collect fields from either the default fields.yml files provided by the Beat or get the fields from setup.template.fields
  2. User defined fields are gathered from the fields.yml files from setup.template.external_fields.
  3. More fields are added to these fields which is configured in setup.template.append_fields.

Fixes

From now on it is possible to load custom template into ES when loading dashboards. It means that users can load their own dashboards with user defined fields using a Beat.

TODO

  • add configuration option to reference
  • add E2E test cases

Depends on #11198

paths []string
}

func DefaultSupport(log *logp.Logger, beat string, cfg *common.Config) (Supporter, error) {

Choose a reason for hiding this comment

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

exported function DefaultSupport should have comment or be unexported

@kvch kvch force-pushed the feature-libbeat-external-fields-yml branch 2 times, most recently from e2aac89 to 4c147ae Compare March 13, 2019 11:34
paths []string
}

func DefaultSupport(log *logp.Logger, beat string, cfg *common.Config) (Supporter, error) {

Choose a reason for hiding this comment

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

exported function DefaultSupport should have comment or be unexported

@kvch kvch force-pushed the feature-libbeat-external-fields-yml branch 4 times, most recently from 730fb9d to 7c37a28 Compare March 14, 2019 09:59
@kvch kvch force-pushed the feature-libbeat-external-fields-yml branch from 7c37a28 to 564d098 Compare March 14, 2019 10:01
@@ -867,6 +867,10 @@ output.elasticsearch:
#- name: field_name
# type: field_type

# List of external fields.yml files to load.
#setup.template.external_fields:
#- /path/to/external/fields.yml
Copy link

Choose a reason for hiding this comment

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

To me the name kind of clashes with setup.template.fields. Like: which one has priority, does one replace the other, are the two files combined, why not have setup.template.fields accept a list of files.

@@ -77,6 +77,8 @@ type Beat struct {
Config beatConfig
RawConfig *common.Config // Raw config that can be unpacked to get Beat specific config data.

Mapping mapping.Supporter // Get all fields of the Beat
Copy link

Choose a reason for hiding this comment

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

I so wish I hadn't named it 'Supporter' :)

if err != nil {
return err
}
m := b.index.Manager(esClient, idxmgmt.BeatsAssets(fields))
Copy link

Choose a reason for hiding this comment

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

The 'BeatsAssets' has been a place-holder, due to not having some better options availble. I'd be in favor of passing a more rich interface instead of a raw blob to be parsed and interpreted somewhere else.

If possible we want to be able to install multiple templates in the future, each with it's own set of fields. A more generic type better being integrated with the index manager would be helpful.

The index manager is supposed to be the only component/interface Beats should use for configuring and preparing indices (facade pattern). Maybe we can move the fields/templating support inside the index manager?

@kvch
Copy link
Contributor Author

kvch commented Jun 23, 2021

Closing because this PR is outdated and we are moving towards integration in the long run.

@kvch kvch closed this Jun 23, 2021
@mergify
Copy link
Contributor

mergify bot commented Sep 22, 2021

This pull request does not have a backport label. Could you fix it @kvch? 🙏
To fixup this pull request, you need to add the backport labels for the needed
branches, such as:

  • backport-v./d./d./d is the label to automatically backport to the 7./d branch. /d is the digit

NOTE: backport-skip has been added to this pull request.

@mergify mergify bot added the backport-skip Skip notification from the automated backport with mergify label Sep 22, 2021
@botelastic botelastic bot added the needs_team Indicates that the issue/PR needs a Team:* label label Sep 22, 2021
@botelastic
Copy link

botelastic bot commented Sep 22, 2021

This pull request doesn't have a Team:<team> label.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-skip Skip notification from the automated backport with mergify needs_team Indicates that the issue/PR needs a Team:* label
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants