Closes #785 - Added autocompleter for declared metric names & fixed bug in ModelAutoCompleter #788

MariusBrill · 2020-06-23T06:41:45Z

This change is

…elds.

codecov · 2020-06-25T07:00:05Z

Codecov Report

Merging #788 into master will decrease coverage by 0.10%.
The diff coverage is 79.65%.

@@             Coverage Diff              @@
##             master     #788      +/-   ##
============================================
- Coverage     83.00%   82.91%   -0.10%     
- Complexity     1736     1759      +23     
============================================
  Files           174      178       +4     
  Lines          5366     5470     +104     
  Branches        650      658       +8     
============================================
+ Hits           4454     4535      +81     
- Misses          675      694      +19     
- Partials        237      241       +4

Impacted Files	Coverage Δ	Complexity Δ
...t/ocelot/config/model/tracing/TracingSettings.java	`100.00% <ø> (ø)`	`0.00 <0.00> (ø)`
...t/ocelot/config/validation/PropertyPathHelper.java	`79.59% <0.00%> (-1.02%)`	`42.00 <0.00> (-1.00)`
...lation/log/adapters/JBossLogmanagerMDCAdapter.java	`66.67% <66.67%> (ø)`	`2.00 <2.00> (?)`
...nstrumentation/context/ContextPropagationUtil.java	`84.73% <70.37%> (-3.00%)`	`31.00 <3.00> (+5.00)`	⬇️
...rumentation/context/propagation/DatadogFormat.java	`77.14% <77.14%> (ø)`	`7.00 <7.00> (?)`
...trumentation/context/PropagationFormatManager.java	`80.00% <80.00%> (ø)`	`3.00 <3.00> (?)`
...ocelot/config/model/tracing/PropagationFormat.java	`100.00% <100.00%> (ø)`	`1.00 <1.00> (?)`
...ore/instrumentation/correlation/log/MDCAccess.java	`87.80% <100.00%> (+3.93%)`	`16.00 <3.00> (+3.00)`
...ion/correlation/log/adapters/Log4J1MDCAdapter.java	`80.00% <100.00%> (+2.22%)`	`4.00 <1.00> (+1.00)`
...ion/correlation/log/adapters/Log4J2MDCAdapter.java	`80.00% <100.00%> (+2.22%)`	`4.00 <1.00> (+1.00)`
... and 11 more

mariusoe

Reviewed 4 of 4 files at r1.
Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @mbrill-nt)

components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/autocomplete/autocompleterimpl/MetricsAutoCompleter.java, line 13 at r1 (raw file):

import java.util.List;

@Component

Small docs please :)

components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/autocomplete/autocompleterimpl/MetricsAutoCompleter.java, line 22 at r1 (raw file):

     * The path under which metric names are defined.
     */
    private final static List<String> METRICS_DECLARATION_PATH = Arrays.asList("inspectit", "instrumentation", "metrics");

This does not exist. The metrics are defined under the key inspectit.metrics.definitions

components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/autocomplete/autocompleterimpl/MetricsAutoCompleter.java, line 29 at r1 (raw file):

    private static final List<List<String>> RULE_SUGGESTION_PATHS = Arrays.asList(
            METRICS_DECLARATION_PATH,
            Arrays.asList("inspectit", "instrumentation", "rules", "*", "metrics")

There is another place where you can use these suggestions. You can define them under the following key as well:

inspectit.instrumentation.rules.*.metrics.*.metric

inspectit-ocelot-config/src/main/java/rocks/inspectit/ocelot/config/validation/PropertyPathHelper.java, line 119 at r1 (raw file):

            return ((Class<?>) type).isEnum()
                    || ((Class<?>) type).isPrimitive()
                    || InspectitConfigConversionService.getInstance().canConvert((Class<?>) type, String.class)

I think this is wrong. You can basically convert everything into a string..
As far as I understand this method, you want to check whether you can convert a simple string or number into an object, right?

inspectit-ocelot-config/src/test/java/rocks/inspectit/ocelot/config/validation/PropertyPathHelperTest.java, line 149 at r1 (raw file):

        @Test
        public void convertibles() {
            boolean result = PropertyPathHelper.isTerminal(MetricRecordingSettings.class);

Please recheck whether this is correct. Why is isTerminal returning true for this class? It's a complex type, basically a map, thus, returning true seems wrong because the property path is not terminating here (for example, the constant-tagsis defining more elements)

MariusBrill

Reviewable status: 1 of 4 files reviewed, 5 unresolved discussions (waiting on @mariusoe)

components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/autocomplete/autocompleterimpl/MetricsAutoCompleter.java, line 13 at r1 (raw file):