asafm commented on PR #16758:
URL: https://github.com/apache/pulsar/pull/16758#issuecomment-1207782630
> Hi @asafm
>
> > As I understand, this only happens once per instance of
MLTransactionLogImpl. So I don't understand why you want to pass metric by
metric. Something doesn't add up here.
> > I guess my main question is: how many MLTransactionLogImpl are there in
a single broker process? More than 1?
>
> Yes, maybe more than one.
Ok, if more than one, then the design must change. I thought the whole idea
was that you have a single instance of metrics per metric prefix.
If not, after much thought I suggest the following:
```java
abstract class BufferMetrics {
protected abstract void observeRecordsPerBatch(int)
protected abstract void incFlushTriggeredByMaxRecords(int)
}
MLTransactionMetadataStoreBufferedWriterMetrics extends BufferMetrics {
static private Histogram recordsPerBatchMetric = new Histogram.Builder()
.name("pulsar_tx_store_bufferedwriter_batch_record_count")
.labelNames(new String[]{"txCoordinatorId"})
.help("Records per batch histogram")
.buckets(RECORD_COUNT_PER_ENTRY_BUCKETS)
.register(registry));
private Histogram.Child recordsPerBatchHistogram;
public MLTransactionMetadataStoreBufferedWriterMetrics(String
txCoordinatorId) {
recordsPerBatchHistogram =
recordsPerBatchHistogram.labels(txCoordinatorId)
}
}
```
The pros:
* It's explicit
* No confusing pass of label names multiple times which after 2nd time are
not really used.
The cons:
* A bit awkward
Another approach which I disliked a bit, but it's still ok:
Add to Pulsar Common:
```java
class PrometheusRegistryChecker {
static defaultMetricRegistryNameToCollector = new HashMap<String,
Collector>()
static Collector registerIfNotExists(collector) {}
}
```
Like `FunctionCollectorRegistryImpl`
--
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]
