From 3038cb5d9faabc5256304ed6336d1e08be00887f Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Mon, 6 Jan 2025 09:07:28 -0800 Subject: [PATCH] Move SlowLogFieldProvider instantiation to node construction (#117949) SPI from plugins should be created at node startup. This commit moves creation of SlowLogFieldProvider into node construction and passes it in to IndicesService so that it is not recreated on each index creation. relates #102103 --- docs/changelog/117949.yaml | 5 ++ .../org/elasticsearch/index/IndexModule.java | 5 +- .../elasticsearch/index/IndexingSlowLog.java | 42 ++---------- .../elasticsearch/index/SearchSlowLog.java | 23 ++++--- .../index/SlowLogFieldProvider.java | 18 +----- .../elasticsearch/index/SlowLogFields.java | 30 +++++++++ .../elasticsearch/indices/IndicesService.java | 31 ++------- .../indices/IndicesServiceBuilder.java | 24 +++++++ .../elasticsearch/node/NodeConstruction.java | 28 ++++++++ .../index/IndexingSlowLogTests.java | 18 +++--- .../index/SearchSlowLogTests.java | 18 +++--- .../indices/IndicesServiceTests.java | 64 ++++++++++--------- .../slowlog/SecuritySlowLogFieldProvider.java | 58 +++++++++-------- 13 files changed, 200 insertions(+), 164 deletions(-) create mode 100644 docs/changelog/117949.yaml create mode 100644 server/src/main/java/org/elasticsearch/index/SlowLogFields.java diff --git a/docs/changelog/117949.yaml b/docs/changelog/117949.yaml new file mode 100644 index 0000000000000..b67f36a224094 --- /dev/null +++ b/docs/changelog/117949.yaml @@ -0,0 +1,5 @@ +pr: 117949 +summary: Move `SlowLogFieldProvider` instantiation to node construction +area: Infra/Logging +type: bug +issues: [] diff --git a/server/src/main/java/org/elasticsearch/index/IndexModule.java b/server/src/main/java/org/elasticsearch/index/IndexModule.java index 35d83586ce177..a9f563a8360c8 100644 --- a/server/src/main/java/org/elasticsearch/index/IndexModule.java +++ b/server/src/main/java/org/elasticsearch/index/IndexModule.java @@ -208,8 +208,9 @@ public IndexModule( this.engineFactory = Objects.requireNonNull(engineFactory); // Need to have a mutable arraylist for plugins to add listeners to it this.searchOperationListeners = new ArrayList<>(searchOperationListeners); - this.searchOperationListeners.add(new SearchSlowLog(indexSettings, slowLogFieldProvider)); - this.indexOperationListeners.add(new IndexingSlowLog(indexSettings, slowLogFieldProvider)); + SlowLogFields slowLogFields = slowLogFieldProvider.create(indexSettings); + this.searchOperationListeners.add(new SearchSlowLog(indexSettings, slowLogFields)); + this.indexOperationListeners.add(new IndexingSlowLog(indexSettings, slowLogFields)); this.directoryFactories = Collections.unmodifiableMap(directoryFactories); this.allowExpensiveQueries = allowExpensiveQueries; this.expressionResolver = expressionResolver; diff --git a/server/src/main/java/org/elasticsearch/index/IndexingSlowLog.java b/server/src/main/java/org/elasticsearch/index/IndexingSlowLog.java index 3ae4c0eb82ad0..5a7990a4e70c5 100644 --- a/server/src/main/java/org/elasticsearch/index/IndexingSlowLog.java +++ b/server/src/main/java/org/elasticsearch/index/IndexingSlowLog.java @@ -103,7 +103,7 @@ public final class IndexingSlowLog implements IndexingOperationListener { * characters of the source. */ private int maxSourceCharsToLog; - private final SlowLogFieldProvider slowLogFieldProvider; + private final SlowLogFields slowLogFields; /** * Reads how much of the source to log. The user can specify any value they @@ -125,8 +125,8 @@ public final class IndexingSlowLog implements IndexingOperationListener { Property.IndexScope ); - IndexingSlowLog(IndexSettings indexSettings, SlowLogFieldProvider slowLogFieldProvider) { - this.slowLogFieldProvider = slowLogFieldProvider; + IndexingSlowLog(IndexSettings indexSettings, SlowLogFields slowLogFields) { + this.slowLogFields = slowLogFields; this.index = indexSettings.getIndex(); indexSettings.getScopedSettings().addSettingsUpdateConsumer(INDEX_INDEXING_SLOWLOG_REFORMAT_SETTING, this::setReformat); @@ -179,47 +179,19 @@ public void postIndex(ShardId shardId, Engine.Index indexOperation, Engine.Index final long tookInNanos = result.getTook(); if (indexWarnThreshold >= 0 && tookInNanos > indexWarnThreshold) { indexLogger.warn( - IndexingSlowLogMessage.of( - this.slowLogFieldProvider.indexSlowLogFields(), - index, - doc, - tookInNanos, - reformat, - maxSourceCharsToLog - ) + IndexingSlowLogMessage.of(this.slowLogFields.indexFields(), index, doc, tookInNanos, reformat, maxSourceCharsToLog) ); } else if (indexInfoThreshold >= 0 && tookInNanos > indexInfoThreshold) { indexLogger.info( - IndexingSlowLogMessage.of( - this.slowLogFieldProvider.indexSlowLogFields(), - index, - doc, - tookInNanos, - reformat, - maxSourceCharsToLog - ) + IndexingSlowLogMessage.of(this.slowLogFields.indexFields(), index, doc, tookInNanos, reformat, maxSourceCharsToLog) ); } else if (indexDebugThreshold >= 0 && tookInNanos > indexDebugThreshold) { indexLogger.debug( - IndexingSlowLogMessage.of( - this.slowLogFieldProvider.indexSlowLogFields(), - index, - doc, - tookInNanos, - reformat, - maxSourceCharsToLog - ) + IndexingSlowLogMessage.of(this.slowLogFields.indexFields(), index, doc, tookInNanos, reformat, maxSourceCharsToLog) ); } else if (indexTraceThreshold >= 0 && tookInNanos > indexTraceThreshold) { indexLogger.trace( - IndexingSlowLogMessage.of( - this.slowLogFieldProvider.indexSlowLogFields(), - index, - doc, - tookInNanos, - reformat, - maxSourceCharsToLog - ) + IndexingSlowLogMessage.of(this.slowLogFields.indexFields(), index, doc, tookInNanos, reformat, maxSourceCharsToLog) ); } } diff --git a/server/src/main/java/org/elasticsearch/index/SearchSlowLog.java b/server/src/main/java/org/elasticsearch/index/SearchSlowLog.java index e4836a391bfec..81e7cff862e32 100644 --- a/server/src/main/java/org/elasticsearch/index/SearchSlowLog.java +++ b/server/src/main/java/org/elasticsearch/index/SearchSlowLog.java @@ -45,7 +45,7 @@ public final class SearchSlowLog implements SearchOperationListener { private static final Logger queryLogger = LogManager.getLogger(INDEX_SEARCH_SLOWLOG_PREFIX + ".query"); private static final Logger fetchLogger = LogManager.getLogger(INDEX_SEARCH_SLOWLOG_PREFIX + ".fetch"); - private final SlowLogFieldProvider slowLogFieldProvider; + private final SlowLogFields slowLogFields; public static final Setting INDEX_SEARCH_SLOWLOG_INCLUDE_USER_SETTING = Setting.boolSetting( INDEX_SEARCH_SLOWLOG_PREFIX + ".include.user", @@ -126,9 +126,8 @@ public final class SearchSlowLog implements SearchOperationListener { private static final ToXContent.Params FORMAT_PARAMS = new ToXContent.MapParams(Collections.singletonMap("pretty", "false")); - public SearchSlowLog(IndexSettings indexSettings, SlowLogFieldProvider slowLogFieldProvider) { - slowLogFieldProvider.init(indexSettings); - this.slowLogFieldProvider = slowLogFieldProvider; + public SearchSlowLog(IndexSettings indexSettings, SlowLogFields slowLogFields) { + this.slowLogFields = slowLogFields; indexSettings.getScopedSettings() .addSettingsUpdateConsumer(INDEX_SEARCH_SLOWLOG_THRESHOLD_QUERY_WARN_SETTING, this::setQueryWarnThreshold); this.queryWarnThreshold = indexSettings.getValue(INDEX_SEARCH_SLOWLOG_THRESHOLD_QUERY_WARN_SETTING).nanos(); @@ -159,26 +158,26 @@ public SearchSlowLog(IndexSettings indexSettings, SlowLogFieldProvider slowLogFi @Override public void onQueryPhase(SearchContext context, long tookInNanos) { if (queryWarnThreshold >= 0 && tookInNanos > queryWarnThreshold) { - queryLogger.warn(SearchSlowLogMessage.of(this.slowLogFieldProvider.searchSlowLogFields(), context, tookInNanos)); + queryLogger.warn(SearchSlowLogMessage.of(this.slowLogFields.searchFields(), context, tookInNanos)); } else if (queryInfoThreshold >= 0 && tookInNanos > queryInfoThreshold) { - queryLogger.info(SearchSlowLogMessage.of(this.slowLogFieldProvider.searchSlowLogFields(), context, tookInNanos)); + queryLogger.info(SearchSlowLogMessage.of(this.slowLogFields.searchFields(), context, tookInNanos)); } else if (queryDebugThreshold >= 0 && tookInNanos > queryDebugThreshold) { - queryLogger.debug(SearchSlowLogMessage.of(this.slowLogFieldProvider.searchSlowLogFields(), context, tookInNanos)); + queryLogger.debug(SearchSlowLogMessage.of(this.slowLogFields.searchFields(), context, tookInNanos)); } else if (queryTraceThreshold >= 0 && tookInNanos > queryTraceThreshold) { - queryLogger.trace(SearchSlowLogMessage.of(this.slowLogFieldProvider.searchSlowLogFields(), context, tookInNanos)); + queryLogger.trace(SearchSlowLogMessage.of(this.slowLogFields.searchFields(), context, tookInNanos)); } } @Override public void onFetchPhase(SearchContext context, long tookInNanos) { if (fetchWarnThreshold >= 0 && tookInNanos > fetchWarnThreshold) { - fetchLogger.warn(SearchSlowLogMessage.of(this.slowLogFieldProvider.searchSlowLogFields(), context, tookInNanos)); + fetchLogger.warn(SearchSlowLogMessage.of(this.slowLogFields.searchFields(), context, tookInNanos)); } else if (fetchInfoThreshold >= 0 && tookInNanos > fetchInfoThreshold) { - fetchLogger.info(SearchSlowLogMessage.of(this.slowLogFieldProvider.searchSlowLogFields(), context, tookInNanos)); + fetchLogger.info(SearchSlowLogMessage.of(this.slowLogFields.searchFields(), context, tookInNanos)); } else if (fetchDebugThreshold >= 0 && tookInNanos > fetchDebugThreshold) { - fetchLogger.debug(SearchSlowLogMessage.of(this.slowLogFieldProvider.searchSlowLogFields(), context, tookInNanos)); + fetchLogger.debug(SearchSlowLogMessage.of(this.slowLogFields.searchFields(), context, tookInNanos)); } else if (fetchTraceThreshold >= 0 && tookInNanos > fetchTraceThreshold) { - fetchLogger.trace(SearchSlowLogMessage.of(this.slowLogFieldProvider.searchSlowLogFields(), context, tookInNanos)); + fetchLogger.trace(SearchSlowLogMessage.of(this.slowLogFields.searchFields(), context, tookInNanos)); } } diff --git a/server/src/main/java/org/elasticsearch/index/SlowLogFieldProvider.java b/server/src/main/java/org/elasticsearch/index/SlowLogFieldProvider.java index c61d4d4c85a86..e93edccc83b15 100644 --- a/server/src/main/java/org/elasticsearch/index/SlowLogFieldProvider.java +++ b/server/src/main/java/org/elasticsearch/index/SlowLogFieldProvider.java @@ -9,28 +9,14 @@ package org.elasticsearch.index; -import java.util.Map; - /** * Interface for providing additional fields to the slow log from a plugin. * Intended to be loaded through SPI. */ public interface SlowLogFieldProvider { /** - * Initialize field provider with index level settings to be able to listen for updates and set initial values + * Create a field provider with index level settings to be able to listen for updates and set initial values * @param indexSettings settings for the index */ - void init(IndexSettings indexSettings); - - /** - * Slow log fields for indexing events - * @return map of field name to value - */ - Map indexSlowLogFields(); - - /** - * Slow log fields for search events - * @return map of field name to value - */ - Map searchSlowLogFields(); + SlowLogFields create(IndexSettings indexSettings); } diff --git a/server/src/main/java/org/elasticsearch/index/SlowLogFields.java b/server/src/main/java/org/elasticsearch/index/SlowLogFields.java new file mode 100644 index 0000000000000..e018e3a4d6bb7 --- /dev/null +++ b/server/src/main/java/org/elasticsearch/index/SlowLogFields.java @@ -0,0 +1,30 @@ +/* + * 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", the "GNU Affero General Public License v3.0 only", 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", the "GNU Affero General Public + * License v3.0 only", or the "Server Side Public License, v 1". + */ + +package org.elasticsearch.index; + +import java.util.Map; + +/** + * Fields for the slow log. These may be different each call depending on the state of the system. + */ +public interface SlowLogFields { + + /** + * Slow log fields for indexing events + * @return map of field name to value + */ + Map indexFields(); + + /** + * Slow log fields for search events + * @return map of field name to value + */ + Map searchFields(); +} diff --git a/server/src/main/java/org/elasticsearch/indices/IndicesService.java b/server/src/main/java/org/elasticsearch/indices/IndicesService.java index e6d8290286a78..12dae9001551d 100644 --- a/server/src/main/java/org/elasticsearch/indices/IndicesService.java +++ b/server/src/main/java/org/elasticsearch/indices/IndicesService.java @@ -267,6 +267,7 @@ public class IndicesService extends AbstractLifecycleComponent private final MapperMetrics mapperMetrics; private final List searchOperationListeners; private final QueryRewriteInterceptor queryRewriteInterceptor; + final SlowLogFieldProvider slowLogFieldProvider; // pkg-private for testingå @Override protected void doStart() { @@ -385,6 +386,7 @@ public void onRemoval(ShardId shardId, String fieldName, boolean wasEvicted, lon this.timestampFieldMapperService = new TimestampFieldMapperService(settings, threadPool, this); this.searchOperationListeners = builder.searchOperationListener; + this.slowLogFieldProvider = builder.slowLogFieldProvider; } private static final String DANGLING_INDICES_UPDATE_THREAD_NAME = "DanglingIndices#updateTask"; @@ -755,7 +757,7 @@ private synchronized IndexService createIndexService( () -> allowExpensiveQueries, indexNameExpressionResolver, recoveryStateFactories, - loadSlowLogFieldProvider(), + slowLogFieldProvider, mapperMetrics, searchOperationListeners ); @@ -835,7 +837,7 @@ public synchronized MapperService createIndexMapperServiceForValidation(IndexMet () -> allowExpensiveQueries, indexNameExpressionResolver, recoveryStateFactories, - loadSlowLogFieldProvider(), + slowLogFieldProvider, mapperMetrics, searchOperationListeners ); @@ -1437,31 +1439,6 @@ int numPendingDeletes(Index index) { } } - // pkg-private for testing - SlowLogFieldProvider loadSlowLogFieldProvider() { - List slowLogFieldProviders = pluginsService.loadServiceProviders(SlowLogFieldProvider.class); - return new SlowLogFieldProvider() { - @Override - public void init(IndexSettings indexSettings) { - slowLogFieldProviders.forEach(provider -> provider.init(indexSettings)); - } - - @Override - public Map indexSlowLogFields() { - return slowLogFieldProviders.stream() - .flatMap(provider -> provider.indexSlowLogFields().entrySet().stream()) - .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue)); - } - - @Override - public Map searchSlowLogFields() { - return slowLogFieldProviders.stream() - .flatMap(provider -> provider.searchSlowLogFields().entrySet().stream()) - .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue)); - } - }; - } - /** * Checks if all pending deletes have completed. Used by tests to ensure we don't check directory contents * while deletion still ongoing. * The reason is that, on Windows, browsing the directory contents can interfere diff --git a/server/src/main/java/org/elasticsearch/indices/IndicesServiceBuilder.java b/server/src/main/java/org/elasticsearch/indices/IndicesServiceBuilder.java index f0f0f453e3be8..d88bbfa3eba17 100644 --- a/server/src/main/java/org/elasticsearch/indices/IndicesServiceBuilder.java +++ b/server/src/main/java/org/elasticsearch/indices/IndicesServiceBuilder.java @@ -23,6 +23,8 @@ import org.elasticsearch.features.FeatureService; import org.elasticsearch.gateway.MetaStateService; import org.elasticsearch.index.IndexSettings; +import org.elasticsearch.index.SlowLogFieldProvider; +import org.elasticsearch.index.SlowLogFields; import org.elasticsearch.index.analysis.AnalysisRegistry; import org.elasticsearch.index.engine.EngineFactory; import org.elasticsearch.index.mapper.MapperMetrics; @@ -79,6 +81,22 @@ public class IndicesServiceBuilder { MapperMetrics mapperMetrics; List searchOperationListener = List.of(); QueryRewriteInterceptor queryRewriteInterceptor = null; + SlowLogFieldProvider slowLogFieldProvider = new SlowLogFieldProvider() { + @Override + public SlowLogFields create(IndexSettings indexSettings) { + return new SlowLogFields() { + @Override + public Map indexFields() { + return Map.of(); + } + + @Override + public Map searchFields() { + return Map.of(); + } + }; + } + }; public IndicesServiceBuilder settings(Settings settings) { this.settings = settings; @@ -191,6 +209,11 @@ public IndicesServiceBuilder searchOperationListeners(List slowLogFieldProviders = pluginsService.loadServiceProviders(SlowLogFieldProvider.class); + // NOTE: the response of index/search slow log fields below must be calculated dynamically on every call + // because the responses may change dynamically at runtime + SlowLogFieldProvider slowLogFieldProvider = indexSettings -> { + final List fields = new ArrayList<>(); + for (var provider : slowLogFieldProviders) { + fields.add(provider.create(indexSettings)); + } + return new SlowLogFields() { + @Override + public Map indexFields() { + return fields.stream() + .flatMap(f -> f.indexFields().entrySet().stream()) + .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue)); + } + + @Override + public Map searchFields() { + return fields.stream() + .flatMap(f -> f.searchFields().entrySet().stream()) + .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue)); + } + }; + }; + IndicesService indicesService = new IndicesServiceBuilder().settings(settings) .pluginsService(pluginsService) .nodeEnvironment(nodeEnvironment) @@ -829,6 +856,7 @@ private void construct( .requestCacheKeyDifferentiator(searchModule.getRequestCacheKeyDifferentiator()) .mapperMetrics(mapperMetrics) .searchOperationListeners(searchOperationListeners) + .slowLogFieldProvider(slowLogFieldProvider) .build(); final var parameters = new IndexSettingProvider.Parameters(clusterService, indicesService::createIndexMapperServiceForValidation); diff --git a/server/src/test/java/org/elasticsearch/index/IndexingSlowLogTests.java b/server/src/test/java/org/elasticsearch/index/IndexingSlowLogTests.java index 753602e73a30a..7d489bffd24ca 100644 --- a/server/src/test/java/org/elasticsearch/index/IndexingSlowLogTests.java +++ b/server/src/test/java/org/elasticsearch/index/IndexingSlowLogTests.java @@ -81,7 +81,7 @@ public void testLevelPrecedence() { String uuid = UUIDs.randomBase64UUID(); IndexMetadata metadata = createIndexMetadata("index-precedence", settings(uuid)); IndexSettings settings = new IndexSettings(metadata, Settings.EMPTY); - IndexingSlowLog log = new IndexingSlowLog(settings, mock(SlowLogFieldProvider.class)); + IndexingSlowLog log = new IndexingSlowLog(settings, mock(SlowLogFields.class)); ParsedDocument doc = EngineTestCase.createParsedDoc("1", null); Engine.Index index = new Engine.Index(Uid.encodeId("doc_id"), randomNonNegativeLong(), doc); @@ -142,7 +142,7 @@ public void testTwoLoggersDifferentLevel() { ), Settings.EMPTY ); - IndexingSlowLog log1 = new IndexingSlowLog(index1Settings, mock(SlowLogFieldProvider.class)); + IndexingSlowLog log1 = new IndexingSlowLog(index1Settings, mock(SlowLogFields.class)); IndexSettings index2Settings = new IndexSettings( createIndexMetadata( @@ -155,7 +155,7 @@ public void testTwoLoggersDifferentLevel() { ), Settings.EMPTY ); - IndexingSlowLog log2 = new IndexingSlowLog(index2Settings, mock(SlowLogFieldProvider.class)); + IndexingSlowLog log2 = new IndexingSlowLog(index2Settings, mock(SlowLogFields.class)); ParsedDocument doc = EngineTestCase.createParsedDoc("1", null); Engine.Index index = new Engine.Index(Uid.encodeId("doc_id"), randomNonNegativeLong(), doc); @@ -179,12 +179,12 @@ public void testMultipleSlowLoggersUseSingleLog4jLogger() { LoggerContext context = (LoggerContext) LogManager.getContext(false); IndexSettings index1Settings = new IndexSettings(createIndexMetadata("index1", settings(UUIDs.randomBase64UUID())), Settings.EMPTY); - IndexingSlowLog log1 = new IndexingSlowLog(index1Settings, mock(SlowLogFieldProvider.class)); + IndexingSlowLog log1 = new IndexingSlowLog(index1Settings, mock(SlowLogFields.class)); int numberOfLoggersBefore = context.getLoggers().size(); IndexSettings index2Settings = new IndexSettings(createIndexMetadata("index2", settings(UUIDs.randomBase64UUID())), Settings.EMPTY); - IndexingSlowLog log2 = new IndexingSlowLog(index2Settings, mock(SlowLogFieldProvider.class)); + IndexingSlowLog log2 = new IndexingSlowLog(index2Settings, mock(SlowLogFields.class)); context = (LoggerContext) LogManager.getContext(false); int numberOfLoggersAfter = context.getLoggers().size(); @@ -355,7 +355,7 @@ public void testReformatSetting() { .build() ); IndexSettings settings = new IndexSettings(metadata, Settings.EMPTY); - IndexingSlowLog log = new IndexingSlowLog(settings, mock(SlowLogFieldProvider.class)); + IndexingSlowLog log = new IndexingSlowLog(settings, mock(SlowLogFields.class)); assertFalse(log.isReformat()); settings.updateIndexMetadata( newIndexMeta("index", Settings.builder().put(IndexingSlowLog.INDEX_INDEXING_SLOWLOG_REFORMAT_SETTING.getKey(), "true").build()) @@ -372,7 +372,7 @@ public void testReformatSetting() { metadata = newIndexMeta("index", Settings.builder().put(IndexMetadata.SETTING_VERSION_CREATED, IndexVersion.current()).build()); settings = new IndexSettings(metadata, Settings.EMPTY); - log = new IndexingSlowLog(settings, mock(SlowLogFieldProvider.class)); + log = new IndexingSlowLog(settings, mock(SlowLogFields.class)); assertTrue(log.isReformat()); try { settings.updateIndexMetadata( @@ -405,7 +405,7 @@ public void testSetLevels() { .build() ); IndexSettings settings = new IndexSettings(metadata, Settings.EMPTY); - IndexingSlowLog log = new IndexingSlowLog(settings, mock(SlowLogFieldProvider.class)); + IndexingSlowLog log = new IndexingSlowLog(settings, mock(SlowLogFields.class)); assertEquals(TimeValue.timeValueMillis(100).nanos(), log.getIndexTraceThreshold()); assertEquals(TimeValue.timeValueMillis(200).nanos(), log.getIndexDebugThreshold()); assertEquals(TimeValue.timeValueMillis(300).nanos(), log.getIndexInfoThreshold()); @@ -436,7 +436,7 @@ public void testSetLevels() { assertEquals(TimeValue.timeValueMillis(-1).nanos(), log.getIndexWarnThreshold()); settings = new IndexSettings(metadata, Settings.EMPTY); - log = new IndexingSlowLog(settings, mock(SlowLogFieldProvider.class)); + log = new IndexingSlowLog(settings, mock(SlowLogFields.class)); assertEquals(TimeValue.timeValueMillis(-1).nanos(), log.getIndexTraceThreshold()); assertEquals(TimeValue.timeValueMillis(-1).nanos(), log.getIndexDebugThreshold()); diff --git a/server/src/test/java/org/elasticsearch/index/SearchSlowLogTests.java b/server/src/test/java/org/elasticsearch/index/SearchSlowLogTests.java index 50e3269a6b9ba..359118c7cb5a1 100644 --- a/server/src/test/java/org/elasticsearch/index/SearchSlowLogTests.java +++ b/server/src/test/java/org/elasticsearch/index/SearchSlowLogTests.java @@ -103,7 +103,7 @@ public void testLevelPrecedence() { try (SearchContext ctx = searchContextWithSourceAndTask(createIndex("index"))) { String uuid = UUIDs.randomBase64UUID(); IndexSettings settings = new IndexSettings(createIndexMetadata("index", settings(uuid)), Settings.EMPTY); - SearchSlowLog log = new SearchSlowLog(settings, mock(SlowLogFieldProvider.class)); + SearchSlowLog log = new SearchSlowLog(settings, mock(SlowLogFields.class)); // For this test, when level is not breached, the level below should be used. { @@ -187,7 +187,7 @@ public void testTwoLoggersDifferentLevel() { ), Settings.EMPTY ); - SearchSlowLog log1 = new SearchSlowLog(settings1, mock(SlowLogFieldProvider.class)); + SearchSlowLog log1 = new SearchSlowLog(settings1, mock(SlowLogFields.class)); IndexSettings settings2 = new IndexSettings( createIndexMetadata( @@ -200,7 +200,7 @@ public void testTwoLoggersDifferentLevel() { ), Settings.EMPTY ); - SearchSlowLog log2 = new SearchSlowLog(settings2, mock(SlowLogFieldProvider.class)); + SearchSlowLog log2 = new SearchSlowLog(settings2, mock(SlowLogFields.class)); { // threshold set on WARN only, should not log @@ -223,7 +223,7 @@ public void testMultipleSlowLoggersUseSingleLog4jLogger() { try (SearchContext ctx1 = searchContextWithSourceAndTask(createIndex("index-1"))) { IndexSettings settings1 = new IndexSettings(createIndexMetadata("index-1", settings(UUIDs.randomBase64UUID())), Settings.EMPTY); - SearchSlowLog log1 = new SearchSlowLog(settings1, mock(SlowLogFieldProvider.class)); + SearchSlowLog log1 = new SearchSlowLog(settings1, mock(SlowLogFields.class)); int numberOfLoggersBefore = context.getLoggers().size(); try (SearchContext ctx2 = searchContextWithSourceAndTask(createIndex("index-2"))) { @@ -231,7 +231,7 @@ public void testMultipleSlowLoggersUseSingleLog4jLogger() { createIndexMetadata("index-2", settings(UUIDs.randomBase64UUID())), Settings.EMPTY ); - SearchSlowLog log2 = new SearchSlowLog(settings2, mock(SlowLogFieldProvider.class)); + SearchSlowLog log2 = new SearchSlowLog(settings2, mock(SlowLogFields.class)); int numberOfLoggersAfter = context.getLoggers().size(); assertThat(numberOfLoggersAfter, equalTo(numberOfLoggersBefore)); @@ -323,7 +323,7 @@ public void testSetQueryLevels() { .build() ); IndexSettings settings = new IndexSettings(metadata, Settings.EMPTY); - SearchSlowLog log = new SearchSlowLog(settings, mock(SlowLogFieldProvider.class)); + SearchSlowLog log = new SearchSlowLog(settings, mock(SlowLogFields.class)); assertEquals(TimeValue.timeValueMillis(100).nanos(), log.getQueryTraceThreshold()); assertEquals(TimeValue.timeValueMillis(200).nanos(), log.getQueryDebugThreshold()); assertEquals(TimeValue.timeValueMillis(300).nanos(), log.getQueryInfoThreshold()); @@ -354,7 +354,7 @@ public void testSetQueryLevels() { assertEquals(TimeValue.timeValueMillis(-1).nanos(), log.getQueryWarnThreshold()); settings = new IndexSettings(metadata, Settings.EMPTY); - log = new SearchSlowLog(settings, mock(SlowLogFieldProvider.class)); + log = new SearchSlowLog(settings, mock(SlowLogFields.class)); assertEquals(TimeValue.timeValueMillis(-1).nanos(), log.getQueryTraceThreshold()); assertEquals(TimeValue.timeValueMillis(-1).nanos(), log.getQueryDebugThreshold()); @@ -429,7 +429,7 @@ public void testSetFetchLevels() { .build() ); IndexSettings settings = new IndexSettings(metadata, Settings.EMPTY); - SearchSlowLog log = new SearchSlowLog(settings, mock(SlowLogFieldProvider.class)); + SearchSlowLog log = new SearchSlowLog(settings, mock(SlowLogFields.class)); assertEquals(TimeValue.timeValueMillis(100).nanos(), log.getFetchTraceThreshold()); assertEquals(TimeValue.timeValueMillis(200).nanos(), log.getFetchDebugThreshold()); assertEquals(TimeValue.timeValueMillis(300).nanos(), log.getFetchInfoThreshold()); @@ -460,7 +460,7 @@ public void testSetFetchLevels() { assertEquals(TimeValue.timeValueMillis(-1).nanos(), log.getFetchWarnThreshold()); settings = new IndexSettings(metadata, Settings.EMPTY); - log = new SearchSlowLog(settings, mock(SlowLogFieldProvider.class)); + log = new SearchSlowLog(settings, mock(SlowLogFields.class)); assertEquals(TimeValue.timeValueMillis(-1).nanos(), log.getFetchTraceThreshold()); assertEquals(TimeValue.timeValueMillis(-1).nanos(), log.getFetchDebugThreshold()); diff --git a/server/src/test/java/org/elasticsearch/indices/IndicesServiceTests.java b/server/src/test/java/org/elasticsearch/indices/IndicesServiceTests.java index 17975b7d18dd8..b56afadb14924 100644 --- a/server/src/test/java/org/elasticsearch/indices/IndicesServiceTests.java +++ b/server/src/test/java/org/elasticsearch/indices/IndicesServiceTests.java @@ -44,6 +44,7 @@ import org.elasticsearch.index.IndexVersion; import org.elasticsearch.index.IndexVersions; import org.elasticsearch.index.SlowLogFieldProvider; +import org.elasticsearch.index.SlowLogFields; import org.elasticsearch.index.engine.Engine; import org.elasticsearch.index.engine.EngineConfig; import org.elasticsearch.index.engine.EngineFactory; @@ -209,16 +210,18 @@ static void setFields(Map fields) { } @Override - public void init(IndexSettings indexSettings) {} - - @Override - public Map indexSlowLogFields() { - return fields; - } - - @Override - public Map searchSlowLogFields() { - return fields; + public SlowLogFields create(IndexSettings indexSettings) { + return new SlowLogFields() { + @Override + public Map indexFields() { + return fields; + } + + @Override + public Map searchFields() { + return fields; + } + }; } } @@ -231,16 +234,18 @@ static void setFields(Map fields) { } @Override - public void init(IndexSettings indexSettings) {} - - @Override - public Map indexSlowLogFields() { - return fields; - } - - @Override - public Map searchSlowLogFields() { - return fields; + public SlowLogFields create(IndexSettings indexSettings) { + return new SlowLogFields() { + @Override + public Map indexFields() { + return fields; + } + + @Override + public Map searchFields() { + return fields; + } + }; } } @@ -805,33 +810,34 @@ public void testLoadSlowLogFieldProvider() { TestAnotherSlowLogFieldProvider.setFields(Map.of("key2", "value2")); var indicesService = getIndicesService(); - SlowLogFieldProvider fieldProvider = indicesService.loadSlowLogFieldProvider(); + SlowLogFieldProvider fieldProvider = indicesService.slowLogFieldProvider; + SlowLogFields fields = fieldProvider.create(null); // The map of fields from the two providers are merged to a single map of fields - assertEquals(Map.of("key1", "value1", "key2", "value2"), fieldProvider.searchSlowLogFields()); - assertEquals(Map.of("key1", "value1", "key2", "value2"), fieldProvider.indexSlowLogFields()); + assertEquals(Map.of("key1", "value1", "key2", "value2"), fields.searchFields()); + assertEquals(Map.of("key1", "value1", "key2", "value2"), fields.indexFields()); TestSlowLogFieldProvider.setFields(Map.of("key1", "value1")); TestAnotherSlowLogFieldProvider.setFields(Map.of("key1", "value2")); // There is an overlap of field names, since this isn't deterministic and probably a // programming error (two providers provide the same field) throw an exception - assertThrows(IllegalStateException.class, fieldProvider::searchSlowLogFields); - assertThrows(IllegalStateException.class, fieldProvider::indexSlowLogFields); + assertThrows(IllegalStateException.class, fields::searchFields); + assertThrows(IllegalStateException.class, fields::indexFields); TestSlowLogFieldProvider.setFields(Map.of("key1", "value1")); TestAnotherSlowLogFieldProvider.setFields(Map.of()); // One provider has no fields - assertEquals(Map.of("key1", "value1"), fieldProvider.searchSlowLogFields()); - assertEquals(Map.of("key1", "value1"), fieldProvider.indexSlowLogFields()); + assertEquals(Map.of("key1", "value1"), fields.searchFields()); + assertEquals(Map.of("key1", "value1"), fields.indexFields()); TestSlowLogFieldProvider.setFields(Map.of()); TestAnotherSlowLogFieldProvider.setFields(Map.of()); // Both providers have no fields - assertEquals(Map.of(), fieldProvider.searchSlowLogFields()); - assertEquals(Map.of(), fieldProvider.indexSlowLogFields()); + assertEquals(Map.of(), fields.searchFields()); + assertEquals(Map.of(), fields.indexFields()); } public void testWithTempIndexServiceHandlesExistingIndex() throws Exception { diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/slowlog/SecuritySlowLogFieldProvider.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/slowlog/SecuritySlowLogFieldProvider.java index 1610aedd1d363..b5327b6779656 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/slowlog/SecuritySlowLogFieldProvider.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/slowlog/SecuritySlowLogFieldProvider.java @@ -9,6 +9,7 @@ import org.elasticsearch.index.IndexSettings; import org.elasticsearch.index.SlowLogFieldProvider; +import org.elasticsearch.index.SlowLogFields; import org.elasticsearch.xpack.security.Security; import java.util.Map; @@ -18,8 +19,36 @@ public class SecuritySlowLogFieldProvider implements SlowLogFieldProvider { private final Security plugin; - private boolean includeUserInIndexing = false; - private boolean includeUserInSearch = false; + + private class SecuritySlowLogFields implements SlowLogFields { + private boolean includeUserInIndexing = false; + private boolean includeUserInSearch = false; + + SecuritySlowLogFields(IndexSettings indexSettings) { + indexSettings.getScopedSettings() + .addSettingsUpdateConsumer(INDEX_SEARCH_SLOWLOG_INCLUDE_USER_SETTING, newValue -> this.includeUserInSearch = newValue); + this.includeUserInSearch = indexSettings.getValue(INDEX_SEARCH_SLOWLOG_INCLUDE_USER_SETTING); + indexSettings.getScopedSettings() + .addSettingsUpdateConsumer(INDEX_INDEXING_SLOWLOG_INCLUDE_USER_SETTING, newValue -> this.includeUserInIndexing = newValue); + this.includeUserInIndexing = indexSettings.getValue(INDEX_INDEXING_SLOWLOG_INCLUDE_USER_SETTING); + } + + @Override + public Map indexFields() { + if (includeUserInIndexing) { + return plugin.getAuthContextForSlowLog(); + } + return Map.of(); + } + + @Override + public Map searchFields() { + if (includeUserInSearch) { + return plugin.getAuthContextForSlowLog(); + } + return Map.of(); + } + } public SecuritySlowLogFieldProvider() { throw new IllegalStateException("Provider must be constructed using PluginsService"); @@ -30,28 +59,7 @@ public SecuritySlowLogFieldProvider(Security plugin) { } @Override - public void init(IndexSettings indexSettings) { - indexSettings.getScopedSettings() - .addSettingsUpdateConsumer(INDEX_SEARCH_SLOWLOG_INCLUDE_USER_SETTING, newValue -> this.includeUserInSearch = newValue); - this.includeUserInSearch = indexSettings.getValue(INDEX_SEARCH_SLOWLOG_INCLUDE_USER_SETTING); - indexSettings.getScopedSettings() - .addSettingsUpdateConsumer(INDEX_INDEXING_SLOWLOG_INCLUDE_USER_SETTING, newValue -> this.includeUserInIndexing = newValue); - this.includeUserInIndexing = indexSettings.getValue(INDEX_INDEXING_SLOWLOG_INCLUDE_USER_SETTING); - } - - @Override - public Map indexSlowLogFields() { - if (includeUserInIndexing) { - return plugin.getAuthContextForSlowLog(); - } - return Map.of(); - } - - @Override - public Map searchSlowLogFields() { - if (includeUserInSearch) { - return plugin.getAuthContextForSlowLog(); - } - return Map.of(); + public SlowLogFields create(IndexSettings indexSettings) { + return new SecuritySlowLogFields(indexSettings); } }