Backend jmx metrics #64

yazebochan · 2020-06-23T07:29:36Z

WIP

kafka-ui-contract/src/main/resources/swagger/kafka-ui-api.yaml

germanosin · 2020-06-29T11:08:40Z

kafka-ui-contract/src/main/resources/swagger/kafka-ui-api.yaml

@@ -303,14 +358,10 @@ components:
          type: integer
        topicCount:
          type: integer
-        bytesInPerSec:


please keep these params here

kafka-ui-contract/src/main/resources/swagger/kafka-ui-api.yaml

kafka-ui-api/src/main/java/com/provectus/kafka/ui/cluster/util/JmxClusterUtil.java

kafka-ui-contract/src/main/resources/swagger/kafka-ui-api.yaml

germanosin · 2020-07-02T06:46:58Z

kafka-ui-api/src/main/java/com/provectus/kafka/ui/cluster/util/JmxClusterUtil.java


-    public Map<String, Number> getJmxTrafficMetrics(int jmxPort, String jmxHost, String metricName) {
+    private Map<String, Object> getJmxMetric(int jmxPort, String jmxHost, String canonicalName) {


Why object here?

germanosin · 2020-07-02T06:48:01Z

kafka-ui-api/src/main/java/com/provectus/kafka/ui/cluster/util/JmxClusterUtil.java

        String jmxUrl = JMX_URL + jmxHost + ":" + jmxPort + "/" + JMX_SERVICE_TYPE;
-        Map<String, Number> result = new HashMap<>();
+
+        Map<String, Object> resultAttr = new HashMap<>();


I think it's better to collect metrics once, instead of doing it cluster-wide and then per broker

apetrovs · 2020-07-03T12:50:17Z

On my way.

…

On 2 Jul 2020, 09:48 +0300, German Osin ***@***.***>, wrote: @germanosin requested changes on this pull request. In kafka-ui-contract/src/main/resources/swagger/kafka-ui-api.yaml: > @@ -268,6 +273,61 @@ paths: schema: $ref: '#/components/schemas/ConsumerGroupDetails' +# /api/clusters/{clusterName}/brokers/{brokerId}/metrics/{canonicalName}: please clean up In kafka-ui-api/src/main/java/com/provectus/kafka/ui/cluster/util/JmxClusterUtil.java: > - public Map<String, Number> getJmxTrafficMetrics(int jmxPort, String jmxHost, String metricName) { + private Map<String, Object> getJmxMetric(int jmxPort, String jmxHost, String canonicalName) { Why object here? In kafka-ui-api/src/main/java/com/provectus/kafka/ui/cluster/util/JmxClusterUtil.java: > String jmxUrl = JMX_URL + jmxHost + ":" + jmxPort + "/" + JMX_SERVICE_TYPE; - Map<String, Number> result = new HashMap<>(); + + Map<String, Object> resultAttr = new HashMap<>(); I think it's better to collect metrics once, instead of doing it cluster-wide and then per broker — You are receiving this because your review was requested. Reply to this email directly, view it on GitHub, or unsubscribe.

germanosin · 2020-07-09T09:43:16Z

kafka-ui-api/src/main/java/com/provectus/kafka/ui/kafka/KafkaService.java

+                .flatMapIterable(nodes -> nodes)
+                .map(broker -> {
+                    var jmx = getJmxMetric(clusterName, broker);
+                    Map<Integer, InternalBrokerMetrics> result = new HashMap<>();


germanosin

please remove package-lock from commit

germanosin · 2020-07-09T09:44:23Z

kafka-ui-api/src/main/java/com/provectus/kafka/ui/kafka/KafkaService.java

+                })
+                .collectList()
+                .map(s -> { var brokerMetrics = ClusterUtil.toSingleMap(s.stream());
+                    return internalClusterMetrics.toBuilder().internalBrokerMetrics(brokerMetrics).build();


Please make it one line

germanosin · 2020-07-09T09:46:56Z

kafka-ui-api/src/main/java/com/provectus/kafka/ui/kafka/KafkaService.java

-                                    .internalBrokerMetrics((brokers.stream().map(Node::id).collect(Collectors.toMap(k -> k, v -> InternalBrokerMetrics.builder().build()))))
-                                    .bytesOutPerSec(bytesOutPerSec)
-                                    .bytesInPerSec(bytesInPerSec);
+                                    .internalBrokerMetrics((brokers.stream().map(Node::id).collect(Collectors.toMap(k -> k, v -> InternalBrokerMetrics.builder().build()))));


Don't you miss something here?

collect(Collectors.toMap(k -> k, v -> InternalBrokerMetrics.builder().build())))

germanosin · 2020-07-14T11:05:32Z

kafka-ui-api/src/main/java/com/provectus/kafka/ui/cluster/service/ClusterService.java

+                    brokerMetrics.setJmxMetrics(s.getInternalBrokerMetrics().get(id).getJmxMetrics());
+                    brokerMetrics.setSegmentZise(Long.valueOf(s.getSegmentSize()).intValue());
+                    return brokerMetrics;
+                }).orElseThrow());


Empty mono?

germanosin · 2020-07-14T11:08:04Z

kafka-ui-api/src/main/java/com/provectus/kafka/ui/cluster/util/JmxClusterUtil.java

+            jmxMetrics.forEach(j -> {
+                JmxMetric metric = new JmxMetric();
+                metric.setCanonicalName(j.getCanonicalName());
+                metric.setValue(getJmxMetric(jmxPort, jmxHost, j.getCanonicalName()));


Looks strange

germanosin · 2020-07-14T11:08:43Z

kafka-ui-api/src/main/java/com/provectus/kafka/ui/cluster/util/JmxClusterUtil.java

@@ -72,4 +99,12 @@ private void closeConnectionExceptionally(String url, JMXConnector srv) {
            log.error("Cannot invalidate object in pool, {}", url);
        }
    }
+
+    public static BigDecimal metricValueSum(Number value1, Number value2) {


germanosin · 2020-07-14T11:10:22Z

kafka-ui-api/src/main/java/com/provectus/kafka/ui/kafka/KafkaService.java

+        return getOrCreateAdminClient(cluster)
+                .flatMap(
+                ac -> getClusterMetrics(ac.getAdminClient())
+                        .flatMap(i -> fillJmxMetrics(i, cluster.getName()))


you can pass adminclient to method

germanosin · 2020-07-14T11:11:55Z

kafka-ui-api/src/main/java/com/provectus/kafka/ui/kafka/KafkaService.java

+
+    public List<JmxMetric> getJmxMetric(String clusterName, Node node) {
+        return clustersStorage.getClusterByName(clusterName)
+                        .map(c -> jmxClusterUtil.getJmxMetrics(c.getJmxPort(), node.host())).orElseThrow();


Empty list better here

…rsec keywords in canonicalname

germanosin · 2020-07-16T11:06:25Z

kafka-ui-api/src/main/java/com/provectus/kafka/ui/cluster/util/JmxClusterUtil.java

+        return internalClusterMetrics.getInternalBrokerMetrics().values().stream()
+                .map(c ->
+                        c.getJmxMetrics().stream()
+                                .filter(j -> StringUtils.containsIgnoreCase(j.getCanonicalName(), "num") || StringUtils.containsIgnoreCase(j.getCanonicalName(), "persec"))


move to constants and array

Let's set list of metrics we could rollup

germanosin · 2020-07-16T11:09:02Z

kafka-ui-api/src/main/java/com/provectus/kafka/ui/cluster/util/JmxClusterUtil.java

+                                .filter(j -> StringUtils.containsIgnoreCase(j.getCanonicalName(), "num") || StringUtils.containsIgnoreCase(j.getCanonicalName(), "persec"))
+                                .map(j -> j.getValue().entrySet().stream()
+                                        .map(e -> Pair.of(j.getCanonicalName() + "+" + e.getKey(), e.getValue()))
+                                        .collect(Collectors.toList()))


flatmap stream?

germanosin · 2020-07-16T11:10:51Z

kafka-ui-api/src/main/java/com/provectus/kafka/ui/kafka/KafkaService.java

+                        jmx.setCanonicalName(c.getKey());
+                        jmx.setValue(Map.of(c.getValue().getKey(), c.getValue().getValue()));
+                        return jmx;
+                    }).collect(Collectors.groupingBy(JmxMetric::getCanonicalName, Collectors.toList()))


you can reduce on groupingby

germanosin · 2020-07-16T11:11:46Z

kafka-ui-api/src/main/java/com/provectus/kafka/ui/cluster/util/JmxClusterUtil.java

+                        c.getJmxMetrics().stream()
+                                .filter(j -> StringUtils.containsIgnoreCase(j.getCanonicalName(), "num") || StringUtils.containsIgnoreCase(j.getCanonicalName(), "persec"))
+                                .map(j -> j.getValue().entrySet().stream()
+                                        .map(e -> Pair.of(j.getCanonicalName() + "+" + e.getKey(), e.getValue()))


Think how to improve all these key transformations

germanosin · 2020-07-24T11:15:18Z

kafka-ui-contract/src/main/resources/swagger/kafka-ui-api.yaml

-          type: object
-          additionalProperties:
-            type: number
+        jmxMetrics:


let's name it metrics, not a jmx metrics

germanosin · 2020-07-24T11:15:31Z

kafka-ui-contract/src/main/resources/swagger/kafka-ui-api.yaml

+            $ref: '#/components/schemas/ConsumerTopicPartitionDetail'
+
+    JmxMetric:


* Start doing endpoint for jmx metrics * Added endpoint for getting jmx metric per broker * Cluster jmx metrics sum endpoit added * Added endpoints for cluster metrics and broker metrics * Cleared some code * Fixed jmxmetrics names * Changed to all values in metrics * Removed redundant imports * Renamed param constant * Changed to calculate brokers and clusters metrics in one place * Removed redundant imports * Fixed some mistakes * Replaced multiple method usage into single * Fixed mulptiple call * Removed cluster level metrics, now only broker-level metrics in cluster * Just small fixes * removed redundant variable * Renamed method for cluster level metrics * Fixed after PR and added sum for number cluster metrics by num and persec keywords in canonicalname * Added metricdto object * Added list of metrics to enum * Renames and optimizings * Renamed jmxmetrics objects param to metrics Co-authored-by: Roman Nedzvetskiy <roman@Romans-MacBook-Pro.local>

Roman Nedzvetskiy added 4 commits June 16, 2020 13:19

Start doing endpoint for jmx metrics

d46a39e

Merge branch 'master' into backend-jmx-metrics

914360e

Merge branch 'master' into backend-jmx-metrics

a562c98

Added endpoint for getting jmx metric per broker

f427d01

yazebochan requested review from germanosin and apetrovs June 23, 2020 07:29

Roman Nedzvetskiy added 4 commits June 23, 2020 14:03

Cluster jmx metrics sum endpoit added

a24b218

Added endpoints for cluster metrics and broker metrics

90df65b

Cleared some code

b320aa6

Fixed jmxmetrics names

319d027

germanosin suggested changes Jun 29, 2020

View reviewed changes

Roman Nedzvetskiy added 4 commits June 30, 2020 22:29

Changed to all values in metrics

0c72e07

Removed redundant imports

e5c1bb8

Renamed param constant

4e624c1

Merge branch 'master' into backend-jmx-metrics

65bc33e

germanosin suggested changes Jul 2, 2020

View reviewed changes

Roman Nedzvetskiy added 6 commits July 4, 2020 16:18

Changed to calculate brokers and clusters metrics in one place

40e21c4

Removed redundant imports

6035003

Fixed some mistakes

bceb161

Replaced multiple method usage into single

c64579c

Fixed mulptiple call

51a6a74

Removed cluster level metrics, now only broker-level metrics in cluster

b218600

germanosin reviewed Jul 9, 2020

View reviewed changes

germanosin suggested changes Jul 9, 2020

View reviewed changes

germanosin reviewed Jul 9, 2020

View reviewed changes

Roman Nedzvetskiy added 3 commits July 9, 2020 14:54

Just small fixes

21ca9a0

removed redundant variable

04ca14d

Renamed method for cluster level metrics

105ef89

germanosin suggested changes Jul 14, 2020

View reviewed changes

Fixed after PR and added sum for number cluster metrics by num and pe…

4edc5f8

…rsec keywords in canonicalname

germanosin suggested changes Jul 16, 2020

View reviewed changes

Roman Nedzvetskiy added 3 commits July 23, 2020 10:12

Added metricdto object

5beb6ae

Added list of metrics to enum

5ef0269

Renames and optimizings

9b04938

germanosin suggested changes Jul 24, 2020

View reviewed changes

Roman Nedzvetskiy added 2 commits July 27, 2020 10:28

Renamed jmxmetrics objects param to metrics

c81c51d

Fixed some issues

3e212aa

germanosin merged commit efc35a9 into master Jul 30, 2020

germanosin deleted the backend-jmx-metrics branch July 30, 2020 11:03

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Backend jmx metrics #64

Backend jmx metrics #64

yazebochan commented Jun 23, 2020

germanosin Jun 29, 2020

germanosin Jul 2, 2020

germanosin Jul 2, 2020

apetrovs commented Jul 3, 2020 via email

germanosin Jul 9, 2020

germanosin left a comment

germanosin Jul 9, 2020

germanosin Jul 9, 2020

germanosin Jul 14, 2020

germanosin Jul 14, 2020

germanosin Jul 14, 2020

germanosin Jul 14, 2020

germanosin Jul 14, 2020

germanosin Jul 16, 2020

germanosin Jul 16, 2020

germanosin Jul 16, 2020

germanosin Jul 16, 2020

germanosin Jul 16, 2020

germanosin Jul 24, 2020

germanosin Jul 24, 2020


		public Map<String, Number> getJmxTrafficMetrics(int jmxPort, String jmxHost, String metricName) {
		private Map<String, Object> getJmxMetric(int jmxPort, String jmxHost, String canonicalName) {

		$ref: '#/components/schemas/ConsumerTopicPartitionDetail'

		JmxMetric:

Backend jmx metrics #64

Backend jmx metrics #64

Conversation

yazebochan commented Jun 23, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

apetrovs commented Jul 3, 2020 via email

Choose a reason for hiding this comment

germanosin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment