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

[Metrics store] Add support for custom templates #1854

Closed

Conversation

inqueue
Copy link
Member

@inqueue inqueue commented May 30, 2024

This change adds the ability to customize indices in the metrics store. The goal is to support ILM policies and eventually shift to using data streams.

  • Support the addition of component templates having the @custom suffix.
  • Add documentation.

@inqueue inqueue added the enhancement Improves the status quo label May 30, 2024
@inqueue inqueue requested a review from a team May 30, 2024 23:21
@inqueue inqueue self-assigned this May 30, 2024
* ``rally-races-*``: Contains the metadata of a benchmark. This is a subset of the metrics records but only contains the metadata of a benchmark.
* ``rally-metrics-*``: Contains all metrics records.
* ``rally-results-*``: Contains the results of a benchmark. This is a subset of the metrics records but only contains the final results of a benchmark.
* ``rally-annotations-*``: Contains annotations that can be added to a benchmark. This is a subset of the metrics records but only contains the annotations of a benchmark.
Copy link
Member

Choose a reason for hiding this comment

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

The rally-annotations is a single alias of a single backing index. (ie rally-annotations -> reindexed-v7-rally-annotations ) I think we can remove the comment about the subset, as the documents itself just refer to chart names, rather than metrics themselves

@gbanasiak
Copy link
Contributor

TypeError: put_index_template() got an unexpected keyword argument 'ignore_missing_component_templates'

We would need to modify assumption on metric store ES version to support this approach. We're still maintaining compatibility with 6.8 for target cluster which I think is something we could drop, but 7.17.x must stay for now. Unfortunately ignore_missing_component_templates was added in 8.7. Do we want to bifurcate and have different requirements for metric store and target cluster?

@inqueue
Copy link
Member Author

inqueue commented May 31, 2024

TypeError: put_index_template() got an unexpected keyword argument 'ignore_missing_component_templates'

We would need to modify assumption on metric store ES version to support this approach. We're still maintaining compatibility with 6.8 for target cluster which I think is something we could drop, but 7.17.x must stay for now. Unfortunately ignore_missing_component_templates was added in 8.7. Do we want to bifurcate and have different requirements for metric store and target cluster?

Thanks, Grzegorz. I hadn't realized ignore_missing_component_templates was so new. The parameter is not even supported in elasticsearch-py until 8.14 via elastic/elasticsearch-py#2525 (Rally uses 8.6.1 currently).

Rally would need to load an empty *@custom component template to support this, if one did was not already in the metrics cluster:

{
  "template": {}
}

The ultimate goal was to slowly move toward using data streams, or at least provide the option for it, to allow for better management of metrics retention, and pushing metrics to serverless. We are not there yet on any of these fronts, and therefore, closing this PR.

@inqueue inqueue closed this May 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improves the status quo
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants