You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by "dragosvictor (via GitHub)" <gi...@apache.org> on 2024/06/18 02:43:58 UTC
[PR] [feat][broker] PIP-264: Add OpenTelemetry broker connection metrics [pulsar]
dragosvictor opened a new pull request, #22931:
URL: https://github.com/apache/pulsar/pull/22931
[PIP-264](https://github.com/apache/pulsar/blob/master/pip/pip-264.md)
### Motivation
Adds broker connection metrics to the OpenTelemetry pipeline.
### Modifications
- Adds the metrics described in https://github.com/apache/pulsar-site/pull/921 to the collector runs.
### Verifying this change
- [x] Make sure that the change passes the CI checks.
This change added tests and can be verified as follows:
- Added test in `org.apache.pulsar.broker.stats.OpenTelemetryBrokerOperabilityStatsTest`
### Does this pull request potentially affect one of the following parts:
- [ ] Dependencies (add or upgrade a dependency)
- [ ] The public API
- [ ] The schema
- [ ] The default values of configurations
- [ ] The threading model
- [ ] The binary protocol
- [ ] The REST endpoints
- [ ] The admin CLI options
- [x] The metrics: As described in https://github.com/apache/pulsar-site/pull/921
- [ ] Anything that affects deployment
### Documentation
- [ ] `doc` <!-- Your PR contains doc changes. -->
- [x] `doc-required` https://github.com/apache/pulsar-site/pull/921
- [ ] `doc-not-needed` <!-- Your PR changes do not impact docs -->
- [ ] `doc-complete` <!-- Docs have been already added -->
### Matching PR in forked repository
PR in forked repository: https://github.com/dragosvictor/pulsar/pull/28
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [feat][broker] PIP-264: Add OpenTelemetry broker connection metrics [pulsar]
Posted by "dragosvictor (via GitHub)" <gi...@apache.org>.
dragosvictor commented on code in PR #22931:
URL: https://github.com/apache/pulsar/pull/22931#discussion_r1644879996
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/stats/BrokerOperabilityMetrics.java:
##########
@@ -18,39 +18,80 @@
*/
package org.apache.pulsar.broker.stats;
+import io.opentelemetry.api.metrics.ObservableLongCounter;
import io.prometheus.client.Counter;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.LongAdder;
+import org.apache.pulsar.broker.PulsarService;
import org.apache.pulsar.common.stats.Metrics;
+import org.apache.pulsar.opentelemetry.OpenTelemetryAttributes.ConnectionCreateStatus;
+import org.apache.pulsar.opentelemetry.OpenTelemetryAttributes.ConnectionStatus;
/**
*/
-public class BrokerOperabilityMetrics {
+public class BrokerOperabilityMetrics implements AutoCloseable {
private static final Counter TOPIC_LOAD_FAILED = Counter.build("topic_load_failed", "-").register();
private final List<Metrics> metricsList;
private final String localCluster;
private final DimensionStats topicLoadStats;
private final String brokerName;
private final LongAdder connectionTotalCreatedCount;
- private final LongAdder connectionCreateSuccessCount;
- private final LongAdder connectionCreateFailCount;
private final LongAdder connectionTotalClosedCount;
private final LongAdder connectionActive;
- public BrokerOperabilityMetrics(String localCluster, String brokerName) {
+ private final LongAdder connectionCreateSuccessCount;
+ private final LongAdder connectionCreateFailCount;
+
+ public static final String CONNECTION_COUNTER_METRIC_NAME = "pulsar.broker.connection.count";
+ private final ObservableLongCounter connectionCounter;
+
+ public static final String CONNECTION_CREATE_COUNTER_METRIC_NAME =
+ "pulsar.broker.connection.create.operation.count";
+ private final ObservableLongCounter connectionCreateCounter;
+
+ public BrokerOperabilityMetrics(PulsarService pulsar) {
this.metricsList = new ArrayList<>();
- this.localCluster = localCluster;
+ this.localCluster = pulsar.getConfiguration().getClusterName();
this.topicLoadStats = new DimensionStats("pulsar_topic_load_times", 60);
- this.brokerName = brokerName;
+ this.brokerName = pulsar.getAdvertisedAddress();
Review Comment:
Note that the old Prometheus metrics are not changed here, this is just a refactoring.
Regarding the broker ID: agreed. OpenTelemetry automatically includes host information as attributes (hostname, for instance). It can be used most of the time as a proxy for the broker ID, but would not work in the unlikely situation that multiple instances are running on the same machine.
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [feat][broker] PIP-264: Add OpenTelemetry broker connection metrics [pulsar]
Posted by "merlimat (via GitHub)" <gi...@apache.org>.
merlimat merged PR #22931:
URL: https://github.com/apache/pulsar/pull/22931
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [feat][broker] PIP-264: Add OpenTelemetry broker connection metrics [pulsar]
Posted by "dao-jun (via GitHub)" <gi...@apache.org>.
dao-jun closed pull request #22931: [feat][broker] PIP-264: Add OpenTelemetry broker connection metrics
URL: https://github.com/apache/pulsar/pull/22931
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [feat][broker] PIP-264: Add OpenTelemetry broker connection metrics [pulsar]
Posted by "heesung-sn (via GitHub)" <gi...@apache.org>.
heesung-sn commented on code in PR #22931:
URL: https://github.com/apache/pulsar/pull/22931#discussion_r1644840123
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/stats/BrokerOperabilityMetrics.java:
##########
@@ -18,39 +18,80 @@
*/
package org.apache.pulsar.broker.stats;
+import io.opentelemetry.api.metrics.ObservableLongCounter;
import io.prometheus.client.Counter;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.LongAdder;
+import org.apache.pulsar.broker.PulsarService;
import org.apache.pulsar.common.stats.Metrics;
+import org.apache.pulsar.opentelemetry.OpenTelemetryAttributes.ConnectionCreateStatus;
+import org.apache.pulsar.opentelemetry.OpenTelemetryAttributes.ConnectionStatus;
/**
*/
-public class BrokerOperabilityMetrics {
+public class BrokerOperabilityMetrics implements AutoCloseable {
private static final Counter TOPIC_LOAD_FAILED = Counter.build("topic_load_failed", "-").register();
private final List<Metrics> metricsList;
private final String localCluster;
private final DimensionStats topicLoadStats;
private final String brokerName;
private final LongAdder connectionTotalCreatedCount;
- private final LongAdder connectionCreateSuccessCount;
- private final LongAdder connectionCreateFailCount;
private final LongAdder connectionTotalClosedCount;
private final LongAdder connectionActive;
- public BrokerOperabilityMetrics(String localCluster, String brokerName) {
+ private final LongAdder connectionCreateSuccessCount;
+ private final LongAdder connectionCreateFailCount;
+
+ public static final String CONNECTION_COUNTER_METRIC_NAME = "pulsar.broker.connection.count";
+ private final ObservableLongCounter connectionCounter;
+
+ public static final String CONNECTION_CREATE_COUNTER_METRIC_NAME =
+ "pulsar.broker.connection.create.operation.count";
+ private final ObservableLongCounter connectionCreateCounter;
+
+ public BrokerOperabilityMetrics(PulsarService pulsar) {
this.metricsList = new ArrayList<>();
- this.localCluster = localCluster;
+ this.localCluster = pulsar.getConfiguration().getClusterName();
this.topicLoadStats = new DimensionStats("pulsar_topic_load_times", 60);
- this.brokerName = brokerName;
+ this.brokerName = pulsar.getAdvertisedAddress();
Review Comment:
nit: we probably need to think about using brokerId for the metrics, but I think that would require a pip discussion.
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org