Extra testing and some cleanups for filtering on field caps

[romseygeek]

• adds a test for mixed cluster requests
• fixes a bad stream version check (above test will fail if this isn't included)
• replaces private FieldCapsFilter interface with Predicate
• renames 'allowedTypes' to 'types' to maintain consistency with external API
• adds javadoc to ResponseRewriter
• removes isRuntimeField from FieldTypeLookup

Relates to #83636

[elasticsearchmachine]

Hi @romseygeek, I've created a changelog YAML for you.

[romseygeek]

So the test is failing because the rest client picks a random host to hit with its request, and we in fact need it to pick the upgraded host so that the new parameters are recognised by the co-ordinating node.

[javanna]

I left a few comments, thanks for working on this.

Do we still need to expand the serialization unit tests or was that covered in the meantime? Do you plan on opening another PR for the docs improvements?

[javanna]

you can plug in a NodeSelector to the RestClient to have it only point to nodes with specific characteristics. Version can be a factor.

[romseygeek]

Frustratingly that doesn't help here, because the RestClientBuilder doesn't know the version of its Nodes up-front, and so we can't get the version in the NodeSelector.

[javanna]

ok for my understanding, maybe the problem is that the existing client does not point to the upgraded nodes? I guess it depends on how it is initialized. The point of node selector is to influence node selection when making each request, but then all nodes need to be added to the client rotation. I see that the client gets the nodes from tests.rest.cluster set on build.gradle. Is there a chance that not all nodes are included?

[romseygeek]

All the nodes are included; the problem is that the RestClientBuilder.builder(HttpHost hosts...) that gets called here then builds its Nodes using plain Node::new, which doesn't set any Node metadata (including the version).

[javanna]

got it. Only the sniffer sets these automatically as it has knowledge about the metadata, but you can set metadata up yourself by using the builder(Node ... nodes) method. That makes this change not so useful and along the lines of what you already have though. Have a look if it makes sense to make the change or not?

[romseygeek]

Do we still need to expand the serialization unit tests or was that covered in the meantime? Do you plan on opening another PR for the docs improvements?

Serialization should be covered by the rolling upgrade IT test. I've expanded on the docs a little in the latest update.

[javanna]

Can we add setting and handling the additional fields in FieldCapabilitiesRequestTests as well as FieldCapabilitiesNodeRequestTests? LGTM besides that, no need for another round.

[romseygeek]

Thanks for the reviews @javanna!