mlbiscoc commented on code in PR #3519:
URL: https://github.com/apache/solr/pull/3519#discussion_r2312893354


##########
solr/core/src/java/org/apache/solr/util/stats/OtelInstrumentedExecutorService.java:
##########
@@ -0,0 +1,283 @@
+package org.apache.solr.util.stats;
+
+import static org.apache.solr.metrics.SolrMetricProducer.CATEGORY_ATTR;
+import static org.apache.solr.metrics.SolrMetricProducer.TYPE_ATTR;
+
+import io.opentelemetry.api.common.AttributeKey;
+import io.opentelemetry.api.common.Attributes;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.List;
+import java.util.concurrent.BlockingQueue;
+import java.util.concurrent.Callable;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.ForkJoinPool;
+import java.util.concurrent.Future;
+import java.util.concurrent.ThreadPoolExecutor;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.TimeoutException;
+import org.apache.solr.core.SolrInfoBean;
+import org.apache.solr.metrics.SolrMetricsContext;
+import org.apache.solr.metrics.otel.OtelUnit;
+import org.apache.solr.metrics.otel.instruments.AttributedLongCounter;
+import org.apache.solr.metrics.otel.instruments.AttributedLongTimer;
+import 
org.apache.solr.metrics.otel.instruments.AttributedLongTimer.MetricTimer;
+import org.apache.solr.metrics.otel.instruments.AttributedLongUpDownCounter;
+
+/**
+ * OTEL instrumentation wrapper around {@link ExecutorService}. Based on {@link
+ * com.codahale.metrics.InstrumentedExecutorService}.
+ */
+public class OtelInstrumentedExecutorService implements ExecutorService {
+  public static final AttributeKey<String> NAME_ATTR = 
AttributeKey.stringKey("executor_name");
+
+  private final ExecutorService delegate;
+  private final AttributedLongCounter submitted;
+  private final AttributedLongUpDownCounter running;
+  private final AttributedLongCounter completed;
+  private final AttributedLongTimer idle;
+  private final AttributedLongTimer duration;
+
+  public OtelInstrumentedExecutorService(
+      ExecutorService delegate,
+      SolrMetricsContext ctx,
+      SolrInfoBean.Category category,
+      String executorName) {
+    this.delegate = delegate;
+
+    Attributes attrs =
+        Attributes.builder()
+            .put(CATEGORY_ATTR, category.toString())
+            .put(NAME_ATTR, executorName)
+            .build();
+
+    // Each metric type needs a separate name to avoid obscuring other types
+    this.submitted =
+        new AttributedLongCounter(
+            ctx.longCounter("solr_executor_tasks", "Number of ExecutorService 
tasks"),
+            attrs.toBuilder().put(TYPE_ATTR, "submitted").build());
+    this.completed =
+        new AttributedLongCounter(
+            ctx.longCounter("solr_executor_tasks", "Number of ExecutorService 
tasks"),
+            attrs.toBuilder().put(TYPE_ATTR, "completed").build());

Review Comment:
   So here were [some 
snippets](https://opentelemetry.io/docs/languages/java/api/#counter) from the 
OTEL java documentation. The point of attributed wrappers is so we are not 
populating the classes with shared instrument members and attribute 
initializations and figuring out how to link them together at recording. The 
attributed classes wrap them together for us and make it look very similar to 
Dropwizard. But I did some digging in the OTEL spec and found this:
   
   > If the attribute names and types are provided during the [counter 
creation](https://opentelemetry.io/docs/specs/otel/metrics/api/#counter-creation),
 the [OpenTelemetry 
API](https://opentelemetry.io/docs/specs/otel/overview/#api) authors MAY allow 
attribute values to be passed in using a more efficient way (e.g. strong typed 
struct allocated on the callstack, tuple). The API MUST allow callers to 
provide flexible attributes at invocation time rather than having to register 
all the possible attribute names during the instrument creation.
   
   Attributes MAY be passed but the API MUST have attributes passed at 
invocation time rather than insturment creation. Digging even further I found 
they actually did allow for whats called `Bound` insturments to link attributes 
at creation which looks close to what my attributed wrappers did until they 
removed it in 1.10. See this 
[PR](https://github.com/open-telemetry/opentelemetry-java/pull/3928). So 
attributes at invocation time is the only way you can do it right now.



-- 
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]

Reply via email to