ctubbsii commented on a change in pull request #2259:
URL: https://github.com/apache/accumulo/pull/2259#discussion_r717926929
##########
File path: server/tracer/pom.xml
##########
@@ -31,95 +31,294 @@
<name>Apache Accumulo Tracer Server</name>
<description>The tracer server for Apache Accumulo to listen for, and store,
distributed tracing messages.</description>
<dependencies>
- <dependency>
- <groupId>com.beust</groupId>
- <artifactId>jcommander</artifactId>
- </dependency>
<dependency>
<groupId>com.google.auto.service</groupId>
<artifactId>auto-service</artifactId>
<optional>true</optional>
</dependency>
<dependency>
- <groupId>com.google.guava</groupId>
- <artifactId>guava</artifactId>
+ <groupId>io.opentelemetry</groupId>
+ <artifactId>opentelemetry-api</artifactId>
+ </dependency>
+ <dependency>
+ <groupId>io.opentelemetry</groupId>
+ <artifactId>opentelemetry-sdk</artifactId>
+ </dependency>
+ <dependency>
+ <groupId>io.opentelemetry</groupId>
+ <artifactId>opentelemetry-sdk-extension-autoconfigure</artifactId>
+ <version>1.5.0-alpha</version>
</dependency>
<dependency>
<groupId>org.apache.accumulo</groupId>
<artifactId>accumulo-core</artifactId>
</dependency>
<dependency>
- <groupId>org.apache.accumulo</groupId>
- <artifactId>accumulo-server-base</artifactId>
+ <groupId>org.apache.commons</groupId>
+ <artifactId>commons-lang3</artifactId>
</dependency>
<dependency>
- <groupId>org.apache.accumulo</groupId>
- <artifactId>accumulo-start</artifactId>
+ <groupId>com.google.protobuf</groupId>
+ <artifactId>protobuf-java-util</artifactId>
+ <version>3.17.2</version>
+ <scope>runtime</scope>
+ </dependency>
+ <dependency>
+ <groupId>com.squareup.okhttp3</groupId>
+ <artifactId>okhttp</artifactId>
+ <version>3.14.9</version>
+ <scope>runtime</scope>
+ </dependency>
+ <dependency>
+ <groupId>com.squareup.okio</groupId>
+ <artifactId>okio</artifactId>
+ <version>1.17.2</version>
+ <scope>runtime</scope>
+ </dependency>
+ <dependency>
+ <groupId>io.grpc</groupId>
+ <artifactId>grpc-api</artifactId>
+ <version>1.38.0</version>
+ <scope>runtime</scope>
+ </dependency>
+ <dependency>
+ <groupId>io.grpc</groupId>
+ <artifactId>grpc-context</artifactId>
+ <version>1.38.0</version>
+ <scope>runtime</scope>
</dependency>
<dependency>
- <groupId>org.apache.hadoop</groupId>
- <artifactId>hadoop-client-api</artifactId>
+ <groupId>io.grpc</groupId>
+ <artifactId>grpc-core</artifactId>
+ <version>1.38.0</version>
+ <scope>runtime</scope>
</dependency>
<dependency>
- <groupId>org.apache.htrace</groupId>
- <artifactId>htrace-core</artifactId>
+ <groupId>io.grpc</groupId>
+ <artifactId>grpc-netty-shaded</artifactId>
+ <version>1.38.0</version>
+ <scope>runtime</scope>
</dependency>
<dependency>
- <groupId>org.apache.thrift</groupId>
- <artifactId>libthrift</artifactId>
+ <groupId>io.grpc</groupId>
+ <artifactId>grpc-protobuf</artifactId>
+ <version>1.38.0</version>
+ <scope>runtime</scope>
</dependency>
<dependency>
- <groupId>org.apache.zookeeper</groupId>
- <artifactId>zookeeper</artifactId>
+ <groupId>io.grpc</groupId>
+ <artifactId>grpc-protobuf-lite</artifactId>
+ <version>1.38.0</version>
+ <scope>runtime</scope>
</dependency>
<dependency>
- <groupId>org.slf4j</groupId>
- <artifactId>slf4j-api</artifactId>
+ <groupId>io.grpc</groupId>
+ <artifactId>grpc-stub</artifactId>
+ <version>1.38.0</version>
+ <scope>runtime</scope>
</dependency>
<dependency>
- <groupId>org.apache.hadoop</groupId>
- <artifactId>hadoop-client-runtime</artifactId>
+ <groupId>io.opentelemetry</groupId>
+ <artifactId>opentelemetry-api-metrics</artifactId>
+ <version>1.5.0-alpha</version>
<scope>runtime</scope>
</dependency>
<dependency>
- <groupId>junit</groupId>
- <artifactId>junit</artifactId>
- <scope>test</scope>
+ <groupId>io.opentelemetry</groupId>
+ <artifactId>opentelemetry-exporter-jaeger</artifactId>
+ <scope>runtime</scope>
</dependency>
<dependency>
- <groupId>org.apache.logging.log4j</groupId>
- <artifactId>log4j-slf4j-impl</artifactId>
- <scope>test</scope>
+ <groupId>io.opentelemetry</groupId>
+ <artifactId>opentelemetry-exporter-logging</artifactId>
+ <scope>runtime</scope>
</dependency>
<dependency>
- <groupId>org.easymock</groupId>
- <artifactId>easymock</artifactId>
- <scope>test</scope>
+ <groupId>io.opentelemetry</groupId>
+ <artifactId>opentelemetry-exporter-otlp</artifactId>
+ <scope>runtime</scope>
+ </dependency>
+ <dependency>
+ <groupId>io.opentelemetry</groupId>
+ <artifactId>opentelemetry-exporter-otlp-common</artifactId>
+ <scope>runtime</scope>
+ </dependency>
+ <dependency>
+ <groupId>io.opentelemetry</groupId>
+ <artifactId>opentelemetry-exporter-otlp-trace</artifactId>
+ <scope>runtime</scope>
+ </dependency>
+ <dependency>
+ <groupId>io.opentelemetry</groupId>
+ <artifactId>opentelemetry-exporter-prometheus</artifactId>
+ <version>1.5.0-alpha</version>
+ <scope>runtime</scope>
+ </dependency>
+ <dependency>
+ <groupId>io.opentelemetry</groupId>
+ <artifactId>opentelemetry-exporter-zipkin</artifactId>
+ <scope>runtime</scope>
+ </dependency>
+ <dependency>
+ <groupId>io.opentelemetry</groupId>
+ <artifactId>opentelemetry-proto</artifactId>
+ <version>1.5.0-alpha</version>
+ <scope>runtime</scope>
+ </dependency>
+ <dependency>
+ <groupId>io.opentelemetry</groupId>
+ <artifactId>opentelemetry-sdk-common</artifactId>
+ <scope>runtime</scope>
+ </dependency>
+ <dependency>
+ <groupId>io.opentelemetry</groupId>
+ <artifactId>opentelemetry-sdk-metrics</artifactId>
+ <version>1.5.0-alpha</version>
+ <scope>runtime</scope>
+ </dependency>
+ <dependency>
+ <groupId>io.opentelemetry</groupId>
+ <artifactId>opentelemetry-sdk-trace</artifactId>
+ <scope>runtime</scope>
+ </dependency>
+ <dependency>
+ <groupId>io.opentelemetry</groupId>
+ <artifactId>opentelemetry-semconv</artifactId>
+ <version>1.5.0-alpha</version>
+ <scope>runtime</scope>
+ </dependency>
+ <dependency>
+ <groupId>io.perfmark</groupId>
+ <artifactId>perfmark-api</artifactId>
+ <version>0.23.0</version>
+ <scope>runtime</scope>
+ </dependency>
+ <dependency>
+ <groupId>io.perfmark</groupId>
+ <artifactId>perfmark-impl</artifactId>
+ <version>0.23.0</version>
+ <scope>runtime</scope>
+ </dependency>
+ <dependency>
+ <groupId>io.prometheus</groupId>
+ <artifactId>simpleclient</artifactId>
+ <version>0.11.0</version>
+ <scope>runtime</scope>
+ </dependency>
+ <dependency>
+ <groupId>io.prometheus</groupId>
+ <artifactId>simpleclient_tracer_common</artifactId>
+ <version>0.11.0</version>
+ <scope>runtime</scope>
+ </dependency>
+ <dependency>
+ <groupId>io.prometheus</groupId>
+ <artifactId>simpleclient_tracer_otel</artifactId>
+ <version>0.11.0</version>
+ <scope>runtime</scope>
+ </dependency>
+ <dependency>
+ <groupId>io.prometheus</groupId>
+ <artifactId>simpleclient_tracer_otel_agent</artifactId>
+ <version>0.11.0</version>
+ <scope>runtime</scope>
+ </dependency>
+ <dependency>
+ <groupId>io.zipkin.reporter2</groupId>
+ <artifactId>zipkin-reporter</artifactId>
+ <version>2.16.3</version>
+ <scope>runtime</scope>
+ </dependency>
+ <dependency>
+ <groupId>io.zipkin.reporter2</groupId>
+ <artifactId>zipkin-sender-okhttp3</artifactId>
+ <version>2.16.3</version>
+ <scope>runtime</scope>
+ </dependency>
+ <dependency>
+ <groupId>io.zipkin.zipkin2</groupId>
+ <artifactId>zipkin</artifactId>
+ <version>2.23.2</version>
+ <scope>runtime</scope>
</dependency>
</dependencies>
- <profiles>
- <profile>
- <id>thrift</id>
- <build>
- <plugins>
- <plugin>
- <groupId>org.codehaus.mojo</groupId>
- <artifactId>exec-maven-plugin</artifactId>
- <executions>
- <execution>
- <id>generate-thrift</id>
- <goals>
- <goal>exec</goal>
- </goals>
- <phase>generate-sources</phase>
- <configuration>
-
<executable>${basedir}/src/main/scripts/generate-thrift.sh</executable>
- </configuration>
- </execution>
- </executions>
- </plugin>
- </plugins>
- </build>
- </profile>
- </profiles>
+ <build>
+ <plugins>
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-shade-plugin</artifactId>
+ <!-- override version to prevent IllegalArgumentException -->
+ <version>3.2.4</version>
Review comment:
Even if we have a separate jar with the auto-configured impl, I don't
think it needs to be shaded. In any case, if we use
`GlobalOpenTelementry.get()` method to conditionally load the the
autoconfigured impl, if it's available, I think we can move our default
provider into our core without adding any dependencies specific to the
autoconfigured impl (they can still be optional). That should work because
`GlobalOpenTelemetry.get()` loads and initializing
`OpenTelemetrySdkAutoConfiguration` dynamically, rather than us specifying the
`OpenTelemetrySdkAutoConfiguration` class directly.
That would allow us to go ahead and drop this entire module/jar, while still
having the option of creating a separate repo to bundle any of the
autoconfigured impl's runtime dependencies. That separate repo would not need
to even reference any Accumulo interfaces. It might be as simple as dropping
the opentelemetry-sdk-extension-autoconfigure jar and its dependencies onto the
class path. We probably wouldn't even need a separate repo.
##########
File path:
core/src/main/java/org/apache/accumulo/core/util/threads/ThreadPools.java
##########
@@ -101,88 +106,105 @@ public static ExecutorService
createExecutorService(final AccumuloConfiguration
switch (p) {
case GENERAL_SIMPLETIMER_THREADPOOL_SIZE:
- return createScheduledExecutorService(conf.getCount(p), "SimpleTimer",
false);
+ return createScheduledExecutorService(conf.getCount(p), "SimpleTimer");
case MANAGER_BULK_THREADPOOL_SIZE:
return createFixedThreadPool(conf.getCount(p),
conf.getTimeInMillis(Property.MANAGER_BULK_THREADPOOL_TIMEOUT),
TimeUnit.MILLISECONDS,
- "bulk import", true);
+ "bulk import");
case MANAGER_RENAME_THREADS:
- return createFixedThreadPool(conf.getCount(p), "bulk move", false);
+ return createFixedThreadPool(conf.getCount(p), "bulk move");
case MANAGER_FATE_THREADPOOL_SIZE:
- return createFixedThreadPool(conf.getCount(p), "Repo Runner", false);
+ return createFixedThreadPool(conf.getCount(p), "Repo Runner");
case MANAGER_STATUS_THREAD_POOL_SIZE:
int threads = conf.getCount(p);
if (threads == 0) {
return createThreadPool(0, Integer.MAX_VALUE, 60L, TimeUnit.SECONDS,
- "GatherTableInformation", new SynchronousQueue<Runnable>(),
OptionalInt.empty(),
- false);
+ "GatherTableInformation", new SynchronousQueue<Runnable>(),
OptionalInt.empty());
} else {
- return createFixedThreadPool(threads, "GatherTableInformation",
false);
+ return createFixedThreadPool(threads, "GatherTableInformation");
}
case TSERV_WORKQ_THREADS:
- return createFixedThreadPool(conf.getCount(p), "distributed work
queue", false);
+ return createFixedThreadPool(conf.getCount(p), "distributed work
queue");
case TSERV_MINC_MAXCONCURRENT:
- return createFixedThreadPool(conf.getCount(p), 0L,
TimeUnit.MILLISECONDS, "minor compactor",
- true);
+ return createFixedThreadPool(conf.getCount(p), 0L,
TimeUnit.MILLISECONDS,
+ "minor compactor");
case TSERV_MIGRATE_MAXCONCURRENT:
return createFixedThreadPool(conf.getCount(p), 0L,
TimeUnit.MILLISECONDS,
- "tablet migration", true);
+ "tablet migration");
case TSERV_ASSIGNMENT_MAXCONCURRENT:
return createFixedThreadPool(conf.getCount(p), 0L,
TimeUnit.MILLISECONDS,
- "tablet assignment", true);
+ "tablet assignment");
case TSERV_SUMMARY_RETRIEVAL_THREADS:
return createThreadPool(conf.getCount(p), conf.getCount(p), 60,
TimeUnit.SECONDS,
- "summary file retriever", true);
+ "summary file retriever");
case TSERV_SUMMARY_REMOTE_THREADS:
return createThreadPool(conf.getCount(p), conf.getCount(p), 60,
TimeUnit.SECONDS,
- "summary remote", true);
+ "summary remote");
case TSERV_SUMMARY_PARTITION_THREADS:
return createThreadPool(conf.getCount(p), conf.getCount(p), 60,
TimeUnit.SECONDS,
- "summary partition", true);
+ "summary partition");
case GC_DELETE_THREADS:
- return createFixedThreadPool(conf.getCount(p), "deleting", false);
+ return createFixedThreadPool(conf.getCount(p), "deleting");
case REPLICATION_WORKER_THREADS:
- return createFixedThreadPool(conf.getCount(p), "replication task",
false);
+ return createFixedThreadPool(conf.getCount(p), "replication task");
default:
throw new RuntimeException("Unhandled thread pool property: " + p);
}
}
- public static ThreadPoolExecutor createFixedThreadPool(int numThreads, final
String name,
- boolean enableTracing) {
- return createFixedThreadPool(numThreads, DEFAULT_TIMEOUT_MILLISECS,
TimeUnit.MILLISECONDS, name,
- enableTracing);
+ public static ThreadPoolExecutor createFixedThreadPool(int numThreads, final
String name) {
+ return createFixedThreadPool(numThreads, DEFAULT_TIMEOUT_MILLISECS,
TimeUnit.MILLISECONDS,
+ name);
}
public static ThreadPoolExecutor createFixedThreadPool(int numThreads, final
String name,
- BlockingQueue<Runnable> queue, boolean enableTracing) {
+ BlockingQueue<Runnable> queue) {
return createThreadPool(numThreads, numThreads, DEFAULT_TIMEOUT_MILLISECS,
- TimeUnit.MILLISECONDS, name, queue, OptionalInt.empty(),
enableTracing);
+ TimeUnit.MILLISECONDS, name, queue, OptionalInt.empty());
}
public static ThreadPoolExecutor createFixedThreadPool(int numThreads, long
timeOut,
- TimeUnit units, final String name, boolean enableTracing) {
+ TimeUnit units, final String name) {
return createThreadPool(numThreads, numThreads, timeOut, units, name,
- new LinkedBlockingQueue<Runnable>(), OptionalInt.empty(),
enableTracing);
+ new LinkedBlockingQueue<Runnable>(), OptionalInt.empty());
}
public static ThreadPoolExecutor createThreadPool(int coreThreads, int
maxThreads, long timeOut,
- TimeUnit units, final String name, boolean enableTracing) {
+ TimeUnit units, final String name) {
return createThreadPool(coreThreads, maxThreads, timeOut, units, name,
- new LinkedBlockingQueue<Runnable>(), OptionalInt.empty(),
enableTracing);
+ new LinkedBlockingQueue<Runnable>(), OptionalInt.empty());
}
public static ThreadPoolExecutor createThreadPool(int coreThreads, int
maxThreads, long timeOut,
- TimeUnit units, final String name, BlockingQueue<Runnable> queue,
OptionalInt priority,
- boolean enableTracing) {
- ThreadPoolExecutor result = null;
- if (enableTracing) {
- result = new TracingThreadPoolExecutor(coreThreads, maxThreads, timeOut,
units, queue,
- new NamedThreadFactory(name, priority));
- } else {
- result = new ThreadPoolExecutor(coreThreads, maxThreads, timeOut, units,
queue,
- new NamedThreadFactory(name, priority));
- }
+ TimeUnit units, final String name, BlockingQueue<Runnable> queue,
OptionalInt priority) {
+ ThreadPoolExecutor result = new ThreadPoolExecutor(coreThreads,
maxThreads, timeOut, units,
+ queue, new NamedThreadFactory(name, priority)) {
+
+ @Override
+ public void execute(Runnable arg0) {
+ super.execute(Context.current().wrap(arg0));
+ }
Review comment:
> However, we have some code that calls methods on the returned
ThreadPoolExecutor object which is unreachable this way.
Well, that sucks. Maybe your impl is good enough for now. Maybe this is an
area where we could contribute an improvement to upstream OpenTelemetry's API
in future?
--
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]
