Skip to content

Commit

Permalink
[3.x] Fix improper handling of metrics global tags (#5812)
Browse files Browse the repository at this point in the history
* Fix improper handling of metrics global tags
  • Loading branch information
tjquinno authored Jan 10, 2023
1 parent da33523 commit c4e1028
Show file tree
Hide file tree
Showing 6 changed files with 168 additions and 8 deletions.
60 changes: 59 additions & 1 deletion metrics/api/pom.xml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<?xml version="1.0" encoding="UTF-8"?>
<!--
Copyright (c) 2021, 2022 Oracle and/or its affiliates.
Copyright (c) 2021, 2023 Oracle and/or its affiliates.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -93,6 +93,64 @@
</annotationProcessorPaths>
</configuration>
</plugin>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-surefire-plugin</artifactId>
<executions>
<execution>
<id>default-test</id>
<configuration>
<excludes>
<!-- Avoid any config pollution -->
<exclude>**/TestGlobalTags.java</exclude>
</excludes>
</configuration>
</execution>
<execution>
<!-- Without the bug fix, this test fails with
java.lang.IllegalArgumentException: Error(s) in global tag expression: [Missing '=': topLevel]
-->
<id>check-top-level-tags-only</id>
<goals>
<goal>test</goal>
</goals>
<configuration>
<systemPropertyVariables>
<testSelection>topLevel</testSelection>
</systemPropertyVariables>
<environmentVariables>
<tags>topLevel</tags>
</environmentVariables>
<includes>
<include>**/TestGlobalTags.java</include>
</includes>
</configuration>
</execution>
<execution>
<!-- Without the bug fix, this test fails with
java.lang.IllegalArgumentException: Error(s) in global tag expression: [Missing '=': topLevel]
-->
<id>check-top-level-and-metrics-level-tags</id>
<goals>
<goal>test</goal>
</goals>
<configuration>
<systemPropertyVariables>
<testSelection>topLevelAndMetricsLevel</testSelection>
<!-- Use properties for the metrics global tag instead of env var;
dots in env names seem to not always work. -->
<metrics.tags>myTag=myValue</metrics.tags>
</systemPropertyVariables>
<environmentVariables>
<tags>topLevel</tags>
</environmentVariables>
<includes>
<include>**/TestGlobalTags.java</include>
</includes>
</configuration>
</execution>
</executions>
</plugin>
</plugins>
</build>

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2021, 2022 Oracle and/or its affiliates.
* Copyright (c) 2021, 2023 Oracle and/or its affiliates.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -33,12 +33,12 @@
public interface MetricsSettings {

/**
* Returns default metrics settings based on default config.
* Returns default metrics settings based on the {@code metrics} section of the default config.
*
* @return new settings reflecting the default config
* @return new settings reflecting the default metrics config
*/
static MetricsSettings create() {
return create(Config.create());
return create(Config.create().get(Builder.METRICS_CONFIG_KEY));
}

/**
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
/*
* Copyright (c) 2023 Oracle and/or its affiliates.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package io.helidon.metrics.api;

import java.util.AbstractMap;

import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.condition.DisabledIfSystemProperty;
import org.junit.jupiter.api.condition.EnabledIfSystemProperty;

import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.emptyIterable;
import static org.hamcrest.Matchers.hasItem;

class TestGlobalTags {

@Test
@DisabledIfSystemProperty(named = "testSelection", matches = "topLevelAndMetricsLevel")
void testTopLevelTagsIgnoredForMetrics() {
MetricsSettings metricsSettings = MetricsSettings.create();
assertThat("Global tags with top-level 'tags' assigned", metricsSettings.globalTags().entrySet(), emptyIterable());
}

@Test
@EnabledIfSystemProperty(named = "testSelection", matches = "topLevelAndMetricsLevel")
void testGlobalTagsForMetrics() {
String tag = "myTag";
String value = "myValue";
String globalTags = tag + "=" + value;
MetricsSettings metricsSettings = MetricsSettings.create();
assertThat("Global tags with top-level and metrics 'tags' assigned",
metricsSettings.globalTags().entrySet(),
hasItem(new AbstractMap.SimpleEntry<>(tag, value)));
}
}
17 changes: 16 additions & 1 deletion microprofile/metrics/pom.xml
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<?xml version="1.0" encoding="UTF-8"?>
<!--
Copyright (c) 2018, 2022 Oracle and/or its affiliates.
Copyright (c) 2018, 2023 Oracle and/or its affiliates.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -118,6 +118,7 @@
<exclude>**/TestExtendedKPIMetrics.java</exclude>
<exclude>**/TestMetricsOnOwnSocket.java</exclude>
<exclude>**/TestDisabledMetrics.java</exclude>
<exclude>**/TestConfigProcessing.java</exclude>
</excludes>
</configuration>
</execution>
Expand Down Expand Up @@ -151,6 +152,20 @@
</excludes>
</configuration>
</execution>
<execution>
<id>test-config-with-top-level-tags</id>
<goals>
<goal>test</goal>
</goals>
<configuration>
<includes>
<include>**/TestConfigProcessing.java</include>
</includes>
<environmentVariables>
<TAGS>topLevel</TAGS>
</environmentVariables>
</configuration>
</execution>
</executions>
</plugin>
</plugins>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2018, 2022 Oracle and/or its affiliates.
* Copyright (c) 2018, 2023 Oracle and/or its affiliates.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -781,7 +781,9 @@ protected Config seComponentConfig() {

Config metricsConfig = mpConfig.get("metrics").detach();

Config.Builder builder = Config.builder();
Config.Builder builder = Config.builder()
.disableEnvironmentVariablesSource()
.disableSystemPropertiesSource();
if (!mpConfigSettings.isEmpty()) {
builder.addSource(ConfigSources.create(mpConfigSettings));
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
/*
* Copyright (c) 2023 Oracle and/or its affiliates.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package io.helidon.microprofile.metrics;

import io.helidon.config.Config;
import io.helidon.microprofile.tests.junit5.HelidonTest;

import jakarta.enterprise.inject.spi.CDI;
import org.junit.jupiter.api.Test;

import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.is;

@HelidonTest()
class TestConfigProcessing {

@Test
void checkTopLeveTagsIgnoredForMetrics() {
MetricsCdiExtension extension = CDI.current().getBeanManager().getExtension(MetricsCdiExtension.class);
Config seConfig = extension.seComponentConfig();
Config metricsTags = seConfig.get("tags");
assertThat("Tags setting is present", metricsTags.asString().isPresent(), is(false));
}
}

0 comments on commit c4e1028

Please sign in to comment.