diff --git a/instrumentation/jmx-metrics/javaagent/README.md b/instrumentation/jmx-metrics/javaagent/README.md index 8272e893e327..b91ce05a7b79 100644 --- a/instrumentation/jmx-metrics/javaagent/README.md +++ b/instrumentation/jmx-metrics/javaagent/README.md @@ -307,6 +307,40 @@ rules: For now, only the `lowercase` transformation is supported, other additions might be added in the future if needed. +### Unit conversions + +Sometimes JMX attribute values are reported in units that are not aligned with semantic conventions. +For example, duration values are usually reported as milliseconds while semantic conventions recommend using seconds. + +This issue can be solved by providing an optional `sourceUnit` metric property together with the `unit` metric property. +`sourceUnit` defines the native unit of value retrieved from JMX attribute, while `unit` defines the unit of the metric reported to the backend. +If a conversion between `sourceUnit` and `unit` is available, then it is automatically applied before reporting the metric. +If such a conversion is not available, then an error is reported during JMX metrics processing. + +Currently available unit conversions: + +| `sourceUnit` | `unit` | +|--------------|-------| +| ms | s | +| us | s | +| ns | s | + +Example of defining unit conversion in yaml file: +```yaml +rules: + - beans: + - Catalina:type=GlobalRequestProcessor,name=* + prefix: http.server.tomcat. + mapping: + maxTime: + metric: maxTime + type: gauge + sourceUnit: ms + unit: s + desc: The longest request processing time +``` +`sourceUnit` can also be defined on rule level (see [Making shortcuts](#making-shortcuts)) + ### General Syntax Here is the general description of the accepted configuration file syntax. The whole contents of the file is case-sensitive, with exception for `type` as described in the table below. diff --git a/instrumentation/jmx-metrics/library/src/main/java/io/opentelemetry/instrumentation/jmx/engine/MetricDef.java b/instrumentation/jmx-metrics/library/src/main/java/io/opentelemetry/instrumentation/jmx/engine/MetricDef.java index 3feb99e4741a..c2a42ae49194 100644 --- a/instrumentation/jmx-metrics/library/src/main/java/io/opentelemetry/instrumentation/jmx/engine/MetricDef.java +++ b/instrumentation/jmx-metrics/library/src/main/java/io/opentelemetry/instrumentation/jmx/engine/MetricDef.java @@ -42,12 +42,14 @@ // new MetricInfo( // "my.own.jvm.memory.pool.used", // "Pool memory currently used", +// null, // "By", // MetricInfo.Type.UPDOWNCOUNTER); // MetricInfo poolLimitInfo = // new MetricInfo( // "my.own.jvm.memory.pool.limit", // "Maximum obtainable memory pool size", +// null, // "By", // MetricInfo.Type.UPDOWNCOUNTER); // diff --git a/instrumentation/jmx-metrics/library/src/main/java/io/opentelemetry/instrumentation/jmx/engine/MetricInfo.java b/instrumentation/jmx-metrics/library/src/main/java/io/opentelemetry/instrumentation/jmx/engine/MetricInfo.java index 9cb35ae53b15..d357ea0bb882 100644 --- a/instrumentation/jmx-metrics/library/src/main/java/io/opentelemetry/instrumentation/jmx/engine/MetricInfo.java +++ b/instrumentation/jmx-metrics/library/src/main/java/io/opentelemetry/instrumentation/jmx/engine/MetricInfo.java @@ -27,7 +27,8 @@ public enum Type { // How to report the metric using OpenTelemetry API private final String metricName; // used as Instrument name @Nullable private final String description; - @Nullable private final String unit; + @Nullable private final String sourceUnit; + private final String unit; private final Type type; /** @@ -35,13 +36,19 @@ public enum Type { * * @param metricName a String that will be used as a metric name, it should be unique * @param description a human readable description of the metric + * @param sourceUnit a human readable unit of measurement that is received from metric source * @param unit a human readable unit of measurement * @param type the instrument typ to be used for the metric */ public MetricInfo( - String metricName, @Nullable String description, String unit, @Nullable Type type) { + String metricName, + @Nullable String description, + @Nullable String sourceUnit, + String unit, + @Nullable Type type) { this.metricName = metricName; this.description = description; + this.sourceUnit = sourceUnit; this.unit = unit; this.type = type == null ? Type.GAUGE : type; } @@ -56,6 +63,10 @@ public String getDescription() { } @Nullable + public String getSourceUnit() { + return sourceUnit; + } + public String getUnit() { return unit; } diff --git a/instrumentation/jmx-metrics/library/src/main/java/io/opentelemetry/instrumentation/jmx/engine/MetricRegistrar.java b/instrumentation/jmx-metrics/library/src/main/java/io/opentelemetry/instrumentation/jmx/engine/MetricRegistrar.java index 482b1e99f3b7..ce500a3c6a20 100644 --- a/instrumentation/jmx-metrics/library/src/main/java/io/opentelemetry/instrumentation/jmx/engine/MetricRegistrar.java +++ b/instrumentation/jmx-metrics/library/src/main/java/io/opentelemetry/instrumentation/jmx/engine/MetricRegistrar.java @@ -20,6 +20,7 @@ import java.util.Optional; import java.util.function.Consumer; import java.util.logging.Logger; +import javax.annotation.Nullable; import javax.management.MBeanServerConnection; import javax.management.ObjectName; @@ -61,6 +62,7 @@ void enrollExtractor( return; } + boolean recordDoubleValue = attributeInfo.usesDoubleValues(); MetricInfo metricInfo = extractor.getInfo(); String metricName = metricInfo.getMetricName(); MetricInfo.Type instrumentType = metricInfo.getType(); @@ -69,6 +71,12 @@ void enrollExtractor( ? metricInfo.getDescription() : attributeInfo.getDescription(); String unit = metricInfo.getUnit(); + String sourceUnit = metricInfo.getSourceUnit(); + + UnitConverter unitConverter = UnitConverter.getInstance(sourceUnit, unit); + if (unitConverter != null) { + recordDoubleValue = true; + } switch (instrumentType) { // CHECKSTYLE:OFF @@ -77,10 +85,10 @@ void enrollExtractor( // CHECKSTYLE:ON LongCounterBuilder builder = meter.counterBuilder(metricName); Optional.ofNullable(description).ifPresent(builder::setDescription); - Optional.ofNullable(unit).ifPresent(builder::setUnit); + builder.setUnit(unit); - if (attributeInfo.usesDoubleValues()) { - builder.ofDoubles().buildWithCallback(doubleTypeCallback(extractor)); + if (recordDoubleValue) { + builder.ofDoubles().buildWithCallback(doubleTypeCallback(extractor, unitConverter)); } else { builder.buildWithCallback(longTypeCallback(extractor)); } @@ -94,10 +102,10 @@ void enrollExtractor( // CHECKSTYLE:ON LongUpDownCounterBuilder builder = meter.upDownCounterBuilder(metricName); Optional.ofNullable(description).ifPresent(builder::setDescription); - Optional.ofNullable(unit).ifPresent(builder::setUnit); + builder.setUnit(unit); - if (attributeInfo.usesDoubleValues()) { - builder.ofDoubles().buildWithCallback(doubleTypeCallback(extractor)); + if (recordDoubleValue) { + builder.ofDoubles().buildWithCallback(doubleTypeCallback(extractor, unitConverter)); } else { builder.buildWithCallback(longTypeCallback(extractor)); } @@ -111,10 +119,10 @@ void enrollExtractor( // CHECKSTYLE:ON DoubleGaugeBuilder builder = meter.gaugeBuilder(metricName); Optional.ofNullable(description).ifPresent(builder::setDescription); - Optional.ofNullable(unit).ifPresent(builder::setUnit); + builder.setUnit(unit); - if (attributeInfo.usesDoubleValues()) { - builder.buildWithCallback(doubleTypeCallback(extractor)); + if (recordDoubleValue) { + builder.buildWithCallback(doubleTypeCallback(extractor, unitConverter)); } else { builder.ofLongs().buildWithCallback(longTypeCallback(extractor)); } @@ -133,8 +141,10 @@ void enrollExtractor( /* * A method generating metric collection callback for asynchronous Measurement * of Double type. + * If unit converter is provided then conversion is applied before metric is recorded. */ - static Consumer doubleTypeCallback(MetricExtractor extractor) { + static Consumer doubleTypeCallback( + MetricExtractor extractor, @Nullable UnitConverter unitConverter) { return measurement -> { DetectionStatus status = extractor.getStatus(); if (status != null) { @@ -145,6 +155,10 @@ static Consumer doubleTypeCallback(MetricExtractor if (metricValue != null) { // get the metric attributes Attributes attr = createMetricAttributes(connection, objectName, extractor); + + if (unitConverter != null) { + metricValue = unitConverter.convert(metricValue); + } measurement.record(metricValue.doubleValue(), attr); } } diff --git a/instrumentation/jmx-metrics/library/src/main/java/io/opentelemetry/instrumentation/jmx/engine/UnitConverter.java b/instrumentation/jmx-metrics/library/src/main/java/io/opentelemetry/instrumentation/jmx/engine/UnitConverter.java new file mode 100644 index 000000000000..b7627645ed5a --- /dev/null +++ b/instrumentation/jmx-metrics/library/src/main/java/io/opentelemetry/instrumentation/jmx/engine/UnitConverter.java @@ -0,0 +1,107 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.instrumentation.jmx.engine; + +import java.util.HashMap; +import java.util.Map; +import java.util.concurrent.TimeUnit; +import java.util.function.Function; +import javax.annotation.Nullable; + +/** + * This class is responsible for converting a value between metric units using defined conversion + * algorithms. + */ +class UnitConverter { + private static final Map conversionMappings = new HashMap<>(); + + static { + registerConversion("ms", "s", value -> value.doubleValue() / TimeUnit.SECONDS.toMillis(1)); + registerConversion("us", "s", value -> value.doubleValue() / TimeUnit.SECONDS.toMicros(1)); + registerConversion("ns", "s", value -> value.doubleValue() / TimeUnit.SECONDS.toNanos(1)); + } + + private final Function convertingFunction; + + /** + * Get an instance of converter that is able to convert a value from a given source to a target + * unit. + * + * @param sourceUnit a source unit supported by requested converter + * @param targetUnit a target unit supported by requested converter + * @return an instance of converter, or {@literal null} if {@code sourceUnit} is {@literal null} + * or empty, which means that there is no conversion needed. + * @throws IllegalArgumentException if {@code targetUnit} is empty, or matching converter was not + * found for provided units. + */ + @Nullable + public static UnitConverter getInstance(@Nullable String sourceUnit, String targetUnit) { + if (targetUnit.isEmpty()) { + throw new IllegalArgumentException("Non empty targetUnit must be provided"); + } + + if (sourceUnit == null || sourceUnit.isEmpty()) { + // No conversion is needed + return null; + } + + String converterKey = buildConverterKey(sourceUnit, targetUnit); + UnitConverter converter = conversionMappings.get(converterKey); + if (converter == null) { + throw new IllegalArgumentException( + "Unsupported conversion from [" + sourceUnit + "] to [" + targetUnit + "]"); + } + + return converter; + } + + /** + * Register new converter instance that can then be retrieved with {@link #getInstance(String, + * String)}. + * + * @param sourceUnit a source unit supported by the converter + * @param targetUnit a target unit supported by the converter + * @param convertingFunction a function that implements algorithm of conversion between {@code + * sourceUnit} and {@code targetUnit} + * @throws IllegalArgumentException if source or target unit is empty, or when there is converter + * already registered for given {@code sourceUnit} and {@code targetUnit} + */ + // visible for testing + static void registerConversion( + String sourceUnit, String targetUnit, Function convertingFunction) { + if (sourceUnit.isEmpty()) { + throw new IllegalArgumentException("Non empty sourceUnit must be provided"); + } + if (targetUnit.isEmpty()) { + throw new IllegalArgumentException("Non empty targetUnit must be provided"); + } + + String converterKey = buildConverterKey(sourceUnit, targetUnit); + + if (conversionMappings.containsKey(converterKey)) { + throw new IllegalArgumentException( + "Conversion from [" + sourceUnit + "] to [" + targetUnit + "] already defined"); + } + conversionMappings.put(converterKey, new UnitConverter(convertingFunction)); + } + + private static String buildConverterKey(String sourceUnit, String targetUnit) { + return sourceUnit + "->" + targetUnit; + } + + /** + * Create an instance of converter + * + * @param convertingFunction an algorithm applied when converting value + */ + UnitConverter(Function convertingFunction) { + this.convertingFunction = convertingFunction; + } + + public Number convert(Number value) { + return convertingFunction.apply(value); + } +} diff --git a/instrumentation/jmx-metrics/library/src/main/java/io/opentelemetry/instrumentation/jmx/yaml/JmxRule.java b/instrumentation/jmx-metrics/library/src/main/java/io/opentelemetry/instrumentation/jmx/yaml/JmxRule.java index ab09fe3f18f9..2bb0c4b631e3 100644 --- a/instrumentation/jmx-metrics/library/src/main/java/io/opentelemetry/instrumentation/jmx/yaml/JmxRule.java +++ b/instrumentation/jmx-metrics/library/src/main/java/io/opentelemetry/instrumentation/jmx/yaml/JmxRule.java @@ -47,6 +47,7 @@ public class JmxRule extends MetricStructure { @Nullable private String prefix; private Map mapping; + @Nullable public String getBean() { return bean; } @@ -90,6 +91,7 @@ private String validatePrefix(String prefix) { return prefix; } + @Nullable public String getPrefix() { return prefix; } @@ -152,10 +154,13 @@ public MetricDef buildMetricDef() throws Exception { new MetricInfo( prefix == null ? niceAttributeName : (prefix + niceAttributeName), null, + getSourceUnit(), getUnit(), getMetricType()); } else { - metricInfo = m.buildMetricInfo(prefix, niceAttributeName, getUnit(), getMetricType()); + metricInfo = + m.buildMetricInfo( + prefix, niceAttributeName, getSourceUnit(), getUnit(), getMetricType()); } List ownAttributes = getAttributeList(); @@ -226,6 +231,7 @@ protected Number extractNumericalAttribute( new MetricInfo( metricInfo.getMetricName(), metricInfo.getDescription(), + metricInfo.getSourceUnit(), metricInfo.getUnit(), MetricInfo.Type.UPDOWNCOUNTER); diff --git a/instrumentation/jmx-metrics/library/src/main/java/io/opentelemetry/instrumentation/jmx/yaml/Metric.java b/instrumentation/jmx-metrics/library/src/main/java/io/opentelemetry/instrumentation/jmx/yaml/Metric.java index 77c313fb28d4..4ed1c66341be 100644 --- a/instrumentation/jmx-metrics/library/src/main/java/io/opentelemetry/instrumentation/jmx/yaml/Metric.java +++ b/instrumentation/jmx-metrics/library/src/main/java/io/opentelemetry/instrumentation/jmx/yaml/Metric.java @@ -21,6 +21,7 @@ public class Metric extends MetricStructure { @Nullable private String metric; @Nullable private String desc; + @Nullable public String getMetric() { return metric; } @@ -35,6 +36,7 @@ private String validateMetricName(String name) { return name; } + @Nullable public String getDesc() { return desc; } @@ -47,6 +49,7 @@ public void setDesc(String desc) { MetricInfo buildMetricInfo( @Nullable String prefix, String attributeName, + String defaultSourceUnit, String defaultUnit, MetricInfo.Type defaultType) { String metricName; @@ -61,11 +64,16 @@ MetricInfo buildMetricInfo( metricType = defaultType; } + String sourceUnit = getSourceUnit(); + if (sourceUnit == null) { + sourceUnit = defaultSourceUnit; + } + String unit = getUnit(); if (unit == null) { unit = defaultUnit; } - return new MetricInfo(metricName, desc, unit, metricType); + return new MetricInfo(metricName, desc, sourceUnit, unit, metricType); } } diff --git a/instrumentation/jmx-metrics/library/src/main/java/io/opentelemetry/instrumentation/jmx/yaml/MetricStructure.java b/instrumentation/jmx-metrics/library/src/main/java/io/opentelemetry/instrumentation/jmx/yaml/MetricStructure.java index 4f444cd05cf5..980a78545f87 100644 --- a/instrumentation/jmx-metrics/library/src/main/java/io/opentelemetry/instrumentation/jmx/yaml/MetricStructure.java +++ b/instrumentation/jmx-metrics/library/src/main/java/io/opentelemetry/instrumentation/jmx/yaml/MetricStructure.java @@ -19,6 +19,7 @@ *
  • the metric type *
  • the metric attributes *
  • the unit + *
  • the sourceUnit * *

    Known subclasses are {@link JmxRule} and {@link Metric}. */ @@ -45,6 +46,7 @@ abstract class MetricStructure { private Map metricAttribute; private StateMapping stateMapping = StateMapping.empty(); private static final String STATE_MAPPING_DEFAULT = "*"; + private String sourceUnit; private String unit; private MetricInfo.Type metricType; @@ -58,6 +60,14 @@ void setType(String t) { this.metricType = MetricInfo.Type.valueOf(t); } + public String getSourceUnit() { + return sourceUnit; + } + + public void setSourceUnit(String unit) { + this.sourceUnit = validateUnit(unit.trim()); + } + public String getUnit() { return unit; } diff --git a/instrumentation/jmx-metrics/library/src/main/java/io/opentelemetry/instrumentation/jmx/yaml/RuleParser.java b/instrumentation/jmx-metrics/library/src/main/java/io/opentelemetry/instrumentation/jmx/yaml/RuleParser.java index c4da81c7337d..d7042b80d0a6 100644 --- a/instrumentation/jmx-metrics/library/src/main/java/io/opentelemetry/instrumentation/jmx/yaml/RuleParser.java +++ b/instrumentation/jmx-metrics/library/src/main/java/io/opentelemetry/instrumentation/jmx/yaml/RuleParser.java @@ -142,6 +142,10 @@ private static void parseMetricStructure( if (unit != null) { out.setUnit(unit); } + String sourceUnit = (String) metricStructureYaml.remove("sourceUnit"); + if (sourceUnit != null) { + out.setSourceUnit(sourceUnit); + } } private static void failOnExtraKeys(Map yaml) { diff --git a/instrumentation/jmx-metrics/library/src/test/java/io/opentelemetry/instrumentation/jmx/engine/RuleParserTest.java b/instrumentation/jmx-metrics/library/src/test/java/io/opentelemetry/instrumentation/jmx/engine/RuleParserTest.java index 9ea5df7a02b2..23883672f912 100644 --- a/instrumentation/jmx-metrics/library/src/test/java/io/opentelemetry/instrumentation/jmx/engine/RuleParserTest.java +++ b/instrumentation/jmx-metrics/library/src/test/java/io/opentelemetry/instrumentation/jmx/engine/RuleParserTest.java @@ -63,6 +63,7 @@ static void setup() { + " ATTRIBUTE2:\n" + " metric: METRIC_NAME2\n" + " desc: DESCRIPTION2\n" + + " sourceUnit: SOURCE_UNIT2\n" + " unit: UNIT2\n" + " ATTRIBUTE3:\n" + " ATTRIBUTE4:\n" @@ -95,6 +96,7 @@ void testConf2() { assertThat(m1).isNotNull(); assertThat(m1.getMetric()).isEqualTo("METRIC_NAME1"); assertThat(m1.getMetricType()).isEqualTo(MetricInfo.Type.GAUGE); + assertThat(m1.getSourceUnit()).isNull(); assertThat(m1.getUnit()).isEqualTo("UNIT1"); assertThat(m1.getMetricAttribute()).containsExactly(entry("LABEL_KEY3", "const(CONSTANT)")); @@ -102,6 +104,7 @@ void testConf2() { assertThat(m2).isNotNull(); assertThat(m2.getMetric()).isEqualTo("METRIC_NAME2"); assertThat(m2.getDesc()).isEqualTo("DESCRIPTION2"); + assertThat(m2.getSourceUnit()).isEqualTo("SOURCE_UNIT2"); assertThat(m2.getUnit()).isEqualTo("UNIT2"); JmxRule def2 = defs.get(1); @@ -160,12 +163,14 @@ void testConf3() { + " LABEL_KEY2: beanattr(ATTRIBUTE)\n" + " prefix: PREFIX.\n" + " type: upDownCounter\n" + + " sourceUnit: DEFAULT_SOURCE_UNIT\n" + " unit: DEFAULT_UNIT\n" + " mapping:\n" + " A.b:\n" + " metric: METRIC_NAME1\n" + " type: counter\n" + " desc: DESCRIPTION1\n" + + " sourceUnit: SOURCE_UNIT1\n" + " unit: UNIT1\n" + " metricAttribute:\n" + " LABEL_KEY3: const(CONSTANT)\n" @@ -183,6 +188,7 @@ void testConf4() throws Exception { assertThat(defs).hasSize(1); JmxRule jmxDef = defs.get(0); + assertThat(jmxDef.getSourceUnit()).isEqualTo("DEFAULT_SOURCE_UNIT"); assertThat(jmxDef.getUnit()).isEqualTo("DEFAULT_UNIT"); assertThat(jmxDef.getMetricType()).isEqualTo(MetricInfo.Type.UPDOWNCOUNTER); @@ -202,6 +208,7 @@ void testConf4() throws Exception { MetricInfo metricInfo = m.getInfo(); assertThat(metricInfo.getMetricName()).isEqualTo("PREFIX.METRIC_NAME1"); assertThat(metricInfo.getDescription()).isEqualTo("DESCRIPTION1"); + assertThat(metricInfo.getSourceUnit()).isEqualTo("SOURCE_UNIT1"); assertThat(metricInfo.getUnit()).isEqualTo("UNIT1"); assertThat(metricInfo.getType()).isEqualTo(MetricInfo.Type.COUNTER); }) @@ -216,6 +223,7 @@ void testConf4() throws Exception { MetricInfo metricInfo = m.getInfo(); assertThat(metricInfo.getMetricName()).isEqualTo("PREFIX.METRIC_NAME2"); assertThat(metricInfo.getDescription()).isEqualTo("DESCRIPTION2"); + assertThat(metricInfo.getSourceUnit()).isEqualTo(jmxDef.getSourceUnit()); assertThat(metricInfo.getUnit()).isEqualTo("UNIT2"); }) .anySatisfy( @@ -233,6 +241,9 @@ void testConf4() throws Exception { assertThat(metricInfo.getUnit()) .describedAs("default unit should match jmx rule definition") .isEqualTo(jmxDef.getUnit()); + assertThat(metricInfo.getSourceUnit()) + .describedAs("default sourceUnit should match jmx rule definition") + .isEqualTo(jmxDef.getSourceUnit()); }); } diff --git a/instrumentation/jmx-metrics/library/src/test/java/io/opentelemetry/instrumentation/jmx/engine/UnitConverterTest.java b/instrumentation/jmx-metrics/library/src/test/java/io/opentelemetry/instrumentation/jmx/engine/UnitConverterTest.java new file mode 100644 index 000000000000..465e2487698e --- /dev/null +++ b/instrumentation/jmx-metrics/library/src/test/java/io/opentelemetry/instrumentation/jmx/engine/UnitConverterTest.java @@ -0,0 +1,96 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.instrumentation.jmx.engine; + +import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.CsvSource; + +class UnitConverterTest { + + @ParameterizedTest + @CsvSource({ + "1000000000,ns, 1.0", + "25,ns, 0.000000025", + "96614101945,ns, 96.614101945", + "0,ns, 0", + "1000000,us, 1.0", + "25,us, 0.000025", + "96614101945,us, 96614.101945", + "0,ns, 0", + "1000,ms, 1.0", + "25,ms, 0.025", + "9661410,ms, 9661.41", + "0,ms, 0", + }) + void shouldSupportPredefined_to_s_Conversions( + Long originalValue, String originalUnit, Double expectedConvertedValue) { + // Given + String targetUnit = "s"; + + // When + UnitConverter converter = UnitConverter.getInstance(originalUnit, targetUnit); + Number actualValue = converter.convert(originalValue); + + // Then + assertEquals(expectedConvertedValue, actualValue); + } + + @ParameterizedTest + @CsvSource({ + "--, --", + "ms, non-existing", + "non-existing, s", + }) + void shouldHandleUnsupportedConversion(String sourceUnit, String targetUnit) { + assertThatThrownBy(() -> UnitConverter.getInstance(sourceUnit, targetUnit)) + .hasMessage("Unsupported conversion from [" + sourceUnit + "] to [" + targetUnit + "]"); + } + + @ParameterizedTest + @CsvSource({ + ", s", // null -> "s" + "'', s", // "" -> "s" + }) + void shouldSkipConversionWhenSourceUnitNotSpecified(String sourceUnit, String targetUnit) { + UnitConverter converter = UnitConverter.getInstance(sourceUnit, targetUnit); + assertNull(converter); + } + + @Test + void shouldRegisterNewConversion() { + // Given + String sourceUnit = "h"; + String targetUnit = "s"; + + // When + UnitConverter.registerConversion("h", "s", hours -> hours.doubleValue() * 3600.0); + UnitConverter converter = UnitConverter.getInstance(sourceUnit, targetUnit); + Number actualValue = converter.convert(1.5); + + // Then + assertEquals(5400.0, actualValue); + } + + @ParameterizedTest + @CsvSource({ + "'', By", "By, ''", + }) + void shouldNotAllowRegisteringConversionWithAnyUnitEmpty(String sourceUnit, String targetUnit) { + assertThatThrownBy(() -> UnitConverter.registerConversion(sourceUnit, targetUnit, (value) -> 0)) + .hasMessageMatching("Non empty .+Unit must be provided"); + } + + @Test + void shouldNotAllowRegisteringAgainAlreadyExistingConversion() { + assertThatThrownBy(() -> UnitConverter.registerConversion("ms", "s", (v) -> 0)) + .hasMessage("Conversion from [ms] to [s] already defined"); + } +}