From f08c483a409d1a464e259f5480f60b80e2892d58 Mon Sep 17 00:00:00 2001 From: David Turner Date: Wed, 17 Nov 2021 15:29:47 +0000 Subject: [PATCH 1/2] Remove unused TracingPlugin interface --- .../java/org/elasticsearch/node/Node.java | 6 ++---- .../java/org/elasticsearch/tasks/Task.java | 3 +-- .../org/elasticsearch/tasks/TaskTracer.java | 9 ++++----- .../Traceable.java} | 19 +++++-------------- .../java/org/elasticsearch/tasks/Tracer.java | 15 +++++++++++++++ .../org/elasticsearch/xpack/apm/ApmIT.java | 3 +-- .../java/org/elasticsearch/xpack/apm/APM.java | 3 +-- .../elasticsearch/xpack/apm/APMTracer.java | 8 ++++---- 8 files changed, 33 insertions(+), 33 deletions(-) rename server/src/main/java/org/elasticsearch/{plugins/TracingPlugin.java => tasks/Traceable.java} (53%) create mode 100644 server/src/main/java/org/elasticsearch/tasks/Tracer.java diff --git a/server/src/main/java/org/elasticsearch/node/Node.java b/server/src/main/java/org/elasticsearch/node/Node.java index 792d52e6daf2b..165353e777ad3 100644 --- a/server/src/main/java/org/elasticsearch/node/Node.java +++ b/server/src/main/java/org/elasticsearch/node/Node.java @@ -147,7 +147,6 @@ import org.elasticsearch.plugins.SearchPlugin; import org.elasticsearch.plugins.ShutdownAwarePlugin; import org.elasticsearch.plugins.SystemIndexPlugin; -import org.elasticsearch.plugins.TracingPlugin; import org.elasticsearch.repositories.RepositoriesModule; import org.elasticsearch.repositories.RepositoriesService; import org.elasticsearch.rest.RestController; @@ -169,6 +168,7 @@ import org.elasticsearch.tasks.TaskCancellationService; import org.elasticsearch.tasks.TaskResultsService; import org.elasticsearch.tasks.TaskTracer; +import org.elasticsearch.tasks.Tracer; import org.elasticsearch.threadpool.ExecutorBuilder; import org.elasticsearch.threadpool.ThreadPool; import org.elasticsearch.transport.Transport; @@ -753,9 +753,7 @@ protected Node( final IndexingPressure indexingLimits = new IndexingPressure(settings); final TaskTracer taskTracer = transportService.getTaskManager().getTaskTracer(); - pluginComponents.stream() - .map(c -> c instanceof TracingPlugin.Tracer ? (TracingPlugin.Tracer) c : null) - .forEach(taskTracer::addTracer); + pluginComponents.stream().map(c -> c instanceof Tracer ? (Tracer) c : null).forEach(taskTracer::addTracer); final RecoverySettings recoverySettings = new RecoverySettings(settings, settingsModule.getClusterSettings()); RepositoriesModule repositoriesModule = new RepositoriesModule( diff --git a/server/src/main/java/org/elasticsearch/tasks/Task.java b/server/src/main/java/org/elasticsearch/tasks/Task.java index 152569521862d..2e21173b2c71e 100644 --- a/server/src/main/java/org/elasticsearch/tasks/Task.java +++ b/server/src/main/java/org/elasticsearch/tasks/Task.java @@ -11,7 +11,6 @@ import org.elasticsearch.action.ActionResponse; import org.elasticsearch.cluster.node.DiscoveryNode; import org.elasticsearch.common.io.stream.NamedWriteable; -import org.elasticsearch.plugins.TracingPlugin; import org.elasticsearch.xcontent.ToXContent; import org.elasticsearch.xcontent.ToXContentObject; @@ -21,7 +20,7 @@ /** * Current task information */ -public class Task implements TracingPlugin.Traceable { +public class Task implements Traceable { /** * The request header to mark tasks with specific ids diff --git a/server/src/main/java/org/elasticsearch/tasks/TaskTracer.java b/server/src/main/java/org/elasticsearch/tasks/TaskTracer.java index 23c1f12d9529b..9a03a7fd20509 100644 --- a/server/src/main/java/org/elasticsearch/tasks/TaskTracer.java +++ b/server/src/main/java/org/elasticsearch/tasks/TaskTracer.java @@ -11,7 +11,6 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.apache.logging.log4j.message.ParameterizedMessage; -import org.elasticsearch.plugins.TracingPlugin; import java.util.List; import java.util.concurrent.CopyOnWriteArrayList; @@ -20,16 +19,16 @@ public class TaskTracer { private static final Logger logger = LogManager.getLogger(); - private final List tracers = new CopyOnWriteArrayList<>(); + private final List tracers = new CopyOnWriteArrayList<>(); - public void addTracer(TracingPlugin.Tracer tracer) { + public void addTracer(Tracer tracer) { if (tracer != null) { tracers.add(tracer); } } public void onTaskRegistered(Task task) { - for (TracingPlugin.Tracer tracer : tracers) { + for (Tracer tracer : tracers) { try { tracer.onTraceStarted(task); } catch (Exception e) { @@ -48,7 +47,7 @@ public void onTaskRegistered(Task task) { } public void onTaskUnregistered(Task task) { - for (TracingPlugin.Tracer tracer : tracers) { + for (Tracer tracer : tracers) { try { tracer.onTraceStopped(task); } catch (Exception e) { diff --git a/server/src/main/java/org/elasticsearch/plugins/TracingPlugin.java b/server/src/main/java/org/elasticsearch/tasks/Traceable.java similarity index 53% rename from server/src/main/java/org/elasticsearch/plugins/TracingPlugin.java rename to server/src/main/java/org/elasticsearch/tasks/Traceable.java index 02b3cf11b28bd..8bff54988e6cb 100644 --- a/server/src/main/java/org/elasticsearch/plugins/TracingPlugin.java +++ b/server/src/main/java/org/elasticsearch/tasks/Traceable.java @@ -6,23 +6,14 @@ * Side Public License, v 1. */ -package org.elasticsearch.plugins; +package org.elasticsearch.tasks; import java.util.Map; -public interface TracingPlugin { +public interface Traceable { + String getSpanId(); - interface Traceable { - String getSpanId(); + String getSpanName(); - String getSpanName(); - - Map getAttributes(); - } - - interface Tracer { - void onTraceStarted(Traceable traceable); - - void onTraceStopped(Traceable traceable); - } + Map getAttributes(); } diff --git a/server/src/main/java/org/elasticsearch/tasks/Tracer.java b/server/src/main/java/org/elasticsearch/tasks/Tracer.java new file mode 100644 index 0000000000000..31896f1f29b04 --- /dev/null +++ b/server/src/main/java/org/elasticsearch/tasks/Tracer.java @@ -0,0 +1,15 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +package org.elasticsearch.tasks; + +public interface Tracer { + void onTraceStarted(Traceable traceable); + + void onTraceStopped(Traceable traceable); +} diff --git a/x-pack/plugin/apm-integration/src/internalClusterTest/java/org/elasticsearch/xpack/apm/ApmIT.java b/x-pack/plugin/apm-integration/src/internalClusterTest/java/org/elasticsearch/xpack/apm/ApmIT.java index 16726aa5139c7..78b5077a97d8e 100644 --- a/x-pack/plugin/apm-integration/src/internalClusterTest/java/org/elasticsearch/xpack/apm/ApmIT.java +++ b/x-pack/plugin/apm-integration/src/internalClusterTest/java/org/elasticsearch/xpack/apm/ApmIT.java @@ -15,7 +15,6 @@ import org.elasticsearch.common.util.CollectionUtils; import org.elasticsearch.plugins.Plugin; import org.elasticsearch.plugins.PluginsService; -import org.elasticsearch.plugins.TracingPlugin; import org.elasticsearch.tasks.Task; import org.elasticsearch.tasks.TaskId; import org.elasticsearch.tasks.TaskTracer; @@ -53,7 +52,7 @@ public void clearRecordedSpans() { } public void testModule() { - List plugins = internalCluster().getMasterNodeInstance(PluginsService.class).filterPlugins(TracingPlugin.class); + List plugins = internalCluster().getMasterNodeInstance(PluginsService.class).filterPlugins(APM.class); assertThat(plugins, hasSize(1)); TransportService transportService = internalCluster().getInstance(TransportService.class); diff --git a/x-pack/plugin/apm-integration/src/main/java/org/elasticsearch/xpack/apm/APM.java b/x-pack/plugin/apm-integration/src/main/java/org/elasticsearch/xpack/apm/APM.java index fd7f2ea57e493..64227153d4341 100644 --- a/x-pack/plugin/apm-integration/src/main/java/org/elasticsearch/xpack/apm/APM.java +++ b/x-pack/plugin/apm-integration/src/main/java/org/elasticsearch/xpack/apm/APM.java @@ -19,7 +19,6 @@ import org.elasticsearch.env.NodeEnvironment; import org.elasticsearch.plugins.NetworkPlugin; import org.elasticsearch.plugins.Plugin; -import org.elasticsearch.plugins.TracingPlugin; import org.elasticsearch.repositories.RepositoriesService; import org.elasticsearch.script.ScriptService; import org.elasticsearch.tasks.Task; @@ -38,7 +37,7 @@ import java.util.Set; import java.util.function.Supplier; -public class APM extends Plugin implements TracingPlugin, NetworkPlugin { +public class APM extends Plugin implements NetworkPlugin { public static final Set TRACE_HEADERS = Set.of(Task.TRACE_PARENT, Task.TRACE_STATE); diff --git a/x-pack/plugin/apm-integration/src/main/java/org/elasticsearch/xpack/apm/APMTracer.java b/x-pack/plugin/apm-integration/src/main/java/org/elasticsearch/xpack/apm/APMTracer.java index cc8586c0fa5c2..235608856d906 100644 --- a/x-pack/plugin/apm-integration/src/main/java/org/elasticsearch/xpack/apm/APMTracer.java +++ b/x-pack/plugin/apm-integration/src/main/java/org/elasticsearch/xpack/apm/APMTracer.java @@ -38,8 +38,8 @@ import org.elasticsearch.common.settings.Setting; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.util.concurrent.ConcurrentCollections; -import org.elasticsearch.plugins.TracingPlugin; import org.elasticsearch.tasks.Task; +import org.elasticsearch.tasks.Traceable; import org.elasticsearch.threadpool.ThreadPool; import java.security.AccessController; @@ -55,7 +55,7 @@ import java.util.stream.Collectors; import java.util.stream.Stream; -public class APMTracer extends AbstractLifecycleComponent implements TracingPlugin.Tracer { +public class APMTracer extends AbstractLifecycleComponent implements org.elasticsearch.tasks.Tracer { public static final CapturingSpanExporter CAPTURING_SPAN_EXPORTER = new CapturingSpanExporter(); @@ -122,7 +122,7 @@ protected void doStop() { protected void doClose() {} @Override - public void onTraceStarted(TracingPlugin.Traceable traceable) { + public void onTraceStarted(Traceable traceable) { final Tracer tracer = this.tracer; final OpenTelemetry openTelemetry = this.openTelemetry; if (openTelemetry != null && tracer != null) { @@ -190,7 +190,7 @@ public Map getSpanHeadersById(String id) { } @Override - public void onTraceStopped(TracingPlugin.Traceable traceable) { + public void onTraceStopped(Traceable traceable) { final Span span = spans.remove(traceable.getSpanId()); if (span != null) { span.end(); From f73b052ff25963590c329f719d778333f2eee503 Mon Sep 17 00:00:00 2001 From: David Turner Date: Thu, 18 Nov 2021 10:05:18 +0000 Subject: [PATCH 2/2] Move to tracing package --- .../main/java/org/elasticsearch/node/Node.java | 2 +- .../main/java/org/elasticsearch/tasks/Task.java | 1 + .../java/org/elasticsearch/tasks/TaskTracer.java | 1 + .../{tasks => tracing}/Traceable.java | 15 ++++++++++++++- .../elasticsearch/{tasks => tracing}/Tracer.java | 12 +++++++++++- .../org/elasticsearch/xpack/apm/APMTracer.java | 4 ++-- 6 files changed, 30 insertions(+), 5 deletions(-) rename server/src/main/java/org/elasticsearch/{tasks => tracing}/Traceable.java (57%) rename server/src/main/java/org/elasticsearch/{tasks => tracing}/Tracer.java (60%) diff --git a/server/src/main/java/org/elasticsearch/node/Node.java b/server/src/main/java/org/elasticsearch/node/Node.java index e47ee1798d663..d3f75d38d2cae 100644 --- a/server/src/main/java/org/elasticsearch/node/Node.java +++ b/server/src/main/java/org/elasticsearch/node/Node.java @@ -168,9 +168,9 @@ import org.elasticsearch.tasks.TaskCancellationService; import org.elasticsearch.tasks.TaskResultsService; import org.elasticsearch.tasks.TaskTracer; -import org.elasticsearch.tasks.Tracer; import org.elasticsearch.threadpool.ExecutorBuilder; import org.elasticsearch.threadpool.ThreadPool; +import org.elasticsearch.tracing.Tracer; import org.elasticsearch.transport.Transport; import org.elasticsearch.transport.TransportInterceptor; import org.elasticsearch.transport.TransportService; diff --git a/server/src/main/java/org/elasticsearch/tasks/Task.java b/server/src/main/java/org/elasticsearch/tasks/Task.java index 2e21173b2c71e..e9b6f8500cb38 100644 --- a/server/src/main/java/org/elasticsearch/tasks/Task.java +++ b/server/src/main/java/org/elasticsearch/tasks/Task.java @@ -11,6 +11,7 @@ import org.elasticsearch.action.ActionResponse; import org.elasticsearch.cluster.node.DiscoveryNode; import org.elasticsearch.common.io.stream.NamedWriteable; +import org.elasticsearch.tracing.Traceable; import org.elasticsearch.xcontent.ToXContent; import org.elasticsearch.xcontent.ToXContentObject; diff --git a/server/src/main/java/org/elasticsearch/tasks/TaskTracer.java b/server/src/main/java/org/elasticsearch/tasks/TaskTracer.java index 9a03a7fd20509..ec631b369a9e9 100644 --- a/server/src/main/java/org/elasticsearch/tasks/TaskTracer.java +++ b/server/src/main/java/org/elasticsearch/tasks/TaskTracer.java @@ -11,6 +11,7 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.apache.logging.log4j.message.ParameterizedMessage; +import org.elasticsearch.tracing.Tracer; import java.util.List; import java.util.concurrent.CopyOnWriteArrayList; diff --git a/server/src/main/java/org/elasticsearch/tasks/Traceable.java b/server/src/main/java/org/elasticsearch/tracing/Traceable.java similarity index 57% rename from server/src/main/java/org/elasticsearch/tasks/Traceable.java rename to server/src/main/java/org/elasticsearch/tracing/Traceable.java index 8bff54988e6cb..40ad2b92001e2 100644 --- a/server/src/main/java/org/elasticsearch/tasks/Traceable.java +++ b/server/src/main/java/org/elasticsearch/tracing/Traceable.java @@ -6,14 +6,27 @@ * Side Public License, v 1. */ -package org.elasticsearch.tasks; +package org.elasticsearch.tracing; import java.util.Map; +/** + * Something which maps onto a span in a distributed trace. + */ public interface Traceable { + + /** + * @return a key which uniquely identifies the span. + */ String getSpanId(); + /** + * @return the name of the span as seen by the external tracing system (e.g. the action name for a task) + */ String getSpanName(); + /** + * @return extra metadata about the span. + */ Map getAttributes(); } diff --git a/server/src/main/java/org/elasticsearch/tasks/Tracer.java b/server/src/main/java/org/elasticsearch/tracing/Tracer.java similarity index 60% rename from server/src/main/java/org/elasticsearch/tasks/Tracer.java rename to server/src/main/java/org/elasticsearch/tracing/Tracer.java index 31896f1f29b04..dd4454b8e6b62 100644 --- a/server/src/main/java/org/elasticsearch/tasks/Tracer.java +++ b/server/src/main/java/org/elasticsearch/tracing/Tracer.java @@ -6,10 +6,20 @@ * Side Public License, v 1. */ -package org.elasticsearch.tasks; +package org.elasticsearch.tracing; +/** + * Represents a distributed tracing system that keeps track of the start and end of various activities in the cluster. + */ public interface Tracer { + + /** + * Called when the {@link Traceable} activity starts. + */ void onTraceStarted(Traceable traceable); + /** + * Called when the {@link Traceable} activity ends. + */ void onTraceStopped(Traceable traceable); } diff --git a/x-pack/plugin/apm-integration/src/main/java/org/elasticsearch/xpack/apm/APMTracer.java b/x-pack/plugin/apm-integration/src/main/java/org/elasticsearch/xpack/apm/APMTracer.java index fbdbb0ec86db8..2846daa2c9fbb 100644 --- a/x-pack/plugin/apm-integration/src/main/java/org/elasticsearch/xpack/apm/APMTracer.java +++ b/x-pack/plugin/apm-integration/src/main/java/org/elasticsearch/xpack/apm/APMTracer.java @@ -39,8 +39,8 @@ import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.util.concurrent.ConcurrentCollections; import org.elasticsearch.tasks.Task; -import org.elasticsearch.tasks.Traceable; import org.elasticsearch.threadpool.ThreadPool; +import org.elasticsearch.tracing.Traceable; import java.security.AccessController; import java.security.PrivilegedAction; @@ -55,7 +55,7 @@ import java.util.stream.Collectors; import java.util.stream.Stream; -public class APMTracer extends AbstractLifecycleComponent implements org.elasticsearch.tasks.Tracer { +public class APMTracer extends AbstractLifecycleComponent implements org.elasticsearch.tracing.Tracer { public static final CapturingSpanExporter CAPTURING_SPAN_EXPORTER = new CapturingSpanExporter();