elastic / rally-tracks

Track specifications for the Elasticsearch benchmarking tool Rally
19 stars 181 forks source link

Fix tsdb k8s queries for serverless #553

Closed gbanasiak closed 8 months ago

gbanasiak commented 9 months ago

https://github.com/elastic/rally-tracks/pull/493 unblocked tsdb8 k8s queries track for runs in serverless, but wasn't accompanied with serverless-specific ITs. https://github.com/elastic/rally-tracks/pull/513 had broken serverless support by accident by adding ILM configuration in index template section which is rendered when running as operator in serverless (ILM is not available in serverless).

This PR fixes that by removing ILM configuration from that section and adds the track to serverless ITs (which were added since) to prevent this from happening again.

Both templates (container-template.json and pod-template.json) follow the same structure for index settings:

{%- if build_flavor != "serverless" or serverless_operator == true -%}
### serverless settings restricted for operator
{%- endif -%}

{%- if build_flavor != "serverless" -%}
### settings unavailable in serverless
{%- endif -%}

### remaining settings
gizas commented 9 months ago

Thank you for this fix @gbanasiak ! Can we add the new parameters to readme here: https://github.com/elastic/rally-tracks/blob/de22ef18470d058453eca4bdf2fb7a5c7380c016/tsdb_k8s_queries/README.md#parameters

Also usually we set default values (like here), so how about setting them in your case as well?

To your questions:

  1. I agree feel free to comment your sections for if cases. It is really good timing for me to understand what parts of templates we actually need because we are trying to automate the extraction of the template
  2. For 2 I am not totally sure, as we actually merged two templates in one:
    • From metrics index template we have the message (see comment)
    • From metrics-elastic_agent.metricbeat we have the rest see below:

Screenshot 2024-01-18 at 3 04 42 PM

Shall we merge them? Does this affect final rendering times?

gbanasiak commented 9 months ago

Can we add the new parameters to readme here:

These are global variables documented in https://esrally.readthedocs.io/en/stable/serverless.html#tracks. We haven't added them to README track files anywhere, similarly to now global variable which existed prior to serverless.

I agree feel free to comment your sections for if cases.

Ok, I'll add something more descriptive.

For 2 I am not totally sure, as we actually merged two templates in one [..]

I see. With nominal integration setup ES would apply either metrics-elastic_agent.metricbeat or metrics template depending on the name of the datastream. If you need to combine them in Rally, message field should get added together with other fields in a single query.default_field setting. I'm not even sure what is the effect of specifying query.default_field twice.

gbanasiak commented 9 months ago

I've addressed comments in https://github.com/elastic/rally-tracks/pull/553/commits/583101e1dbf5f2e9aa57398392f3ebef41fea099.

gizas commented 8 months ago

@martijnvg this LGTM. As you are the main stakeholder of this track any other objection to merge? @gbanasiak I only left one comment to remove message