Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[stats] Do not expose meaningless stats for publisher #11454

Merged
merged 2 commits into from Jul 28, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -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;
Expand Down Expand Up @@ -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";
Expand All @@ -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<String> itr = node.fieldNames();
while (itr.hasNext()) {
String field = itr.next();
Assert.assertTrue(allowedFields.contains(field), field + " should not be exposed");
}

consumer.close();
Expand Down
Expand Up @@ -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;
Expand All @@ -28,6 +29,7 @@
*/
@Data
public class PublisherStatsImpl implements PublisherStats {
@JsonIgnore
private int count;

public ProducerAccessMode accessMode;
Expand All @@ -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. */
Expand Down
Expand Up @@ -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;
Expand All @@ -37,6 +38,7 @@
*/
@Data
public class TopicStatsImpl implements TopicStats {
@JsonIgnore
private int count;

/** Total rate of messages published on the topic (msg/s). */
Expand Down
Expand Up @@ -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<String> allowedFields = Sets.newHashSet(
"accessMode",
"msgRateIn",
"msgThroughputIn",
"averageMsgSize",
"chunkedMessageRate",
"producerId",
"metadata",
"address",
"connectedSince",
"clientVersion",
"producerName"
);

PublisherStatsImpl stats = new PublisherStatsImpl();
assertNull(stats.getAddress());
assertNull(stats.getClientVersion());
Expand All @@ -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<String> 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());
Expand Down