Support Mp Metrics 5.1 MP Config properties for Histogram and Timer metrics #636
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
fmhwong
reviewed
Aug 18, 2023
implementation/src/main/java/io/smallrye/metrics/legacyapi/HistogramAdapter.java
Outdated
Show resolved
Hide resolved
fmhwong
reviewed
Aug 18, 2023
implementation/src/main/java/io/smallrye/metrics/setup/config/PropertyBooleanConfiguration.java
Outdated
Show resolved
Hide resolved
fmhwong
reviewed
Aug 18, 2023
implementation/src/main/java/io/smallrye/metrics/legacyapi/HistogramAdapter.java
Outdated
Show resolved
Hide resolved
fmhwong
reviewed
Aug 18, 2023
implementation/src/main/java/io/smallrye/metrics/legacyapi/HistogramAdapter.java
Outdated
Show resolved
Hide resolved
fmhwong
reviewed
Aug 18, 2023
implementation/src/main/java/io/smallrye/metrics/legacyapi/TimerAdapter.java
Outdated
Show resolved
Hide resolved
fmhwong
reviewed
Aug 18, 2023
implementation/src/main/java/io/smallrye/metrics/setup/config/HistogramBucketConfiguration.java
Outdated
Show resolved
Hide resolved
fmhwong
reviewed
Aug 18, 2023
| // .filter(s -> s.matches("[0-9]+[.]*[0-9]*")) | ||
| // .map(Double::parseDouble) | ||
| .map(s -> { | ||
| if (s.matches("[0-9]+[.]*[0-9]*")) { |
There was a problem hiding this comment.
Do you intend to match a decimal point? Should the dot be escaped?
There was a problem hiding this comment.
the [ ] character set allows for matching explicitly the characters within it.
fmhwong
reviewed
Aug 18, 2023
...entation/src/main/java/io/smallrye/metrics/setup/config/HistogramBucketMaxConfiguration.java
Outdated
Show resolved
Hide resolved
fmhwong
reviewed
Aug 18, 2023
|
|
||
| // metricGroup=<blank> == invalid | ||
| if (keyValueSplit.length == 2) { | ||
| if (keyValueSplit[1].matches(("[0-9]+[.]*[0-9]*"))) { |
There was a problem hiding this comment.
the [ ] character set allows for matching explicitly the characters within it.
fmhwong
reviewed
Aug 18, 2023
...entation/src/main/java/io/smallrye/metrics/setup/config/HistogramBucketMinConfiguration.java
Outdated
Show resolved
Hide resolved
fmhwong
reviewed
Aug 18, 2023
|
|
||
| // metricGroup=<blank> == invalid | ||
| if (keyValueSplit.length == 2) { | ||
| if (keyValueSplit[1].matches(("[0-9]+[.]*[0-9]*"))) { |
There was a problem hiding this comment.
the [ ] character set allows for matching explicitly the characters within it.
fmhwong
reviewed
Aug 18, 2023
| // Parse values of percentile - ensure value is 0 <= x <= 1.0 | ||
| Double[] percentileValues = Arrays.asList(keyValueSplit[1].split(",")).stream().map(s -> { | ||
|
|
||
| if (s.matches("[0][.][0-9]+")) { |
There was a problem hiding this comment.
the [ ] character set allows for matching explicitly the characters within it.
fmhwong
reviewed
Aug 18, 2023
...ementation/src/main/java/io/smallrye/metrics/setup/config/MetricPercentileConfiguration.java
Outdated
Show resolved
Hide resolved
fmhwong
reviewed
Aug 18, 2023
implementation/src/main/java/io/smallrye/metrics/setup/config/MetricsConfigurationManager.java
Outdated
Show resolved
Hide resolved
fmhwong
reviewed
Aug 18, 2023
implementation/src/main/java/io/smallrye/metrics/setup/config/MetricsConfigurationManager.java
Outdated
Show resolved
Hide resolved
fmhwong
reviewed
Aug 18, 2023
implementation/src/main/java/io/smallrye/metrics/setup/config/MetricsConfigurationManager.java
Outdated
Show resolved
Hide resolved
fmhwong
reviewed
Aug 18, 2023
implementation/src/main/java/io/smallrye/metrics/setup/config/MetricsConfigurationManager.java
Outdated
Show resolved
Hide resolved
fmhwong
reviewed
Aug 18, 2023
implementation/src/main/java/io/smallrye/metrics/setup/config/MetricsConfigurationManager.java
Outdated
Show resolved
Hide resolved
fmhwong
reviewed
Aug 18, 2023
implementation/src/main/java/io/smallrye/metrics/setup/config/MetricsConfigurationManager.java
Outdated
Show resolved
Hide resolved
fmhwong
reviewed
Aug 18, 2023
implementation/src/main/java/io/smallrye/metrics/setup/config/MetricsConfigurationManager.java
Show resolved
Hide resolved
fmhwong
reviewed
Aug 18, 2023
implementation/src/main/java/io/smallrye/metrics/setup/config/MetricsConfigurationManager.java
Outdated
Show resolved
Hide resolved
Channyboy
force-pushed
the
micrometer-histoTimer-properties
branch
from
3760eb1
to
607670c
Compare
fmhwong
approved these changes
Aug 18, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixe #665
Context the new MP Config properties for metrics v5.1 follow the following syntax:
^^ That is to say property is a semi-colon separated values that define the configuration for a specific metric name.
MericsConfigurationMangerhasgetxxx()which returns an respective sub-class ofPropertyConfiguration.For example
getPercentilesConfiguration(...)returnsMetricPercentileConfiguration.This is called as appropriate from the
HistogramAdapterorTimerAdapterduring registration as it needs this info to customize the metric.During the
getxxx(), it checks ifMericsConfigurationMangercontains an existing collection of configuration objects for the app name that is passed in in its respectiveMap<String, Collection<T extends PropertyConfiguration>>. The collection represents the configuration of the whole property (i.e. all metric names and its associative values). The *appname that is passed in is used to parse this collection for the specific metric name and value. This is done by calling a static methodmatches(Collection<T extends PropertyConfiguration> collection, String metricName)which accepts a metric name and the collection to go through.If not, it will retrieve the MP Config property from the ConfigProvider and add to the respective
Map<String, Collection<T extends PropertyConfiguration>>.