Consistency between `setKnownMethods(Set<String>)`, `setCapturedRequestHeaders(List<String>)` and `setCapturedResponseHeaders(List<String>)`

[trask]

Part of #12846

There's a bit of inconsistency in the Http *TelemetryBuilder classes where setKnownMethods takes a Set while setCapturedRequestHeaders and setCapturedResponseHeaders take a List:

• setKnownMethods(Set<String>)
• setCapturedRequestHeaders(List<String>)
• setCapturedResponseHeaders(List<String>)

I think it makes sense to have consistence between these.

I kind of list List because it's generally a more common data type.

On the other hand, I kind of like Set because duplicates are meaningless in all of them.

An additional wrinkle is that we've already stabilized these:

• setCapturedRequestHeaders(List<String>) in HttpClientAttributesExtractorBuilder
• setCapturedResponseHeaders(List<String>) in HttpClientAttributesExtractorBuilder
• setKnownMethods(Set<String>) in HttpClientAttributesExtractorBuilder
• setKnownMethods(Set<String>) in HttpServerAttributesExtractorBuilder
• setKnownMethods(Set<String>) in HttpServerRouteBuilder
• setKnownMethods(Set<String>) in HttpSpanNameExtractorBuilder

Thoughts?

[jack-berg]

From the javadoc, the first sentence of each is:

• List: "An ordered collection."
• Set: "A collection that contains no duplicate elements."

In the case of all these methods, neither order nor the lack of duplicate elements is particularly important. Sure list is a common type (although I'd argue that set got more accessible / common in java 11 with Set.of()), but the implementation is doesn't care about accessing the elements in order. And sure it doesn't make any sense to include duplicate elements, but the implementation can just as easily convert the argument to a set for efficient .contains(T) lookups.

If I had to choose one, I'd lean towards set since the implementation is probably going to convert to a set. But if its really not vital, why not accept Collection instead? This sort of prioritizes user ergonomics, allowing them to use List.o(), Set.of() or whatever collection they prefer, at the expense of a little extra initialization expense for the implementation to convert to set internally.

As for the already stable methods, japicmp complains if you try to change the signature from setCpaturedRequestHeaders(List<String>) to setCapturedRequestHeaders(Collection<String>), claiming that its not binary compatible (I wonder if this is true? 🤔). But supposing changing to Collection is not a binary compatible change, can always deprecate and add a replacement method. Given that the existing stable methods are already inconsistent, it probably worth it to decide a path forward and fix the inconsistency.

[laurit]

@trask can this be closed?