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

[Fleet] Honor value of source_mode for data streams #141211

Closed
Tracked by #132818
kpollich opened this issue Sep 21, 2022 · 19 comments · Fixed by #147019 or #147034
Closed
Tracked by #132818

[Fleet] Honor value of source_mode for data streams #141211

kpollich opened this issue Sep 21, 2022 · 19 comments · Fixed by #147019 or #147034
Assignees
Labels
Team:Fleet Team label for Observability Data Collection Fleet team

Comments

@kpollich
Copy link
Member

kpollich commented Sep 21, 2022

In elastic/package-spec#419, we introduced the source_mode field for data streams, which allows packages to opt in to synthetic _source. Fleet also includes a UI toggle for this value for each data stream as of #140095.

Per the package spec, if the source_mode for a given data stream is

  • synthetic - The UI toggle defaults to enabled and is editable by the user
  • Not provided, e.g. undefined - The UI toggle defaults to disabled and is editable by the user
  • default - The UI toggle defaults to disabled and is not editable by the user
@kpollich kpollich added the Team:Fleet Team label for Observability Data Collection Fleet team label Sep 21, 2022
@elasticmachine
Copy link
Contributor

Pinging @elastic/fleet (Team:Fleet)

@jlind23
Copy link
Contributor

jlind23 commented Sep 22, 2022

@kpollich @jen-huang would it be possible to make it happened in one of the two next sprints?

@hop-dev
Copy link
Contributor

hop-dev commented Oct 28, 2022

@kpollich data_streams.elasticsearch.source_mode currently isn't returned by the registry, so we will need to implement #143198 before we can tackle this.

@hop-dev
Copy link
Contributor

hop-dev commented Oct 28, 2022

Blocked by #143198

@ruflin
Copy link
Contributor

ruflin commented Nov 8, 2022

As soon as this feature lands, I expect packages to adopt synthetic source for metrics. As synthetic source is only available in newer versions of Elasticsearch, it would mean these packages have to require a newer compatibility version. This is fine for new packages. For older packages, it would be nice if these could set synthetic_source but not have to change the compatibility, instead it would be handled by Fleet. Lets assume synthetic source is available in 8.20 (made up number), the package has compatibility to 7.17. As soon as the user upgrades to 8.20, data streams are rolled over (not a must) and and synthetic source is added to the template. Even though synthetic source is already available in 8.15 (made up number) as experimental, it would not be enabled there.

@jsoriano Do we need an additional flag to enable synthetic source for releases where it is still experimental for testing purpose? Or is the feature in Fleet to enable it enough?

@kpollich kpollich assigned criamico and unassigned hop-dev Nov 8, 2022
@hop-dev
Copy link
Contributor

hop-dev commented Nov 8, 2022

@criamico when you pick this up, the test package non_epr_fields has a stream with each possible value for source_mode which should be useful for testing: https://github.com/hop-dev/kibana/blob/99f1d0799922f25068289984fc69109b27c034df/x-pack/test/fleet_api_integration/apis/fixtures/test_packages/non_epr_fields/1.0.0/manifest.yml

@kpollich
Copy link
Member Author

kpollich commented Dec 1, 2022

This also means we'll need to honor the value of this setting during package installation. The package manifest is the source of truth here. The UI toggles are just a convenience for maintainers.

@juliaElastic
Copy link
Contributor

I think both source_mode and index_mode settings can be applied in package install logic by calling a modified version of this function.

@ruflin
Copy link
Contributor

ruflin commented Dec 23, 2022

I'm reopening this issue as it seems there are still issues around using this feature. The issue should still open until the current issues are resolved: #147368 (comment)

@amitkanfer
Copy link

Moving to the current sprint (5)

@jlind23
Copy link
Contributor

jlind23 commented Jan 3, 2023

@nchaulet can you take a look at this one and see what is missing? If I understood Ruflin's comment correctly this can be closed.

@ruflin
Copy link
Contributor

ruflin commented Jan 3, 2023

@jlind23 As described in #147368 (comment) on my end it still does not work as expected.

@jlind23
Copy link
Contributor

jlind23 commented Jan 3, 2023

@ruflin my bad, misunderstood your comment then, @nchaulet will take a look at your comment shortly.

@nchaulet
Copy link
Member

nchaulet commented Jan 3, 2023

Yes there is still a bug, when the package policy package version, is not the same as the package installed version, I am working on fix, and will test this thoroughly.

@nchaulet
Copy link
Member

nchaulet commented Jan 4, 2023

@ruflin I tested it on my side and I think #148292 fixed this.

With elastic/integrations#4749 it give me this for system.memory datastream:
Screen Shot 2023-01-04 at 7 53 58 AM

@nchaulet nchaulet closed this as completed Jan 4, 2023
@ruflin
Copy link
Contributor

ruflin commented Jan 4, 2023

@nchaulet Is #148292 the fix for #141211 (comment) ? In the PR, it doesn't really mention any details around this and how it would be fixed. Happy to test it on my side but if the PR fixes the issue, the issue should be described and how it is fixed.

@nchaulet
Copy link
Member

nchaulet commented Jan 4, 2023

@nchaulet Is #148292 the fix for #141211 (comment) ? In the PR, it doesn't really mention any details around this and how it would be fixed. Happy to test it on my side but if the PR fixes the issue, the issue should be described and how it is fixed.

Yes #148292 fix it as a side effect, the UI was previously settings synthetic source to false if the package was defining index mode time_series, (updated the PR too)

@ruflin
Copy link
Contributor

ruflin commented Jan 9, 2023

I tested this on my end again. It seems the UI shows now the correct things, but when I look at the templates or the data stream (even after rollover), I can't find anywhere index.mode: time_series?

@kpollich
Copy link
Member Author

kpollich commented Jan 9, 2023

I stepped through the testing instructions on #148292 and it seems like the index template settings are working as expected when the synthetic source and TSDB flags come from the package manifest, but we have a regression with the actual toggle values.

I created a new Nginx integration policy on nginx-1.6.0, and enabled both synthetic source + tsdb via the experimental indexing toggles in the policy editor UI. No index template settings were created.

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Team:Fleet Team label for Observability Data Collection Fleet team
Projects
None yet
10 participants