Merge branch 'main' into esql/fix_131028

Author: luigidellaquila

docs/changelog/130427.yaml

@@ -1,5 +1,17 @@
 pr: 130427
-summary: Disallow brackets in unquoted index pattersn
+summary: Disallow brackets in unquoted index patterns
 area: ES|QL
-type: bug
-issues: []
+type: breaking
+issues:
+  - 130378
+breaking:
+  title: Unquoted index patterns do not allow `(` and `)` characters
+  area: ES|QL
+  details: >-
+    Previously, ES|QL accepted unquoted index patterns containing brackets, such as `FROM index(1) | ENRICH policy(2)`.
+
+    This query syntax is no longer valid because it could conflict with subquery syntax, where brackets are used as delimiters.
+
+    Brackets are now only allowed in quoted index patterns. For example: `FROM "index(1)" | ENRICH "policy(2)"`.
+  impact: "This affects existing queries containing brackets in index or policy names, i.e. in FROM, ENRICH, and LOOKUP JOIN commands."
+  notable: false

modules/reindex/src/test/java/org/elasticsearch/reindex/ReindexBasicTests.java

@@ -23,6 +23,7 @@
 import java.util.Map;
 import java.util.stream.Collectors;
 
+import static org.elasticsearch.index.IndexSettings.SYNTHETIC_VECTORS;
 import static org.elasticsearch.index.query.QueryBuilders.termQuery;
 import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked;
 import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertHitCount;
@@ -181,6 +182,7 @@ public void testReindexFromComplexDateMathIndexName() throws Exception {
     }
 
     public void testReindexIncludeVectors() throws Exception {
+        assumeTrue("This test requires synthetic vectors to be enabled", SYNTHETIC_VECTORS);
         var resp1 = prepareCreate("test").setSettings(
             Settings.builder().put(IndexSettings.INDEX_MAPPING_SOURCE_SYNTHETIC_VECTORS_SETTING.getKey(), true).build()
         ).setMapping("foo", "type=dense_vector,similarity=l2_norm", "bar", "type=sparse_vector").get();

modules/reindex/src/test/java/org/elasticsearch/reindex/UpdateByQueryBasicTests.java

@@ -24,6 +24,7 @@
 import java.util.Map;
 import java.util.stream.Collectors;
 
+import static org.elasticsearch.index.IndexSettings.SYNTHETIC_VECTORS;
 import static org.elasticsearch.index.query.QueryBuilders.termQuery;
 import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked;
 import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertHitCount;
@@ -157,6 +158,7 @@ public void testMissingSources() {
     }
 
     public void testUpdateByQueryIncludeVectors() throws Exception {
+        assumeTrue("This test requires synthetic vectors to be enabled", SYNTHETIC_VECTORS);
         var resp1 = prepareCreate("test").setSettings(
             Settings.builder().put(IndexSettings.INDEX_MAPPING_SOURCE_SYNTHETIC_VECTORS_SETTING.getKey(), true).build()
         ).setMapping("foo", "type=dense_vector,similarity=l2_norm", "bar", "type=sparse_vector").get();

server/src/main/java/org/elasticsearch/tasks/TaskManager.java

@@ -170,21 +170,33 @@ public Task register(String type, String action, TaskAwareRequest request, boole
             Task previousTask = tasks.put(task.getId(), task);
             assert previousTask == null;
             if (traceRequest) {
-                startTrace(threadContext, task);
+                maybeStartTrace(threadContext, task);
             }
         }
         return task;
     }
 
-    // package private for testing
-    void startTrace(ThreadContext threadContext, Task task) {
+    /**
+     * Start a new trace span if a parent trace context already exists.
+     * For REST actions this will be the case, otherwise {@link Tracer#startTrace} can be used.
+     */
+    void maybeStartTrace(ThreadContext threadContext, Task task) {
+        if (threadContext.hasParentTraceContext() == false) {
+            return;
+        }
         TaskId parentTask = task.getParentTaskId();
         Map<String, Object> attributes = parentTask.isSet()
             ? Map.of(Tracer.AttributeKeys.TASK_ID, task.getId(), Tracer.AttributeKeys.PARENT_TASK_ID, parentTask.toString())
             : Map.of(Tracer.AttributeKeys.TASK_ID, task.getId());
         tracer.startTrace(threadContext, task, task.getAction(), attributes);
     }
 
+    void maybeStopTrace(ThreadContext threadContext, Task task) {
+        if (threadContext.hasTraceContext()) {
+            tracer.stopTrace(task);
+        }
+    }
+
     public <Request extends ActionRequest, Response extends ActionResponse> Task registerAndExecute(
         String type,
         TransportAction<Request, Response> action,
@@ -247,7 +259,7 @@ private void registerCancellableTask(Task task, long requestId, boolean traceReq
         CancellableTaskHolder holder = new CancellableTaskHolder(cancellableTask);
         cancellableTasks.put(task, requestId, holder);
         if (traceRequest) {
-            startTrace(threadPool.getThreadContext(), task);
+            maybeStartTrace(threadPool.getThreadContext(), task);
         }
         // Check if this task was banned before we start it.
         if (task.getParentTaskId().isSet()) {
@@ -346,7 +358,7 @@ public Task unregister(Task task) {
                 return removedTask;
             }
         } finally {
-            tracer.stopTrace(task);
+            maybeStopTrace(threadPool.getThreadContext(), task);
             for (RemovedTaskListener listener : removedTaskListeners) {
                 listener.onRemoved(task);
             }

server/src/test/java/org/elasticsearch/tasks/TaskManagerTests.java

@@ -68,6 +68,7 @@
 import static org.mockito.Mockito.spy;
 import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.verifyNoInteractions;
 import static org.mockito.Mockito.when;
 
 public class TaskManagerTests extends ESTestCase {
@@ -281,13 +282,72 @@ public void testTaskAccounting() {
     /**
      * Check that registering a task also causes tracing to be started on that task.
      */
-    public void testRegisterTaskStartsTracing() {
+    public void testRegisterTaskStartsTracingIfTraceParentExists() {
         final Tracer mockTracer = mock(Tracer.class);
         final TaskManager taskManager = new TaskManager(Settings.EMPTY, threadPool, Set.of(), mockTracer);
 
+        // fake a trace parent
+        threadPool.getThreadContext().putHeader(Task.TRACE_PARENT_HTTP_HEADER, "traceparent");
         final boolean hasParentTask = randomBoolean();
         final TaskId parentTask = hasParentTask ? new TaskId("parentNode", 1) : TaskId.EMPTY_TASK_ID;
 
+        try (var ignored = threadPool.getThreadContext().newTraceContext()) {
+
+            final Task task = taskManager.register("testType", "testAction", new TaskAwareRequest() {
+
+                @Override
+                public void setParentTask(TaskId taskId) {}
+
+                @Override
+                public void setRequestId(long requestId) {}
+
+                @Override
+                public TaskId getParentTask() {
+                    return parentTask;
+                }
+            });
+
+            Map<String, Object> attributes = hasParentTask
+                ? Map.of(Tracer.AttributeKeys.TASK_ID, task.getId(), Tracer.AttributeKeys.PARENT_TASK_ID, parentTask.toString())
+                : Map.of(Tracer.AttributeKeys.TASK_ID, task.getId());
+            verify(mockTracer).startTrace(any(), eq(task), eq("testAction"), eq(attributes));
+        }
+    }
+
+    /**
+     * Check that registering a task also causes tracing to be started on that task.
+     */
+    public void testRegisterTaskSkipsTracingIfTraceParentMissing() {
+        final Tracer mockTracer = mock(Tracer.class);
+        final TaskManager taskManager = new TaskManager(Settings.EMPTY, threadPool, Set.of(), mockTracer);
+
+        // no trace parent
+        try (var ignored = threadPool.getThreadContext().newTraceContext()) {
+            final Task task = taskManager.register("testType", "testAction", new TaskAwareRequest() {
+
+                @Override
+                public void setParentTask(TaskId taskId) {}
+
+                @Override
+                public void setRequestId(long requestId) {}
+
+                @Override
+                public TaskId getParentTask() {
+                    return TaskId.EMPTY_TASK_ID;
+                }
+            });
+        }
+
+        verifyNoInteractions(mockTracer);
+    }
+
+    /**
+     * Check that unregistering a task also causes tracing to be stopped on that task.
+     */
+    public void testUnregisterTaskStopsTracingIfTraceContextExists() {
+        final Tracer mockTracer = mock(Tracer.class);
+        final TaskManager taskManager = new TaskManager(Settings.EMPTY, threadPool, Set.of(), mockTracer);
+
         final Task task = taskManager.register("testType", "testAction", new TaskAwareRequest() {
 
             @Override
@@ -298,20 +358,21 @@ public void setRequestId(long requestId) {}
 
             @Override
             public TaskId getParentTask() {
-                return parentTask;
+                return TaskId.EMPTY_TASK_ID;
             }
         });
 
-        Map<String, Object> attributes = hasParentTask
-            ? Map.of(Tracer.AttributeKeys.TASK_ID, task.getId(), Tracer.AttributeKeys.PARENT_TASK_ID, parentTask.toString())
-            : Map.of(Tracer.AttributeKeys.TASK_ID, task.getId());
-        verify(mockTracer).startTrace(any(), eq(task), eq("testAction"), eq(attributes));
+        // fake a trace context (trace parent)
+        threadPool.getThreadContext().putHeader(Task.TRACE_PARENT_HTTP_HEADER, "traceparent");
+
+        taskManager.unregister(task);
+        verify(mockTracer).stopTrace(task);
     }
 
     /**
      * Check that unregistering a task also causes tracing to be stopped on that task.
      */
-    public void testUnregisterTaskStopsTracing() {
+    public void testUnregisterTaskStopsTracingIfTraceContextMissing() {
         final Tracer mockTracer = mock(Tracer.class);
         final TaskManager taskManager = new TaskManager(Settings.EMPTY, threadPool, Set.of(), mockTracer);
 
@@ -329,18 +390,22 @@ public TaskId getParentTask() {
             }
         });
 
-        taskManager.unregister(task);
+        // no trace context
 
-        verify(mockTracer).stopTrace(task);
+        taskManager.unregister(task);
+        verifyNoInteractions(mockTracer);
     }
 
     /**
-     * Check that registering and executing a task also causes tracing to be started and stopped on that task.
+     * Check that registering and executing a task also causes tracing to be started if a trace parent exists.
      */
-    public void testRegisterAndExecuteStartsAndStopsTracing() {
+    public void testRegisterAndExecuteStartsTracingIfTraceParentExists() {
         final Tracer mockTracer = mock(Tracer.class);
         final TaskManager taskManager = new TaskManager(Settings.EMPTY, threadPool, Set.of(), mockTracer);
 
+        // fake a trace parent
+        threadPool.getThreadContext().putHeader(Task.TRACE_PARENT_HTTP_HEADER, "traceparent");
+
         final Task task = taskManager.registerAndExecute(
             "testType",
             new TransportAction<ActionRequest, ActionResponse>(
@@ -375,25 +440,68 @@ public TaskId getParentTask() {
         verify(mockTracer).startTrace(any(), eq(task), eq("actionName"), anyMap());
     }
 
+    /**
+     * Check that registering and executing a task skips tracing if trace parent does not exists.
+     */
+    public void testRegisterAndExecuteSkipsTracingIfTraceParentMissing() {
+        final Tracer mockTracer = mock(Tracer.class);
+        final TaskManager taskManager = new TaskManager(Settings.EMPTY, threadPool, Set.of(), mockTracer);
+
+        // clean thread context without trace parent
+
+        final Task task = taskManager.registerAndExecute(
+            "testType",
+            new TransportAction<ActionRequest, ActionResponse>(
+                "actionName",
+                new ActionFilters(Set.of()),
+                taskManager,
+                EsExecutors.DIRECT_EXECUTOR_SERVICE
+            ) {
+                @Override
+                protected void doExecute(Task task, ActionRequest request, ActionListener<ActionResponse> listener) {
+                    listener.onResponse(new ActionResponse() {
+                        @Override
+                        public void writeTo(StreamOutput out) {}
+                    });
+                }
+            },
+            new ActionRequest() {
+                @Override
+                public ActionRequestValidationException validate() {
+                    return null;
+                }
+
+                @Override
+                public TaskId getParentTask() {
+                    return TaskId.EMPTY_TASK_ID;
+                }
+            },
+            null,
+            ActionTestUtils.assertNoFailureListener(r -> {})
+        );
+
+        verifyNoInteractions(mockTracer);
+    }
+
     public void testRegisterWithEnabledDisabledTracing() {
         final Tracer mockTracer = mock(Tracer.class);
         final TaskManager taskManager = spy(new TaskManager(Settings.EMPTY, threadPool, Set.of(), mockTracer));
 
         taskManager.register("type", "action", makeTaskRequest(true, 123), false);
-        verify(taskManager, times(0)).startTrace(any(), any());
+        verify(taskManager, times(0)).maybeStartTrace(any(), any());
 
         taskManager.register("type", "action", makeTaskRequest(false, 234), false);
-        verify(taskManager, times(0)).startTrace(any(), any());
+        verify(taskManager, times(0)).maybeStartTrace(any(), any());
 
         clearInvocations(taskManager);
 
         taskManager.register("type", "action", makeTaskRequest(true, 345), true);
-        verify(taskManager, times(1)).startTrace(any(), any());
+        verify(taskManager, times(1)).maybeStartTrace(any(), any());
 
         clearInvocations(taskManager);
 
         taskManager.register("type", "action", makeTaskRequest(false, 456), true);
-        verify(taskManager, times(1)).startTrace(any(), any());
+        verify(taskManager, times(1)).maybeStartTrace(any(), any());
     }
 
     static class CancellableRequest extends AbstractTransportRequest {