mlbiscoc commented on code in PR #3453:
URL: https://github.com/apache/solr/pull/3453#discussion_r2263833115
##########
solr/core/src/java/org/apache/solr/core/ZkContainer.java:
##########
@@ -141,18 +140,82 @@ public void initZooKeeper(final CoreContainer cc,
CloudConfig config) {
}
this.zkController = zkController;
- MetricsMap metricsMap = new
MetricsMap(zkController.getZkClient().getMetrics());
+
metricProducer =
new SolrMetricProducer() {
SolrMetricsContext ctx;
- // TODO SOLR-17458: Migrate to Otel
@Override
public void initializeMetrics(
SolrMetricsContext parentContext, Attributes attributes,
String scope) {
ctx = parentContext.getChildContext(this);
- ctx.gauge(
- metricsMap, true, scope, null,
SolrInfoBean.Category.CONTAINER.toString());
+
+ var metricsListener = zkController.getZkClient().getMetrics();
+
+ ctx.observableLongCounter(
+ "solr_zk_ops",
+ "Total number of ZooKeeper operations",
+ measurement -> {
+ measurement.record(
+ metricsListener.getReads(),
+ attributes.toBuilder().put(OPERATION_ATTR,
"read").build());
+ measurement.record(
+ metricsListener.getDeletes(),
+ attributes.toBuilder().put(OPERATION_ATTR,
"delete").build());
+ measurement.record(
+ metricsListener.getWrites(),
+ attributes.toBuilder().put(OPERATION_ATTR,
"write").build());
+ measurement.record(
+ metricsListener.getMultiOps(),
+ attributes.toBuilder().put(OPERATION_ATTR,
"multi").build());
+ measurement.record(
+ metricsListener.getExistsChecks(),
+ attributes.toBuilder().put(OPERATION_ATTR,
"exists").build());
+ });
+
+ ctx.observableLongCounter(
+ "solr_zk_bytes_read",
+ "Total bytes read from ZooKeeper",
+ measurement -> {
+ measurement.record(metricsListener.getBytesRead(),
attributes);
+ },
+ "By");
+
+ ctx.observableLongCounter(
+ "solr_zk_watches_fired",
+ "Total number of ZooKeeper watches fired",
+ measurement -> {
+ measurement.record(metricsListener.getWatchesFired(),
attributes);
+ });
+
+ ctx.observableLongCounter(
+ "solr_zk_bytes_written_total",
Review Comment:
I fixed it and updated the PR description with the correct naming in
[ee09699](https://github.com/apache/solr/pull/3453/commits/ee09699eddd7a35ef2811939ced9a821b7ee22af).
Something I think isn't obvious though is how OTELs prometheus metric reader
names things after and how OTEL sets units as it's metadata.
For example, I just fixed up the metric for zk bytes read to just name
`solr_zk_read` but the prometheus output does `solr_zk_read_bytes_total`. Thats
because the metric reader does some sanitizing and tries to follow prometheus
data model and format with the unit metadata having `byte` and counters always
ending with `total` even though I didn't add it.
What I kind of regret of regret was the way I setup the `unit` parameter
because you can see I set bytes as "By". Thats because OTEL only follows
[UCUM](https://ucum.org/ucum) for its units otherwise the metric throws an
exception. I am going to go back with another PR to fix the units so that devs
can just use a common set of constants UCUM units instead of passing in a
string.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
