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.

Sign up for GitHub

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

Jump to bottom

[Ingest Manager] Update dataset.* to data_stream.* in package config SO attributes #74414

Merged
merged 2 commits into from
Aug 6, 2020
Merged

[Ingest Manager] Update dataset.* to data_stream.* in package config SO attributes #74414

merged 2 commits into from
Aug 6, 2020

Conversation

jen-huang
Copy link
Contributor

@jen-huang jen-huang commented Aug 5, 2020 •
edited
Loading

Summary

This PR changes the dataset.* fields in package config saved objects to data_stream.* to support elastic/beats#20420.

@jen-huang jen-huang added v8.0.0 release_note:skip Skip the PR/issue when compiling release notes v7.10.0 v7.9.0 Team:Fleet Team label for Observability Data Collection Fleet team labels Aug 5, 2020
@jen-huang jen-huang self-assigned this Aug 5, 2020
ruflin
ruflin reviewed Aug 5, 2020
View reviewed changes
x-pack/plugins/ingest_manager/common/services/config_to_yaml.ts
@@ -10,6 +10,7 @@ const CONFIG_KEYS_ORDER = [
'id',
'name',
'revision',
'dataset',
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be data_stream? I think I miss how this works.

Copy link
Contributor Author

@jen-huang jen-huang Aug 5, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

this is just for ordering of yaml keys in the UI. I added this so that data_stream.dataset displays above data_stream.type (the same way as before when dataset.name displayed before dataset.type)

image

@jen-huang jen-huang changed the title [Ingest Manager] Update dataset.* to data_stream.* in full agent config yaml [Ingest Manager] Update dataset.* to data_stream.* in package config SO attributes Aug 5, 2020
@jen-huang jen-huang requested a review from michalpristas August 5, 2020 21:57
@jen-huang
Copy link
Contributor Author

I updated this PR to change dataset.* to data_stream.* on the package config saved object itself as well: 4e47949. It is not a trivial change but I feel confident about the changes (pending CI!) after testing with a build from elastic/beats#20420. ES data streams and logs-* and metrics-* look good. UI screenshots:

image

image

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Build metrics

async chunks size

id value diff baseline
ingestManager 1.1MB +56.0B 1.1MB

History

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

michalpristas
michalpristas approved these changes Aug 6, 2020
View reviewed changes
Copy link

@michalpristas michalpristas left a comment

Choose a reason for hiding this comment

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

tested with agent basic scenario, assigning custom config
datastream.* populated correctly and indices named according to rules

@michalpristas michalpristas mentioned this pull request Aug 6, 2020
Merged
6 tasks
@ruflin
Copy link
Contributor

ruflin commented Aug 6, 2020

Tested locally in standalone and managed mode. Only thing that will need adjustment is the logs package as it has a dataset config: https://github.com/elastic/package-storage/blob/production/packages/log/0.3.4/dataset/log/manifest.yml#L19 We should also check if there is any other package with such a config?

LGTM

@jen-huang jen-huang marked this pull request as ready for review August 6, 2020 15:07
@jen-huang jen-huang requested a review from a team as a code owner August 6, 2020 15:07
@jen-huang jen-huang requested a review from a team August 6, 2020 15:07
@jen-huang jen-huang requested a review from a team as a code owner August 6, 2020 15:07
@elasticmachine
Copy link
Contributor

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

@jen-huang jen-huang merged commit 042254f into elastic:master Aug 6, 2020
@jen-huang jen-huang deleted the ingest/data-stream-agent branch August 6, 2020 16:17
This was referenced Aug 6, 2020
Merged
Merged
jen-huang added a commit to jen-huang/kibana that referenced this pull request Aug 6, 2020
@jen-huang
[Ingest Manager] Update dataset.* to data_stream.* in package con…
90a3dd9 
…fig SO attributes (elastic#74414)

* Update `dataset.*` to `data_stream.*` in full agent config yaml

* Replace `dataset.*` with `data_stream.*` in package config saved object attributes
# Conflicts:
#	x-pack/test/page_load_metrics/es_archives/default/mappings.json
@jen-huang jen-huang mentioned this pull request Aug 6, 2020
Closed
jen-huang added a commit that referenced this pull request Aug 6, 2020
@jen-huang
[Ingest Manager] Update dataset.* to data_stream.* in package con…
ed08468 
…fig SO attributes (#74414) (#74558)

* Update `dataset.*` to `data_stream.*` in full agent config yaml

* Replace `dataset.*` with `data_stream.*` in package config saved object attributes
# Conflicts:
#	x-pack/test/page_load_metrics/es_archives/default/mappings.json
jen-huang added a commit that referenced this pull request Aug 6, 2020
@jen-huang
[Ingest Manager] Update dataset.* to data_stream.* in package con…
d772ad1 
…fig SO attributes (#74414) (#74557)

* Update `dataset.*` to `data_stream.*` in full agent config yaml

* Replace `dataset.*` with `data_stream.*` in package config saved object attributes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ruflin ruflin ruflin left review comments

@michalpristas michalpristas michalpristas approved these changes

@skh skh skh approved these changes

Assignees

@jen-huang jen-huang

Labels
release_note:skip Skip the PR/issue when compiling release notes Team:Fleet Team label for Observability Data Collection Fleet team v7.9.0 v7.10.0 v8.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@jen-huang @kibanamachine @ruflin @elasticmachine @skh @michalpristas