asafm commented on code in PR #18102:
URL: https://github.com/apache/pulsar/pull/18102#discussion_r999595092
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/web/WebService.java:
##########
@@ -262,7 +263,16 @@ public void addServlet(String path, ServletHolder
servletHolder, boolean require
});
}
filterInitializer.addFilters(context, requiresAuthentication);
- handlers.add(context);
+ // Enable compress on /metrics endpoint
+ if (isCompress) {
+ GzipHandler gzipHandler = new GzipHandler();
+
gzipHandler.setMinGzipSize(pulsar.getConfiguration().getMinGzipSize());
Review Comment:
So you can only enable compression for the Prometheus servlet, but decide
min gzip size on any? This doesn't make sense to me.
I would leave`addServlet` method as is, uncompressed, and add another method
accepting `CompressionConfiguration(enabled, minResponseSize)`
##########
pulsar-broker-common/src/main/java/org/apache/pulsar/broker/ServiceConfiguration.java:
##########
@@ -327,6 +327,18 @@ public class ServiceConfiguration implements
PulsarConfiguration {
+ "(0 to disable limiting)")
private int maxHttpServerConnections = 2048;
+ @FieldContext(
+ category = CATEGORY_SERVER,
+ doc = "Enable or disable compress output metrics in prometheus"
+ )
+ private boolean compressOutputMetricsInPrometheus = false;
Review Comment:
It's not really in Prometheus, so maybe `compressPrometheusMetricsOutput`?
##########
pulsar-broker-common/src/main/java/org/apache/pulsar/broker/ServiceConfiguration.java:
##########
@@ -327,6 +327,18 @@ public class ServiceConfiguration implements
PulsarConfiguration {
+ "(0 to disable limiting)")
private int maxHttpServerConnections = 2048;
+ @FieldContext(
+ category = CATEGORY_SERVER,
+ doc = "Enable or disable compress output metrics in prometheus"
Review Comment:
Since it's only Gzip, let's write it?
##########
pulsar-broker-common/src/main/java/org/apache/pulsar/broker/ServiceConfiguration.java:
##########
@@ -327,6 +327,18 @@ public class ServiceConfiguration implements
PulsarConfiguration {
+ "(0 to disable limiting)")
private int maxHttpServerConnections = 2048;
+ @FieldContext(
+ category = CATEGORY_SERVER,
+ doc = "Enable or disable compress output metrics in prometheus"
+ )
+ private boolean compressOutputMetricsInPrometheus = false;
+
+ @FieldContext(
+ category = CATEGORY_SERVER,
+ doc = "Set the minimum response size to trigger dynamic
compression"
+ )
+ private int minGzipSize = 32;
Review Comment:
The name is not linked in any way to the other property. Perhaps
`compressPrometheusMetricsOutputMinResponseSize`?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
