From 79dc08e63646c2bd70e39d63f489197913dfbc82 Mon Sep 17 00:00:00 2001 From: Kiran Godishala <53332225+kirangodishala@users.noreply.github.com> Date: Fri, 20 Sep 2024 00:32:50 +0530 Subject: [PATCH] feat(web): populate the MDC when the ApplicationService gathers information from front50 and clouddriver (#1830) so that, for example, when gate receives an http request with an X-SPINNAKER-REQUEST-ID header, that same value appears in outgoing request headers to front50 and clouddriver. This makes troubleshooting a whole lot easier. --- .../gate/services/ApplicationService.groovy | 16 ++-- .../gate/services/MdcWrappedCallable.java | 43 ++++++++++ .../gate/services/MdcWrappedCallableTest.java | 78 +++++++++++++++++++ 3 files changed, 129 insertions(+), 8 deletions(-) create mode 100644 gate-web/src/main/java/com/netflix/spinnaker/gate/services/MdcWrappedCallable.java create mode 100644 gate-web/src/test/java/com/netflix/spinnaker/gate/services/MdcWrappedCallableTest.java diff --git a/gate-web/src/main/groovy/com/netflix/spinnaker/gate/services/ApplicationService.groovy b/gate-web/src/main/groovy/com/netflix/spinnaker/gate/services/ApplicationService.groovy index f886871ab3..8e97fb6797 100644 --- a/gate-web/src/main/groovy/com/netflix/spinnaker/gate/services/ApplicationService.groovy +++ b/gate-web/src/main/groovy/com/netflix/spinnaker/gate/services/ApplicationService.groovy @@ -351,7 +351,7 @@ class ApplicationService { return result } - static class Front50ApplicationListRetriever implements Callable> { + static class Front50ApplicationListRetriever extends MdcWrappedCallable> { private final Front50Service front50 private final AtomicReference> allApplicationsCache private final Object principal @@ -363,7 +363,7 @@ class ApplicationService { } @Override - List call() throws Exception { + List callWithMdc() throws Exception { try { AuthenticatedRequest.propagate({ try { @@ -383,7 +383,7 @@ class ApplicationService { } } - static class Front50ApplicationRetriever implements Callable { + static class Front50ApplicationRetriever extends MdcWrappedCallable { private final String name private final Front50Service front50 private final AtomicReference> allApplicationsCache @@ -399,7 +399,7 @@ class ApplicationService { } @Override - Map call() throws Exception { + Map callWithMdc() throws Exception { try { AuthenticatedRequest.propagate({ try { @@ -422,7 +422,7 @@ class ApplicationService { } } - static class ClouddriverApplicationListRetriever implements Callable> { + static class ClouddriverApplicationListRetriever extends MdcWrappedCallable> { private final ClouddriverService clouddriver private final Object principal private final AtomicReference> allApplicationsCache @@ -438,7 +438,7 @@ class ApplicationService { } @Override - List call() throws Exception { + List callWithMdc() throws Exception { try { AuthenticatedRequest.propagate({ try { @@ -458,7 +458,7 @@ class ApplicationService { } } - static class ClouddriverApplicationRetriever implements Callable { + static class ClouddriverApplicationRetriever extends MdcWrappedCallable { private final String name private final ClouddriverService clouddriver private final Object principal @@ -471,7 +471,7 @@ class ApplicationService { } @Override - Map call() throws Exception { + Map callWithMdc() throws Exception { try { AuthenticatedRequest.propagate({ try { diff --git a/gate-web/src/main/java/com/netflix/spinnaker/gate/services/MdcWrappedCallable.java b/gate-web/src/main/java/com/netflix/spinnaker/gate/services/MdcWrappedCallable.java new file mode 100644 index 0000000000..d73c216633 --- /dev/null +++ b/gate-web/src/main/java/com/netflix/spinnaker/gate/services/MdcWrappedCallable.java @@ -0,0 +1,43 @@ +/* + * Copyright 2023 Salesforce, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.netflix.spinnaker.gate.services; + +import java.util.Map; +import java.util.concurrent.Callable; +import org.slf4j.MDC; + +/** Make a copy of the MDC at construction time available to a task. */ +public abstract class MdcWrappedCallable implements Callable { + private final Map contextMap; + + public MdcWrappedCallable() { + this.contextMap = MDC.getCopyOfContextMap(); + } + + /** Compute a result, with the MDC as it was at construction time of this object */ + public abstract T callWithMdc() throws Exception; + + @Override + public T call() throws Exception { + if (contextMap == null) { + MDC.clear(); + } else { + MDC.setContextMap(contextMap); + } + return callWithMdc(); + } +} diff --git a/gate-web/src/test/java/com/netflix/spinnaker/gate/services/MdcWrappedCallableTest.java b/gate-web/src/test/java/com/netflix/spinnaker/gate/services/MdcWrappedCallableTest.java new file mode 100644 index 0000000000..df3c046425 --- /dev/null +++ b/gate-web/src/test/java/com/netflix/spinnaker/gate/services/MdcWrappedCallableTest.java @@ -0,0 +1,78 @@ +/* + * Copyright 2023 Salesforce, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.netflix.spinnaker.gate.services; + +import static org.assertj.core.api.Assertions.assertThat; + +import ch.qos.logback.classic.Level; +import com.netflix.spinnaker.kork.test.log.MemoryAppender; +import java.util.List; +import java.util.Map; +import java.util.concurrent.Callable; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import org.junit.jupiter.api.Test; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.slf4j.MDC; + +class MdcWrappedCallableTest { + private static final Logger log = LoggerFactory.getLogger(MdcWrappedCallableTest.class); + + @Test + void verifyMdcWrappedCallableIncludesTheMdc() throws Exception { + // Capture the log messages that our test callable generates + MemoryAppender memoryAppender = new MemoryAppender(MdcWrappedCallableTest.class); + + // Provide a way to execute callables in some other thread + ExecutorService executorService = Executors.newCachedThreadPool(); + + // Put something in the MDC here, to see if it makes it into the thread that + // executes the operation. + String mdcKey = "myKey"; + String mdcValue = "myValue"; + MDC.put(mdcKey, mdcValue); + + // The contents of the MDC at construction time of the MdcWrappedCallable + // are what's available when it executes, so construct it after the MDC is set. + Callable testCallable = new TestCallable(); + + // Execute the callable in another thread + executorService.submit(testCallable).get(); + + // Verify that messages logged in the MdcWrappedCallable include the info from the MDC + List logMessages = memoryAppender.search(mdcKey + "=" + mdcValue, Level.INFO); + assertThat(logMessages).hasSize(1); + + // And now clear the MDC and make sure the resulting operation gets the empty MDC. + MDC.clear(); + Callable emptyMdcCallable = new TestCallable(); + executorService.submit(emptyMdcCallable).get(); + + List emptyMdcMessages = memoryAppender.search("contextMap: null", Level.INFO); + assertThat(emptyMdcMessages).hasSize(1); + } + + static class TestCallable extends MdcWrappedCallable { + @Override + public Void callWithMdc() { + Map contextMap = MDC.getCopyOfContextMap(); + log.info("contextMap: {}", contextMap); + return null; + } + } +}