gemmellr commented on code in PR #5230:
URL: https://github.com/apache/activemq-artemis/pull/5230#discussion_r1758942956
##########
artemis-server/src/main/java/org/apache/activemq/artemis/core/server/metrics/NettyPooledAllocatorMetrics.java:
##########
@@ -33,53 +34,64 @@ public final class NettyPooledAllocatorMetrics implements
MeterBinder {
private static final String BYTES_UNIT = "bytes";
private final PooledByteBufAllocatorMetric metric;
+ private final Tags tags;
- public NettyPooledAllocatorMetrics(final PooledByteBufAllocatorMetric
pooledAllocatorMetric) {
+ public NettyPooledAllocatorMetrics(final PooledByteBufAllocatorMetric
pooledAllocatorMetric, final Tags tags) {
this.metric = pooledAllocatorMetric;
+ this.tags = tags;
Review Comment:
Maybe _commonTags_ to make the linkage to where they came from, and how they
are used here, clearer?
##########
tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/plugin/PassthroughMetricsPluginTest.java:
##########
@@ -0,0 +1,78 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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
+ * <br>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <br>
+ * 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 org.apache.activemq.artemis.tests.integration.plugin;
+
+import java.util.Map;
+
+import io.micrometer.core.instrument.Meter;
+import io.micrometer.core.instrument.MeterRegistry;
+import io.micrometer.core.instrument.Tags;
+import io.micrometer.core.instrument.simple.SimpleMeterRegistry;
+import org.apache.activemq.artemis.core.config.MetricsConfiguration;
+import org.apache.activemq.artemis.core.server.ActiveMQServer;
+import org.apache.activemq.artemis.core.server.metrics.ActiveMQMetricsPlugin;
+import org.apache.activemq.artemis.utils.RandomUtil;
+import org.junit.jupiter.api.Disabled;
+import org.junit.jupiter.api.Test;
+
+import static
org.apache.activemq.artemis.core.server.metrics.MetricsManager.BROKER_TAG_NAME;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+public class PassthroughMetricsPluginTest extends MetricsPluginTest {
+
+ private MeterRegistry meterRegistry;
+
+ @Override
+ protected void configureServer(ActiveMQServer server) {
+ meterRegistry = new SimpleMeterRegistry();
+ server.getConfiguration().setMetricsConfiguration(new
MetricsConfiguration().setPlugin(new ActiveMQMetricsPlugin() {
+ @Override
+ public ActiveMQMetricsPlugin init(Map<String, String> map) {
+ return this;
+ }
+
+ @Override
+ public MeterRegistry getRegistry() {
+ return meterRegistry;
+ }
+ }));
+ }
+
+ @Test
+ public void testPassthroughMeterRegistry() {
+ final String meterName = "myGauge";
Review Comment:
Could perhaps use the test name? E.g getTestMethodName();
##########
tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/plugin/PassthroughMetricsPluginTest.java:
##########
@@ -0,0 +1,78 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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
+ * <br>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <br>
Review Comment:
Ditto
##########
tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/plugin/LegacyConfigMetricsPluginTest.java:
##########
@@ -0,0 +1,30 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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
+ * <br>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <br>
+ * 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 org.apache.activemq.artemis.tests.integration.plugin;
+
+import org.apache.activemq.artemis.core.config.MetricsConfiguration;
+import org.apache.activemq.artemis.core.server.ActiveMQServer;
+import
org.apache.activemq.artemis.core.server.metrics.plugins.SimpleMetricsPlugin;
+
+public class LegacyConfigMetricsPluginTest extends MetricsPluginTest {
+
+ @Override
+ protected void configureServer(ActiveMQServer server) {
+ server.getConfiguration().setMetricsConfiguration(new
MetricsConfiguration().setPlugin(new SimpleMetricsPlugin().init(null)));
Review Comment:
C&P error?
This does not align with that the other test did during its _legacyConfig_
runs (snippet below), but rather matches the prior non-legacy runs, and also
exactly matches what the parent non-LegacyConfig MetricsPluginTest is already
doing, i.e they are testing the same things.
```
if (legacyConfig) {
server.getConfiguration().setMetricsPlugin(new
SimpleMetricsPlugin().init(null));
} else {
server.getConfiguration().setMetricsConfiguration(new
MetricsConfiguration().setPlugin(new SimpleMetricsPlugin().init(null)));
}
```
##########
tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/plugin/LegacyConfigMetricsPluginTest.java:
##########
@@ -0,0 +1,30 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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
+ * <br>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <br>
Review Comment:
Isnt Javadoc so shouldnt have the \<br\> elements, and the URL isn't
indented correctly.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscr...@activemq.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscr...@activemq.apache.org
For additional commands, e-mail: gitbox-h...@activemq.apache.org
For further information, visit: https://activemq.apache.org/contact
