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); } }