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

Test LogsDB early with integrations #1842

Closed
ruflin opened this issue May 17, 2024 · 16 comments · Fixed by #1939
Closed

Test LogsDB early with integrations #1842

ruflin opened this issue May 17, 2024 · 16 comments · Fixed by #1939
Assignees

Comments

@ruflin
Copy link
Contributor

ruflin commented May 17, 2024

The Elasticsearch team is working on the logsdb mode for data streams which will provide an optimised way to store logs. To provide the Elasticsearch team with early feedback if this causes any issues with integrations, we should find a way to enable this on integrations at least for test runs. More details on logsdb here: elastic/elasticsearch#106489

Like TSDS, logsdb is an index mode setting on the data stream. How can we inject this setting for a test run on all integrations without having to change each integration? At first, enabling logsdb might reject documents so this goes well together with the testing with failure store enabled: #1832

An alternative way of approaching this is using Fleet / EPM to enable the flag for all logs integrations @kpollich . For example a feature flag in Kibana. Last, it could also be a feature flag in Elasticsearch that enables it. The best place to enable it would be logs@settings. This would not only enable it for all integrations but also for all data going into logs-- which seems ideal for broader testing.

Below a screenshot on all the templates that an integration imports.

Screenshot 2024-05-17 at 08 39 40

@salvatore-campagna Having written down the above option, to me the easiest way to test LogsDB seems to be having a feature flag / setting in Elasticsearch that changes logs@settings and adds the logsdb index mode. It is already possible today to start up elastic-package with specific settings for Elasticsearch so no modifications to elastic-package would be needed. In addition, it means we have a simple way to also test it in our test clusters.

Links

@ruflin
Copy link
Contributor Author

ruflin commented May 17, 2024

elastic/elasticsearch#108762 was just filed which should bring a feature flag to Elasticsearch to change the setting. This means, to test integrations, the only thing needed would be to pass in a config option on startup to Elasticsearch. @jsoriano Am I missing something?

@jsoriano
Copy link
Member

elastic/elasticsearch#108762 was just filed which should bring a feature flag to Elasticsearch to change the setting. This means, to test integrations, the only thing needed would be to pass in a config option on startup to Elasticsearch. @jsoriano Am I missing something?

Yes, we could add a user setting to enable this, and add it in CI.

We will also need to add logsdb to the list of possible index modes in the package-spec.

@jsoriano
Copy link
Member

We will also need to add logsdb to the list of possible index modes in the package-spec.

To be clear, I mean we would need this in the mid/long term for proper support in packages, but we won't need it to test early with integrations if the feature flag is added to ES.

@ruflin
Copy link
Contributor Author

ruflin commented May 17, 2024

We will also need to add logsdb to the list of possible index modes in the package-spec.

Good callout. This is especially for the scenario if an integration would want to disable it. Question is if we should allow it? 🤔 Users could always to it in the @custom template.

@flash1293
Copy link
Contributor

flash1293 commented May 17, 2024

Users could always to it in the @custom template

@salvatore-campagna just to make sure, will it be possible to change the index mode via component template? I vaguely remember that was mentioned in a meeting we had.

@jsoriano
Copy link
Member

We will also need to add logsdb to the list of possible index modes in the package-spec.

Good callout. This is especially for the scenario if an integration would want to disable it. Question is if we should allow it? 🤔 Users could always to it in the @custom template.

Well this would be useful if we migrate packages one by one as we did with tsdb. But if the plan is to make all logs datastreams to use this index mode at a given release, then we wouldn't need the flag.

@flash1293
Copy link
Contributor

As subobjects: false is coupled to the logs index mode which is a breaking change by my understanding, I think we either need to make it a one-by-one package thing or explicitly set subobjects: true if the data stream didn't opt into subobjects: false.

@kpollich
Copy link
Member

kpollich commented Jun 3, 2024

@jsoriano - I think if I understand correclt, once elastic/elasticsearch#108762 lands we can add a flag to start up ES with index.mode: logs set by default for all logs-* data streams and run system tests as normal to test integrations. I think this would satisfy the need for early testing/sanity checking. Do you agree with that?

Adding index.mode: logs as an option in package-spec + supporting it in Fleet would be the more long-term enhancement here that we could capture, separately.

@jsoriano
Copy link
Member

jsoriano commented Jun 3, 2024

Agree, we can support both things.

we can add a flag to start up ES with index.mode: logs set by default for all logs-* data streams and run system tests as normal to test integrations.

This should still be something that developers can enable locally. We would need to keep testing in CI with the default index mode for packages that don't have index.mode and users that don't use the feature flag.

@flash1293
Copy link
Contributor

@kpollich @jsoriano Seems like elastic/elasticsearch#108762 just landed in ES, so it should be possible to run this test now.

@ruflin
Copy link
Contributor Author

ruflin commented Jun 18, 2024

For reference, this is the PR with the change and description around the flag to be used: elastic/elasticsearch#109025

@ruflin
Copy link
Contributor Author

ruflin commented Jun 25, 2024

To start test logsdb on my local setup, I modified elastic-package in the following way: main...ruflin:elastic-package:logsdb-setting It adds cluster.logsdb.enabled: true to the Elasticsearch config. @jsoriano is there a way to pass this in as environment variable so no code change would be needed?

@jsoriano
Copy link
Member

@jsoriano is there a way to pass this in as environment variable so no code change would be needed?

No, we need to add this to the config embedded in elastic-package, but I think it would be fine to add a profile setting for this.

@ruflin
Copy link
Contributor Author

ruflin commented Jun 26, 2024

Can profile settings be set through an environment variable? The reason I'm asking is that I want to make it for users not familiar with elastic-package to have a single command I can share for them to get a stack running with logsdb.

@jsoriano
Copy link
Member

Can profile settings be set through an environment variable? The reason I'm asking is that I want to make it for users not familiar with elastic-package to have a single command I can share for them to get a stack running with logsdb.

They cannot be set through an environment variable at the moment, but they can be set with flags, so a single command is still possible. When added, it would be something like:

elastic-package stack up -d -U stack.logsdb_enabled=true

@ruflin
Copy link
Contributor Author

ruflin commented Jun 26, 2024

elastic-package stack up -d -U stack.logsdb_enabled=true This looks great!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants