asafm commented on code in PR #22058:
URL: https://github.com/apache/pulsar/pull/22058#discussion_r1497610237
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/namespace/NamespaceService.java:
##########
@@ -175,6 +187,25 @@ public NamespaceService(PulsarService pulsar) {
this.bundleSplitListeners = new CopyOnWriteArrayList<>();
this.localBrokerDataCache =
pulsar.getLocalMetadataStore().getMetadataCache(LocalBrokerData.class);
this.redirectManager = new RedirectManager(pulsar);
+
+ var meter = pulsar.getOpenTelemetry().getMeter();
+ this.lookupRedirectsCounter = meter
+ .counterBuilder("pulsar.broker.lookup.redirect")
+ .setDescription("The number of lookup redirected requests")
+ .build();
+ this.lookupFailuresCounter = meter
+ .counterBuilder("pulsar.broker.lookup.failure")
+ .setDescription("The number of lookup failures")
+ .build();
+ this.lookupAnswersCounter = meter
+ .counterBuilder("pulsar.broker.lookup.answer")
+ .setDescription("The number of lookup responses (i.e. not
redirected requests)")
+ .build();
+ this.lookupLatencyHistogram = meter
+ .histogramBuilder("pulsar.broker.lookup.latency")
Review Comment:
Why? If you use, for example, the `histogram_quantile` function:
`histogram_quantile(pulsar_broker_lookup_request_duration)`, you would get the
quantile of the lookup operation regardless of the type of result or if it was
successful or not. You can decide to have it only for success:
`histogram_quantile(pulsar_broker_lookup_request_duration{response_status="success"})`.
It gives much more flexibility - The slice and dice should happen at the
labels, not the instrument name.
> Regarding the response type attribute names, perhaps broker and redirect
are a better fit? Adding the _url suffix implies that the actual response URL
is stored in the value.
@codelipenghui @lhotari WDYT? Maybe it should be the same as defined in the
protobuf? `connect` and `redirect`?
```
message CommandLookupTopicResponse {
enum LookupType {
Redirect = 0;
Connect = 1;
Failed = 2;
}
optional string brokerServiceUrl = 1; // Optional in case of error
optional string brokerServiceUrlTls = 2;
optional LookupType response = 3;
required uint64 request_id = 4;
optional bool authoritative = 5 [default = false];
```
--
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]
