coderzc commented on code in PR #25306:
URL: https://github.com/apache/pulsar/pull/25306#discussion_r2939860211
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/stats/OpenTelemetryTopicStats.java:
##########
@@ -415,73 +415,94 @@ private void recordMetricsForTopic(Topic topic) {
var persistentTopicMetrics =
persistentTopic.getPersistentTopicMetrics();
var persistentTopicAttributes =
persistentTopic.getTopicAttributes();
+ // For persistent topics, get custom attributes once and reuse
+ var customLabels = persistentTopicAttributes.getCustomAttributes();
+ var attributesWithCustomLabels = persistentTopicAttributes
+ .buildAttributesWithCustomLabels(attributes, customLabels);
var managedLedger = persistentTopic.getManagedLedger();
var managedLedgerStats =
persistentTopic.getManagedLedger().getStats();
- storageCounter.record(managedLedgerStats.getStoredMessagesSize(),
attributes);
-
storageLogicalCounter.record(managedLedgerStats.getStoredMessagesLogicalSize(),
attributes);
-
storageBacklogCounter.record(managedLedger.getEstimatedBacklogSize(),
attributes);
- storageOffloadedCounter.record(managedLedger.getOffloadedSize(),
attributes);
-
storageInCounter.record(managedLedgerStats.getReadEntriesSucceededTotal(),
attributes);
-
storageOutCounter.record(managedLedgerStats.getAddEntrySucceedTotal(),
attributes);
+ storageCounter.record(managedLedgerStats.getStoredMessagesSize(),
attributesWithCustomLabels);
Review Comment:
Thanks, I agree the merge logic should be centralized in
`PersistentTopicAttributes`.
My main concern is overriding `getCommonAttributes()`: today it is a cheap
accessor over pre-built static attributes, while custom labels are dynamic and
require reading topic properties. Hiding that work in a common getter makes the
behavior less obvious.
I kept `getCommonAttributes()` unchanged, but moved the custom-label merge
closer to `PersistentTopicAttributes` by introducing an explicit per-scrape
attribute snapshot there. This keeps the dynamic work out of the common getter
while also removing most of the repetitive merging from
`OpenTelemetryTopicStats`.
--
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]
