Slow log must use separate underlying logger for each index

[alexshadow007]

Closes #42432

[elasticmachine]

Pinging @elastic/es-core-infra

[pgomulka]

@alexshadow007 thank you for looking into this. I think your fix should work.
However we certainly miss testing around this. Do you think you could add appropriate unit tests to reproduce this?

[alexshadow007]

@pgomulka I added test for SearchSlowLog, is it fine?
But i have problem with testing of IndexSlowLog.
To call IndexingSlowLog.postIndex i need Engine.IndexResult with correct took time, but took time has package private visibility - so i can't set it from test. Any advices?

[pgomulka]

@alexshadow007 added test is really good! thank you for this
in regards to IndexSLowLogTests how about we simplify the test case there? It is after all only a change about setting LogLevel. I was thinking of something like this

    public void testLevelSettingMultipleIndices() {
        IndexSettings infoSettings = new IndexSettings(
            newIndexMeta("index-1-info", Settings.builder()
                                                 .put(IndexMetaData.SETTING_INDEX_UUID, UUIDs.randomBase64UUID())
                                                 .put(IndexMetaData.SETTING_VERSION_CREATED, Version.CURRENT)
                                                 .put(SearchSlowLog.INDEX_SEARCH_SLOWLOG_THRESHOLD_FETCH_INFO_SETTING.getKey(), "0")
                                                 .put(IndexingSlowLog.INDEX_INDEXING_SLOWLOG_LEVEL_SETTING.getKey(), SlowLogLevel.INFO)
                                                 .build()), Settings.EMPTY);
        IndexingSlowLog infoLog = new IndexingSlowLog(infoSettings);
        assertEquals(infoLog.getLevel(), SlowLogLevel.INFO);


        IndexSettings debugSettings = new IndexSettings(
            newIndexMeta("index-2-debug", Settings.builder()
                                                  .put(IndexMetaData.SETTING_INDEX_UUID, UUIDs.randomBase64UUID())
                                                  .put(IndexMetaData.SETTING_VERSION_CREATED, Version.CURRENT)
                                                  .put(SearchSlowLog.INDEX_SEARCH_SLOWLOG_THRESHOLD_QUERY_DEBUG_SETTING.getKey(), "0")
                                                  .put(IndexingSlowLog.INDEX_INDEXING_SLOWLOG_LEVEL_SETTING.getKey(), SlowLogLevel.DEBUG)
                                                  .build()), Settings.EMPTY);
        IndexingSlowLog debugLog = new IndexingSlowLog(debugSettings);
        assertEquals(debugLog.getLevel(), SlowLogLevel.DEBUG);

        //assert log level is preserved
        assertEquals(infoLog.getLevel(), SlowLogLevel.INFO);
    }

Or maybe we could try that approach in SearchSlowLogTests. What do you think?

[alexshadow007]

@pgomulka
Test case provided by you doesn't verify this bug. Because problem was in underlying logger - log level of slow log instance was right, but log level of underlying log4 logger was different (because instance of log4j logger was shared between all instances of slow log). I can add test case where i verify log level of underlying logger by it's name.

        String uuid = UUIDs.randomBase64UUID();
        IndexSettings infoSettings = new IndexSettings(
            newIndexMeta("index-1-info", Settings.builder()
                                                 .put(IndexMetaData.SETTING_INDEX_UUID, uuid )
                                                 .put(IndexMetaData.SETTING_VERSION_CREATED, Version.CURRENT)
                                                 .put(SearchSlowLog.INDEX_SEARCH_SLOWLOG_THRESHOLD_FETCH_INFO_SETTING.getKey(), "0")
                                                 .put(IndexingSlowLog.INDEX_INDEXING_SLOWLOG_LEVEL_SETTING.getKey(), SlowLogLevel.INFO)
                                                 .build()), Settings.EMPTY);
        IndexingSlowLog infoLog = new IndexingSlowLog(infoSettings);
        assertEquals(LogManager.getLogger("index.search.slowlog.query." + uuid).getLevel, Level.INFO);

[pgomulka]

@alexshadow007 you are right. I was thinking that getLevel would be changed to something to return the level of the underlying logger

 SlowLogLevel getLevel() {
        return SlowLogLevel.parse(indexLogger.getLevel().name());
    }

I am not sure we really need that level variable inside SlowLoggers

[alexshadow007]

@pgomulka Yes, i think it is not bad idea. I will do it and add test case.

[alexshadow007]

@pgomulka Done

[pgomulka]

ok to test

[pgomulka]

@elasticmachine run elasticsearch-ci/packaging-sample

[pgomulka]

@elasticmachine run elasticsearch-ci/2

[pgomulka]

@elasticmachine run elasticsearch-ci/2

[pgomulka]

@alexshadow007 there are some test failures in CI, but they seem unrelated. I will try to confirm with the team the reason it is failing (I can't reproduce it).
But once again I wanted to thank you this PR, the change looks great.

[pgomulka]

@elasticmachine update branch

[pgomulka]

@elasticmachine run elasticsearch-ci/packaging-sample

[pgomulka]

@elasticmachine run elasticsearch-ci/packaging-sample-matrix