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

MultiGauge.register should accept more types #5390

Closed
lbilger opened this issue Aug 15, 2024 · 0 comments · Fixed by #5391
Closed

MultiGauge.register should accept more types #5390

lbilger opened this issue Aug 15, 2024 · 0 comments · Fixed by #5391
Labels
enhancement A general enhancement module: micrometer-core An issue that is related to our core module
Milestone

Comments

@lbilger
Copy link
Contributor

lbilger commented Aug 15, 2024

Describe the bug
The typical way to use a MultiGauge is converting some list to a list of Rows using a Stream. This is also done in theMultiGaugeSample example. Java's type inference is tricky here. In the example, it works because Java knows what is expected by MultiGauge.register and infers List<Row<?>>. But if you separate the conversion and assign the list to a variable, the inferred type is List<Row<Number>>. The same is true for some reason if you use the newer Stream.toList() instead of Stream.collect(Collectors.toList()).

So

            temperatures.register(
                fetchTemperatures().stream()
                                   .map(record -> Row.of(Tags.of("room", record.getRoom().name()), record.getTemperature()))
                                   .collect(Collectors.toList()));

will compile but

            temperatures.register(
                fetchTemperatures().stream()
                                   .map(record -> Row.of(Tags.of("room", record.getRoom().name()), record.getTemperature()))
                                   .toList());

and

            var rows = fetchTemperatures().stream()
                    .map(record -> Row.of(Tags.of("room", record.getRoom().name()), record.getTemperature()))
                    .collect(Collectors.toList());
            temperatures.register(rows);

will not.

I wonder if the type parameter of Row adds any value at all. But removing it would probably break existing code, so I suggest to change the signature of MultiGauge.register to accept an Iterable<? extends Row<?>> instead. Then it could also be called with a List<Row<Number>>.

I will create a PR for this small change.

Environment

  • Micrometer version [e.g. 1.7.1]: 1.13.2, also on the main branch at the time of writing
  • Micrometer registry [e.g. prometheus]: prometheus, but not relevant
  • OS: [e.g. macOS] macOS, but not relevant
  • Java version: [e.g. output of java -version]
openjdk version "21.0.2" 2024-01-16
OpenJDK Runtime Environment GraalVM CE 21.0.2+13.1 (build 21.0.2+13-jvmci-23.1-b30)
OpenJDK 64-Bit Server VM GraalVM CE 21.0.2+13.1 (build 21.0.2+13-jvmci-23.1-b30, mixed mode, sharing)

To Reproduce
See description

Expected behavior
List<Row<Number>> is accepted and no ugly casts are necessary.

@shakuzen shakuzen changed the title The signature of MultiGauge.register should be Iterable<? extends Row<?>> MultiGauge.register should accept more types Aug 28, 2024
@shakuzen shakuzen added enhancement A general enhancement module: micrometer-core An issue that is related to our core module and removed waiting-for-triage labels Aug 28, 2024
@shakuzen shakuzen added this to the 1.14.0-M3 milestone Aug 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement A general enhancement module: micrometer-core An issue that is related to our core module
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants