Iterable<Tag> vs Iterable<? extends Tag>

[Stephan202]

I noticed that Micrometer methods sometimes accept Iterable<? extends Tag>:

$ git grep 'Iterable<? extends Tag>'
micrometer-core/src/main/java/io/micrometer/core/instrument/Tags.java:    public Tags and(@Nullable Iterable<? extends Tag> tags) {
micrometer-core/src/main/java/io/micrometer/core/instrument/Tags.java:    public static Tags concat(@Nullable Iterable<? extends Tag> tags, @Nullable Iterable<Tag> otherTags) {
micrometer-core/src/main/java/io/micrometer/core/instrument/Tags.java:    public static Tags concat(@Nullable Iterable<? extends Tag> tags, @Nullable String... keyValues) {
micrometer-core/src/main/java/io/micrometer/core/instrument/Tags.java:    public static Tags of(@Nullable Iterable<? extends Tag> tags) {

... but mostly require Iterable<Tag>:

$ git grep -l 'Iterable<Tag>' | wc -l
88

Two questions:

• For consistency, shouldn't the second argument to Tags.concat also accept Iterable<? extends Tag>?
• Since Tag is an interface and even has a public subtype provided by Micrometer itself (ImmutableTag), shouldn't most/all other methods also accept an Iterable<? extends Tag>?

(I'm raising this question mostly because it came up while writing a Refaster template. I can understand if the suggested changes aren't worth potentially breaking client code, even though I expect that the suggested changes are mostly safe. If there's interest in this change I'm willing to prepare a PR.)

[jkschneider]

@Stephan202 As far as I can tell, this second argument of concat is the only place where we aren't accepting Iterable<? extends Tag> and it does seem like an oversight. I'm honestly not sure what the ramifications are on binary compatibility, but this could be tested.

[jonatan-ivanov]

Because of type erasure, this should have no effect on the byte code. Also, since <? extends T> is covariant in T, changing the type from Iterable<Tag> to Iterable<? extends Tag> broadens the scope so it does not break anything. Doing the opposite would be a breaking change since it would narrow the scope.

Also, otherTags are passed to and that already expects Iterable<? extends Tag>:

micrometer/micrometer-core/src/main/java/io/micrometer/core/instrument/Tags.java

Lines 194 to 196 in 73747e2

	public static Tags concat(@Nullable Iterable<? extends Tag> tags, @Nullable Iterable<Tag> otherTags) {
	return Tags.of(tags).and(otherTags);
	}

[jkschneider]

@jonatan-ivanov Be careful not to confuse source compatibility with binary compatibility. Whether it is binary compatible isn't immediately obvious (to me anyway), would have to test this. We've run afoul of this in the past. For example, changing an interface to an abstract class is source compatible but binary incompatible.