Expose version through prometheus endpoint #637
Conversation
| ObservabilityConfiguration(@Autowired BuildProperties buildProperties) { | ||
|
|
||
| Gauge.builder(METRIC_INFO_GAUGE_NAME, 1.0, n -> n) | ||
| .strongReference(true) |
There was a problem hiding this comment.
Is this strong reference required?
| @AfterEach | ||
| void afterEach() { | ||
| Metrics.globalRegistry.clear(); | ||
| } | ||
|
|
||
| @BeforeEach | ||
| void init() { | ||
| MockitoAnnotations.openMocks(this); | ||
| } |
There was a problem hiding this comment.
Can we reorder these methods? I would use the following order:
@BeforeAll
@BeforeEach
@AfterEach
@AfterAll| Assertions.assertThat(info).isNotNull(); | ||
| Assertions.assertThat(info.getId()).isNotNull(); | ||
| Assertions.assertThat(info.getId().getTag(ObservabilityConfiguration.METRIC_INFO_LABEL_APP_NAME)).isEqualTo(name); | ||
| Assertions.assertThat(info.getId().getTag(ObservabilityConfiguration.METRIC_INFO_LABEL_APP_VERSION)).isEqualTo(version); |
There was a problem hiding this comment.
as one of the great president said, yes we can
There was a problem hiding this comment.
Do we really need to mock things here ?
Why not simply assert we have BuildProperties with correct values ?
There was a problem hiding this comment.
I succeeded in working without the mock. I had to add 2 annotations to the class.
@ExtendWith(SpringExtension.class)
@Import(ProjectInfoAutoConfiguration.class)
class ObservabilityConfigurationTest
There was a problem hiding this comment.
@mcornaton
to deal with nullpointer, i propose
assertThat(info).isNotNull();
assertAll(
() -> assertThat(observabilityConfiguration).isNotNull(),
() -> assertThat(info.getId()).isNotNull(),
() -> assertThat(info.getId().getTag(ObservabilityConfiguration.METRIC_INFO_LABEL_APP_NAME)).isEqualTo(buildProperties.getName()),
() -> assertThat(info.getId().getTag(ObservabilityConfiguration.METRIC_INFO_LABEL_APP_VERSION)).isEqualTo(buildProperties.getVersion())
);
| import org.springframework.context.annotation.Configuration; | ||
|
|
||
| @Configuration | ||
| public class ObservabilityConfiguration { |
There was a problem hiding this comment.
Global question here.
Would it be meaningful to merge ObservabilityConfiguration + VersionController + VersionService in a single class exposing both metric registry and rest endpoint ?
There was a problem hiding this comment.
If you want, I can:
Add a getAppName in VersionService and add a postConstruct in the Controller to create the metric (by calling VersionService).
OR
Keep only the controller (delete the service which is a bit too much here)
But I liked the idea of managing this metric independently in a Config class because it can be replicated in all projects (we don't necessarily have the controller everywhere).
There was a problem hiding this comment.
The idea for me is to have the controller everywhere.
It will be more work but every app should have the version controller and the version metric endpoint in my opinion.
There was a problem hiding this comment.
Do we want to try to update springdoc-openapi dependency to the highest possible version (1.7.0 as we still use Spring Boot 2.x https://springdoc.org/) ?
|
Kudos, SonarCloud Quality Gate passed! |
thewhitewizard
deleted the
feature/expose-version-through-prometheus-endpoint
branch
No description provided.