Add 'logging' as new metric export type#47442
Add 'logging' as new metric export type#47442vpelikh wants to merge 2 commits intospring-projects:mainfrom
Conversation
Signed-off-by: Vasily Pelikh <2010720+vasilypelikh@users.noreply.github.com>
|
It might make sense to add auto-configuration for
might be easier to resolve using the |
danish-ali
left a comment
There was a problem hiding this comment.
You verify back-off when a Clock is absent, but not when LoggingMeterRegistry itself is missing.
Could we add an ApplicationContextRunner test with a FilteredClassLoader(LoggingMeterRegistry.class) to assert that neither LoggingMeterRegistry nor LoggingRegistryConfig is created? That would complement the @ConditionalOnClass(LoggingMeterRegistry.class) path.
Additionally, minor naming nit in LoggingMetricsExportPropertiesConfigAdapterTests: some methods refer to “AggregationTemporality” but assert logInactive(). Consider renaming to reflect logInactive to avoid confusion.
Good suggestion. But if I remember correctly, these endpoints only shows the latest values. In this case, one of the purposes of metrics - comparing values at different times during the application's execution - is lost.
Yep, and I think this behavior what you should expect from a metrics system - to show metric values once in a while. 😀
Honestly, I never had this problem. But surely new configuration options can be added in future.
You mean use logging registry instead of simple if other options are not suitable? |
|
Oh, thanks for the review, @danish-ali! I will check it out later when I will be around pc. |
Signed-off-by: Vasily Pelikh <2010720+vasilypelikh@users.noreply.github.com>
For gauges, yes. Counters are cumulative (shows the running total not the latest value) both for the metrics- and prometheus endpoints. I guess once you are in the realm of "comparing values at different times", you might also need a metrics backend so you might want to consider running Prometheus from docker-compose so that you can even keep execution results between application restarts.
People usually don't want to wait a minute just to see if a value was updated or not. That's why, for local development, using an on-demand output is usually more convenient. I'm not sure this is what you meant but
No, I meant adding opt-in auto-configuration for |
|
Guess I need to give more context. |
|
Thanks for the suggestion but we don't feel that we've seen sufficient need for this to warrant merging the change. There are good existing alternatives discussed above. For those that do want to use |
It's useful to see metrics in the console during development, without having to set up an LGTM container or write additional code.