From af38da149af0e6f27e06cbd6ad98951c97f167d6 Mon Sep 17 00:00:00 2001 From: Masahiro Sakamoto Date: Mon, 26 Jul 2021 00:38:24 +0900 Subject: [PATCH 1/2] Do not expose meaningless stats for publisher --- .../broker/stats/ConsumerStatsTest.java | 13 +++++--- .../data/stats/PublisherStatsImpl.java | 11 +++++++ .../policies/data/stats/TopicStatsImpl.java | 2 ++ .../policies/data/PublisherStatsTest.java | 32 ++++++++++++++++++- 4 files changed, 53 insertions(+), 5 deletions(-) diff --git a/pulsar-broker/src/test/java/org/apache/pulsar/broker/stats/ConsumerStatsTest.java b/pulsar-broker/src/test/java/org/apache/pulsar/broker/stats/ConsumerStatsTest.java index f9eaa1b1b511a..2f702ab89f645 100644 --- a/pulsar-broker/src/test/java/org/apache/pulsar/broker/stats/ConsumerStatsTest.java +++ b/pulsar-broker/src/test/java/org/apache/pulsar/broker/stats/ConsumerStatsTest.java @@ -38,6 +38,7 @@ import org.testng.annotations.BeforeMethod; import org.testng.annotations.Test; +import java.util.Iterator; import java.util.List; import java.util.Set; import java.util.concurrent.TimeUnit; @@ -193,7 +194,10 @@ public void testConsumerStatsOutput() throws Exception { "lastAckedTimestamp", "lastConsumedTimestamp", "keyHashRanges", - "metadata"); + "metadata", + "address", + "connectedSince", + "clientVersion"); final String topicName = "persistent://prop/use/ns-abc/testConsumerStatsOutput"; final String subName = "my-subscription"; @@ -208,9 +212,10 @@ public void testConsumerStatsOutput() throws Exception { ObjectMapper mapper = ObjectMapperFactory.create(); JsonNode node = mapper.readTree(mapper.writer().writeValueAsString(stats.getSubscriptions() .get(subName).getConsumers().get(0))); - if (node.fieldNames().hasNext()) { - String field = node.fieldNames().next(); - Assert.assertTrue(allowedFields.contains(field)); + Iterator itr = node.fieldNames(); + while (itr.hasNext()) { + String field = itr.next(); + Assert.assertTrue(allowedFields.contains(field), field + " should not be exposed"); } consumer.close(); diff --git a/pulsar-common/src/main/java/org/apache/pulsar/common/policies/data/stats/PublisherStatsImpl.java b/pulsar-common/src/main/java/org/apache/pulsar/common/policies/data/stats/PublisherStatsImpl.java index 683b1eac0ab9f..887bae1722ca2 100644 --- a/pulsar-common/src/main/java/org/apache/pulsar/common/policies/data/stats/PublisherStatsImpl.java +++ b/pulsar-common/src/main/java/org/apache/pulsar/common/policies/data/stats/PublisherStatsImpl.java @@ -18,6 +18,7 @@ */ package org.apache.pulsar.common.policies.data.stats; +import com.fasterxml.jackson.annotation.JsonIgnore; import lombok.Data; import org.apache.pulsar.client.api.ProducerAccessMode; import org.apache.pulsar.common.policies.data.PublisherStats; @@ -28,6 +29,7 @@ */ @Data public class PublisherStatsImpl implements PublisherStats { + @JsonIgnore private int count; public ProducerAccessMode accessMode; @@ -48,25 +50,34 @@ public class PublisherStatsImpl implements PublisherStats { public long producerId; /** Producer name. */ + @JsonIgnore private int producerNameOffset = -1; + @JsonIgnore private int producerNameLength; /** Address of this publisher. */ + @JsonIgnore private int addressOffset = -1; + @JsonIgnore private int addressLength; /** Timestamp of connection. */ + @JsonIgnore private int connectedSinceOffset = -1; + @JsonIgnore private int connectedSinceLength; /** Client library version. */ + @JsonIgnore private int clientVersionOffset = -1; + @JsonIgnore private int clientVersionLength; /** * In order to prevent multiple string objects under stats: create a string-buffer that stores data for all string * place-holders. */ + @JsonIgnore private StringBuilder stringBuffer = new StringBuilder(); /** Metadata (key/value strings) associated with this publisher. */ diff --git a/pulsar-common/src/main/java/org/apache/pulsar/common/policies/data/stats/TopicStatsImpl.java b/pulsar-common/src/main/java/org/apache/pulsar/common/policies/data/stats/TopicStatsImpl.java index 941c318b70195..ec97d6e35c851 100644 --- a/pulsar-common/src/main/java/org/apache/pulsar/common/policies/data/stats/TopicStatsImpl.java +++ b/pulsar-common/src/main/java/org/apache/pulsar/common/policies/data/stats/TopicStatsImpl.java @@ -18,6 +18,7 @@ */ package org.apache.pulsar.common.policies.data.stats; +import com.fasterxml.jackson.annotation.JsonIgnore; import lombok.AccessLevel; import lombok.Data; import lombok.Getter; @@ -37,6 +38,7 @@ */ @Data public class TopicStatsImpl implements TopicStats { + @JsonIgnore private int count; /** Total rate of messages published on the topic (msg/s). */ diff --git a/pulsar-common/src/test/java/org/apache/pulsar/common/policies/data/PublisherStatsTest.java b/pulsar-common/src/test/java/org/apache/pulsar/common/policies/data/PublisherStatsTest.java index 6bc8771127bb9..9f34a9128e48f 100644 --- a/pulsar-common/src/test/java/org/apache/pulsar/common/policies/data/PublisherStatsTest.java +++ b/pulsar-common/src/test/java/org/apache/pulsar/common/policies/data/PublisherStatsTest.java @@ -20,14 +20,35 @@ import static org.testng.Assert.assertEquals; import static org.testng.Assert.assertNull; +import static org.testng.Assert.assertTrue; +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.google.common.collect.Sets; +import java.util.Iterator; +import java.util.Set; import org.apache.pulsar.common.policies.data.stats.PublisherStatsImpl; +import org.apache.pulsar.common.util.ObjectMapperFactory; import org.testng.annotations.Test; public class PublisherStatsTest { @Test - public void testPublisherStats() { + public void testPublisherStats() throws Exception { + Set allowedFields = Sets.newHashSet( + "accessMode", + "msgRateIn", + "msgThroughputIn", + "averageMsgSize", + "chunkedMessageRate", + "producerId", + "metadata", + "address", + "connectedSince", + "clientVersion", + "producerName" + ); + PublisherStatsImpl stats = new PublisherStatsImpl(); assertNull(stats.getAddress()); assertNull(stats.getClientVersion()); @@ -53,6 +74,15 @@ public void testPublisherStats() { assertEquals(stats.getConnectedSince(), "connected"); assertEquals(stats.getAddress(), "address1"); assertEquals(stats.getClientVersion(), "version"); + + // Check if private fields are included in json + ObjectMapper mapper = ObjectMapperFactory.create(); + JsonNode node = mapper.readTree(mapper.writer().writeValueAsString(stats)); + Iterator itr = node.fieldNames(); + while (itr.hasNext()) { + String field = itr.next(); + assertTrue(allowedFields.contains(field), field + " should not be exposed"); + } stats.setAddress(null); assertNull(stats.getAddress()); From 637b7aa9ece9fd3c92d5e8193211edc4a46a7271 Mon Sep 17 00:00:00 2001 From: Masahiro Sakamoto Date: Tue, 27 Jul 2021 16:39:34 +0900 Subject: [PATCH 2/2] Remove fields from json example --- site2/docs/getting-started-docker.md | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/site2/docs/getting-started-docker.md b/site2/docs/getting-started-docker.md index 675fff8219e1f..c3a241a314511 100644 --- a/site2/docs/getting-started-docker.md +++ b/site2/docs/getting-started-docker.md @@ -106,7 +106,6 @@ The output is something like this: ```json { - "count": 0, "msgRateIn": 0.0, "msgThroughputIn": 0.0, "msgRateOut": 1.8332950480217471, @@ -122,22 +121,12 @@ The output is something like this: "offloadedStorageSize": 0, "publishers": [ { - "count": 0, "accessMode": "Shared", "msgRateIn": 0.0, "msgThroughputIn": 0.0, "averageMsgSize": 0.0, "chunkedMessageRate": 0.0, "producerId": 0, - "producerNameOffset": 47, - "producerNameLength": 14, - "addressOffset": 0, - "addressLength": 16, - "connectedSinceOffset": 16, - "connectedSinceLength": 26, - "clientVersionOffset": 42, - "clientVersionLength": 5, - "stringBuffer": "/127.0.0.1:356042021-07-04T09:05:43.04788Z2.8.0standalone-2-5", "metadata": {}, "address": "/127.0.0.1:35604", "connectedSince": "2021-07-04T09:05:43.04788Z", @@ -182,16 +171,9 @@ The output is something like this: "unackedMessages": 0, "avgMessagesPerEntry": 6, "blockedConsumerOnUnackedMsgs": false, - "addressOffset": 0, - "addressLength": 16, - "connectedSinceOffset": 16, - "connectedSinceLength": 27, - "clientVersionOffset": 43, - "clientVersionLength": 5, "lastAckedTimestamp": 1625389546162, "lastConsumedTimestamp": 1625389546070, "metadata": {}, - "stringBuffer": "/127.0.0.1:354722021-07-04T08:58:21.287682Z2.8.0", "address": "/127.0.0.1:35472", "connectedSince": "2021-07-04T08:58:21.287682Z", "clientVersion": "2.8.0"