From eefebb804be29d73640a597fe263ef25fa8fab68 Mon Sep 17 00:00:00 2001 From: Ashley Ou Date: Wed, 22 Jul 2026 16:23:26 -0700 Subject: [PATCH 1/4] api: add ApiTraceFilter with request trace id and active OpenTelemetry span trace id and span id --- api/pom.xml | 5 + .../cloudstack/api/filter/ApiTraceFilter.java | 73 +++++++++++++++ .../apache/cloudstack/context/LogContext.java | 16 ++++ .../api/filter/ApiTraceFilterTest.java | 92 +++++++++++++++++++ client/src/main/webapp/WEB-INF/web.xml | 10 ++ 5 files changed, 196 insertions(+) create mode 100644 api/src/main/java/org/apache/cloudstack/api/filter/ApiTraceFilter.java create mode 100644 api/src/test/java/org/apache/cloudstack/api/filter/ApiTraceFilterTest.java diff --git a/api/pom.xml b/api/pom.xml index d5791bed38e6..27eda410514f 100644 --- a/api/pom.xml +++ b/api/pom.xml @@ -71,6 +71,11 @@ cloud-framework-direct-download ${project.version} + + io.opentelemetry + opentelemetry-api + 1.51.0 + diff --git a/api/src/main/java/org/apache/cloudstack/api/filter/ApiTraceFilter.java b/api/src/main/java/org/apache/cloudstack/api/filter/ApiTraceFilter.java new file mode 100644 index 000000000000..f384bab11a26 --- /dev/null +++ b/api/src/main/java/org/apache/cloudstack/api/filter/ApiTraceFilter.java @@ -0,0 +1,73 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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 org.apache.cloudstack.api.filter; + +import org.apache.cloudstack.context.LogContext; +import java.io.IOException; +import java.util.UUID; +import javax.servlet.FilterConfig; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.Filter; +import javax.servlet.FilterChain; +import javax.servlet.ServletException; +import javax.servlet.ServletRequest; +import javax.servlet.ServletResponse; + +import io.opentelemetry.api.trace.Span; +import io.opentelemetry.api.trace.SpanContext; + +public class ApiTraceFilter implements Filter { + @Override + public void init(FilterConfig filterConfig) throws ServletException { + } + + @Override + public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain) + throws IOException, ServletException { + // Also record the active OpenTelemetry span so log lines can be joined to the + // distributed trace. Sourced from the span, not a header, so no id is invented; + // when there is no valid span the keys are left unset and render empty. + SpanContext spanContext = Span.current().getSpanContext(); + boolean spanApplied = spanContext.isValid(); + try { + HttpServletRequest httpReq = (HttpServletRequest) request; + String traceId = httpReq.getHeader(LogContext.X_B3_TRACEID_KEY); + if (traceId == null || traceId.isEmpty()) { + traceId = UUID.randomUUID().toString(); + } + + LogContext.current().putContextParameter(LogContext.X_B3_TRACEID_KEY, traceId); + if (spanApplied) { + LogContext.current().putContextParameter(LogContext.MOSAIC_TRACE_ID_KEY, spanContext.getTraceId()); + LogContext.current().putContextParameter(LogContext.MOSAIC_SPAN_ID_KEY, spanContext.getSpanId()); + } + chain.doFilter(request, response); + } finally { + LogContext.current().removeContextParameter(LogContext.X_B3_TRACEID_KEY); + if (spanApplied) { + LogContext.current().removeContextParameter(LogContext.MOSAIC_TRACE_ID_KEY); + LogContext.current().removeContextParameter(LogContext.MOSAIC_SPAN_ID_KEY); + } + } + } + + @Override + public void destroy() { + } +} diff --git a/api/src/main/java/org/apache/cloudstack/context/LogContext.java b/api/src/main/java/org/apache/cloudstack/context/LogContext.java index c367975aba3b..4828de90faec 100644 --- a/api/src/main/java/org/apache/cloudstack/context/LogContext.java +++ b/api/src/main/java/org/apache/cloudstack/context/LogContext.java @@ -53,6 +53,10 @@ public class LogContext { private long userId; private final Map context = new HashMap(); + public final static String X_B3_TRACEID_KEY = "traceid"; + public final static String MOSAIC_TRACE_ID_KEY = "mosaic_trace_id"; + public final static String MOSAIC_SPAN_ID_KEY = "mosaic_span_id"; + static EntityManager s_entityMgr; public static void init(EntityManager entityMgr) { @@ -78,6 +82,18 @@ protected LogContext(User user, Account account, String logContextId) { public void putContextParameter(String key, String value) { context.put(key, value); + MDC.put(key, value); + } + + public void removeContextParameter(String key) { + context.remove(key); + MDC.remove(key); + } + + public void removeContextParameters() { + for (Map.Entry entry : context.entrySet()) { + removeContextParameter(entry.getKey()); + } } public String getContextParameter(String key) { diff --git a/api/src/test/java/org/apache/cloudstack/api/filter/ApiTraceFilterTest.java b/api/src/test/java/org/apache/cloudstack/api/filter/ApiTraceFilterTest.java new file mode 100644 index 000000000000..4d5d8654995e --- /dev/null +++ b/api/src/test/java/org/apache/cloudstack/api/filter/ApiTraceFilterTest.java @@ -0,0 +1,92 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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 org.apache.cloudstack.api.filter; + +import javax.servlet.FilterChain; +import javax.servlet.ServletResponse; +import javax.servlet.http.HttpServletRequest; + +import org.apache.cloudstack.context.LogContext; +import org.apache.log4j.MDC; +import org.junit.After; +import org.junit.Assert; +import org.junit.Test; +import org.mockito.Mockito; + +import io.opentelemetry.api.trace.Span; +import io.opentelemetry.api.trace.SpanContext; +import io.opentelemetry.api.trace.TraceFlags; +import io.opentelemetry.api.trace.TraceState; +import io.opentelemetry.context.Scope; + +public class ApiTraceFilterTest { + + private static final String TRACE_ID = "4bf92f3577b34da6a3ce929d0e0e4736"; + private static final String SPAN_ID = "00f067aa0ba902b7"; + + private final ApiTraceFilter filter = new ApiTraceFilter(); + + @After + public void tearDown() { + LogContext.unregister(); + } + + @Test + public void putsMosaicTraceContextOnMdcWhenSpanActive() throws Exception { + HttpServletRequest req = Mockito.mock(HttpServletRequest.class); + Mockito.when(req.getHeader(LogContext.X_B3_TRACEID_KEY)).thenReturn(null); + String[] duringChain = new String[2]; + FilterChain chain = (rq, rs) -> { + duringChain[0] = (String) MDC.get(LogContext.MOSAIC_TRACE_ID_KEY); + duringChain[1] = (String) MDC.get(LogContext.MOSAIC_SPAN_ID_KEY); + }; + + SpanContext spanContext = + SpanContext.create(TRACE_ID, SPAN_ID, TraceFlags.getSampled(), TraceState.getDefault()); + try (Scope scope = Span.wrap(spanContext).makeCurrent()) { + filter.doFilter(req, Mockito.mock(ServletResponse.class), chain); + } + + // Present on the MDC while the request is in flight, from the active span. + Assert.assertEquals(TRACE_ID, duringChain[0]); + Assert.assertEquals(SPAN_ID, duringChain[1]); + // Removed once the request completes, so pooled threads do not leak it. + Assert.assertNull(MDC.get(LogContext.MOSAIC_TRACE_ID_KEY)); + Assert.assertNull(MDC.get(LogContext.MOSAIC_SPAN_ID_KEY)); + } + + @Test + public void leavesMosaicKeysUnsetWhenNoActiveSpan() throws Exception { + HttpServletRequest req = Mockito.mock(HttpServletRequest.class); + Mockito.when(req.getHeader(LogContext.X_B3_TRACEID_KEY)).thenReturn(null); + boolean[] chainInvoked = {false}; + String[] duringChain = new String[1]; + FilterChain chain = (rq, rs) -> { + chainInvoked[0] = true; + duringChain[0] = (String) MDC.get(LogContext.MOSAIC_TRACE_ID_KEY); + }; + + // No span in scope: Span.current() is the invalid default span. + filter.doFilter(req, Mockito.mock(ServletResponse.class), chain); + + Assert.assertTrue(chainInvoked[0]); + Assert.assertNull(duringChain[0]); + Assert.assertNull(MDC.get(LogContext.MOSAIC_TRACE_ID_KEY)); + } +} diff --git a/client/src/main/webapp/WEB-INF/web.xml b/client/src/main/webapp/WEB-INF/web.xml index 43bee7e59d88..fdb899b55562 100644 --- a/client/src/main/webapp/WEB-INF/web.xml +++ b/client/src/main/webapp/WEB-INF/web.xml @@ -36,6 +36,16 @@ classpath:META-INF/cloudstack/webApplicationContext.xml + + apiTraceFilter + org.apache.cloudstack.api.filter.ApiTraceFilter + + + + apiTraceFilter + /api/* + + cloudStartupServlet com.cloud.servlet.CloudStartupServlet From 62020b2edb8eba937b972a3e6d6637db5655c111 Mon Sep 17 00:00:00 2001 From: Ashleyyq Date: Mon, 3 Aug 2026 10:00:24 -0700 Subject: [PATCH 2/4] api: sstamp trace id via a central OpenTelemetry context hook; inherit managed opentelemetry-api version; fix removeContextParameters CME --- api/pom.xml | 1 - .../cloudstack/api/filter/ApiTraceFilter.java | 20 +--- .../apache/cloudstack/context/LogContext.java | 7 +- .../context/TraceContextMdcWrapper.java | 86 +++++++++++++++++ .../api/filter/ApiTraceFilterTest.java | 92 ------------------ .../context/TraceContextMdcWrapperTest.java | 96 +++++++++++++++++++ .../org/apache/cloudstack/ServerDaemon.java | 5 + 7 files changed, 195 insertions(+), 112 deletions(-) create mode 100644 api/src/main/java/org/apache/cloudstack/context/TraceContextMdcWrapper.java delete mode 100644 api/src/test/java/org/apache/cloudstack/api/filter/ApiTraceFilterTest.java create mode 100644 api/src/test/java/org/apache/cloudstack/context/TraceContextMdcWrapperTest.java diff --git a/api/pom.xml b/api/pom.xml index 27eda410514f..d4f9e3d10083 100644 --- a/api/pom.xml +++ b/api/pom.xml @@ -74,7 +74,6 @@ io.opentelemetry opentelemetry-api - 1.51.0 diff --git a/api/src/main/java/org/apache/cloudstack/api/filter/ApiTraceFilter.java b/api/src/main/java/org/apache/cloudstack/api/filter/ApiTraceFilter.java index f384bab11a26..f67cf3f7ced1 100644 --- a/api/src/main/java/org/apache/cloudstack/api/filter/ApiTraceFilter.java +++ b/api/src/main/java/org/apache/cloudstack/api/filter/ApiTraceFilter.java @@ -19,6 +19,8 @@ package org.apache.cloudstack.api.filter; import org.apache.cloudstack.context.LogContext; +import org.apache.commons.lang3.StringUtils; + import java.io.IOException; import java.util.UUID; import javax.servlet.FilterConfig; @@ -29,9 +31,6 @@ import javax.servlet.ServletRequest; import javax.servlet.ServletResponse; -import io.opentelemetry.api.trace.Span; -import io.opentelemetry.api.trace.SpanContext; - public class ApiTraceFilter implements Filter { @Override public void init(FilterConfig filterConfig) throws ServletException { @@ -40,30 +39,17 @@ public void init(FilterConfig filterConfig) throws ServletException { @Override public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain) throws IOException, ServletException { - // Also record the active OpenTelemetry span so log lines can be joined to the - // distributed trace. Sourced from the span, not a header, so no id is invented; - // when there is no valid span the keys are left unset and render empty. - SpanContext spanContext = Span.current().getSpanContext(); - boolean spanApplied = spanContext.isValid(); try { HttpServletRequest httpReq = (HttpServletRequest) request; String traceId = httpReq.getHeader(LogContext.X_B3_TRACEID_KEY); - if (traceId == null || traceId.isEmpty()) { + if (StringUtils.isBlank(traceId)) { traceId = UUID.randomUUID().toString(); } LogContext.current().putContextParameter(LogContext.X_B3_TRACEID_KEY, traceId); - if (spanApplied) { - LogContext.current().putContextParameter(LogContext.MOSAIC_TRACE_ID_KEY, spanContext.getTraceId()); - LogContext.current().putContextParameter(LogContext.MOSAIC_SPAN_ID_KEY, spanContext.getSpanId()); - } chain.doFilter(request, response); } finally { LogContext.current().removeContextParameter(LogContext.X_B3_TRACEID_KEY); - if (spanApplied) { - LogContext.current().removeContextParameter(LogContext.MOSAIC_TRACE_ID_KEY); - LogContext.current().removeContextParameter(LogContext.MOSAIC_SPAN_ID_KEY); - } } } diff --git a/api/src/main/java/org/apache/cloudstack/context/LogContext.java b/api/src/main/java/org/apache/cloudstack/context/LogContext.java index 4828de90faec..e63022f40af2 100644 --- a/api/src/main/java/org/apache/cloudstack/context/LogContext.java +++ b/api/src/main/java/org/apache/cloudstack/context/LogContext.java @@ -16,6 +16,7 @@ // under the License. package org.apache.cloudstack.context; +import java.util.ArrayList; import java.util.HashMap; import java.util.Map; import java.util.UUID; @@ -91,8 +92,10 @@ public void removeContextParameter(String key) { } public void removeContextParameters() { - for (Map.Entry entry : context.entrySet()) { - removeContextParameter(entry.getKey()); + // Iterate over a copy of the keys: removeContextParameter mutates the context + // map, so iterating the live keySet/entrySet would throw ConcurrentModificationException. + for (String key : new ArrayList<>(context.keySet())) { + removeContextParameter(key); } } diff --git a/api/src/main/java/org/apache/cloudstack/context/TraceContextMdcWrapper.java b/api/src/main/java/org/apache/cloudstack/context/TraceContextMdcWrapper.java new file mode 100644 index 000000000000..2b44fe84e260 --- /dev/null +++ b/api/src/main/java/org/apache/cloudstack/context/TraceContextMdcWrapper.java @@ -0,0 +1,86 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you 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 org.apache.cloudstack.context; + +import org.apache.log4j.MDC; + +import io.opentelemetry.api.trace.Span; +import io.opentelemetry.api.trace.SpanContext; +import io.opentelemetry.context.Context; +import io.opentelemetry.context.ContextStorage; +import io.opentelemetry.context.Scope; + +/** + * Mirrors the active OpenTelemetry span onto the Log4j MDC so management-server log + * lines carry mosaic_trace_id and mosaic_span_id on every thread that has an active + * span (API requests, agent-command dispatch, async jobs), not just the servlet path. + * + * The OpenTelemetry agent populates the log MDC automatically for Log4j2 and Logback, + * but not for Log4j 1.2 (reload4j), which the management server uses. This wrapper + * fills that gap by hooking the OpenTelemetry context lifecycle: whenever a span + * becomes current on a thread it copies the ids into the MDC, and restores the + * previous values when that scope closes. Install once at startup via {@link #register()}. + */ +public class TraceContextMdcWrapper implements ContextStorage { + + private final ContextStorage delegate; + + TraceContextMdcWrapper(ContextStorage delegate) { + this.delegate = delegate; + } + + /** + * Install the wrapper. Must be called before the first OpenTelemetry context is + * used, i.e. at management-server startup, before the server accepts requests. + */ + public static void register() { + ContextStorage.addWrapper(TraceContextMdcWrapper::new); + } + + @Override + public Scope attach(Context toAttach) { + Object previousTraceId = MDC.get(LogContext.MOSAIC_TRACE_ID_KEY); + Object previousSpanId = MDC.get(LogContext.MOSAIC_SPAN_ID_KEY); + SpanContext spanContext = Span.fromContext(toAttach).getSpanContext(); + if (spanContext.isValid()) { + MDC.put(LogContext.MOSAIC_TRACE_ID_KEY, spanContext.getTraceId()); + MDC.put(LogContext.MOSAIC_SPAN_ID_KEY, spanContext.getSpanId()); + } else { + MDC.remove(LogContext.MOSAIC_TRACE_ID_KEY); + MDC.remove(LogContext.MOSAIC_SPAN_ID_KEY); + } + Scope delegateScope = delegate.attach(toAttach); + return () -> { + delegateScope.close(); + restore(LogContext.MOSAIC_TRACE_ID_KEY, previousTraceId); + restore(LogContext.MOSAIC_SPAN_ID_KEY, previousSpanId); + }; + } + + private static void restore(String key, Object previous) { + if (previous != null) { + MDC.put(key, previous); + } else { + MDC.remove(key); + } + } + + @Override + public Context current() { + return delegate.current(); + } +} diff --git a/api/src/test/java/org/apache/cloudstack/api/filter/ApiTraceFilterTest.java b/api/src/test/java/org/apache/cloudstack/api/filter/ApiTraceFilterTest.java deleted file mode 100644 index 4d5d8654995e..000000000000 --- a/api/src/test/java/org/apache/cloudstack/api/filter/ApiTraceFilterTest.java +++ /dev/null @@ -1,92 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you 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 org.apache.cloudstack.api.filter; - -import javax.servlet.FilterChain; -import javax.servlet.ServletResponse; -import javax.servlet.http.HttpServletRequest; - -import org.apache.cloudstack.context.LogContext; -import org.apache.log4j.MDC; -import org.junit.After; -import org.junit.Assert; -import org.junit.Test; -import org.mockito.Mockito; - -import io.opentelemetry.api.trace.Span; -import io.opentelemetry.api.trace.SpanContext; -import io.opentelemetry.api.trace.TraceFlags; -import io.opentelemetry.api.trace.TraceState; -import io.opentelemetry.context.Scope; - -public class ApiTraceFilterTest { - - private static final String TRACE_ID = "4bf92f3577b34da6a3ce929d0e0e4736"; - private static final String SPAN_ID = "00f067aa0ba902b7"; - - private final ApiTraceFilter filter = new ApiTraceFilter(); - - @After - public void tearDown() { - LogContext.unregister(); - } - - @Test - public void putsMosaicTraceContextOnMdcWhenSpanActive() throws Exception { - HttpServletRequest req = Mockito.mock(HttpServletRequest.class); - Mockito.when(req.getHeader(LogContext.X_B3_TRACEID_KEY)).thenReturn(null); - String[] duringChain = new String[2]; - FilterChain chain = (rq, rs) -> { - duringChain[0] = (String) MDC.get(LogContext.MOSAIC_TRACE_ID_KEY); - duringChain[1] = (String) MDC.get(LogContext.MOSAIC_SPAN_ID_KEY); - }; - - SpanContext spanContext = - SpanContext.create(TRACE_ID, SPAN_ID, TraceFlags.getSampled(), TraceState.getDefault()); - try (Scope scope = Span.wrap(spanContext).makeCurrent()) { - filter.doFilter(req, Mockito.mock(ServletResponse.class), chain); - } - - // Present on the MDC while the request is in flight, from the active span. - Assert.assertEquals(TRACE_ID, duringChain[0]); - Assert.assertEquals(SPAN_ID, duringChain[1]); - // Removed once the request completes, so pooled threads do not leak it. - Assert.assertNull(MDC.get(LogContext.MOSAIC_TRACE_ID_KEY)); - Assert.assertNull(MDC.get(LogContext.MOSAIC_SPAN_ID_KEY)); - } - - @Test - public void leavesMosaicKeysUnsetWhenNoActiveSpan() throws Exception { - HttpServletRequest req = Mockito.mock(HttpServletRequest.class); - Mockito.when(req.getHeader(LogContext.X_B3_TRACEID_KEY)).thenReturn(null); - boolean[] chainInvoked = {false}; - String[] duringChain = new String[1]; - FilterChain chain = (rq, rs) -> { - chainInvoked[0] = true; - duringChain[0] = (String) MDC.get(LogContext.MOSAIC_TRACE_ID_KEY); - }; - - // No span in scope: Span.current() is the invalid default span. - filter.doFilter(req, Mockito.mock(ServletResponse.class), chain); - - Assert.assertTrue(chainInvoked[0]); - Assert.assertNull(duringChain[0]); - Assert.assertNull(MDC.get(LogContext.MOSAIC_TRACE_ID_KEY)); - } -} diff --git a/api/src/test/java/org/apache/cloudstack/context/TraceContextMdcWrapperTest.java b/api/src/test/java/org/apache/cloudstack/context/TraceContextMdcWrapperTest.java new file mode 100644 index 000000000000..b1ad9dab3399 --- /dev/null +++ b/api/src/test/java/org/apache/cloudstack/context/TraceContextMdcWrapperTest.java @@ -0,0 +1,96 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you 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 org.apache.cloudstack.context; + +import org.apache.log4j.MDC; +import org.junit.After; +import org.junit.Assert; +import org.junit.Test; + +import io.opentelemetry.api.trace.Span; +import io.opentelemetry.api.trace.SpanContext; +import io.opentelemetry.api.trace.TraceFlags; +import io.opentelemetry.api.trace.TraceState; +import io.opentelemetry.context.Context; +import io.opentelemetry.context.ContextStorage; +import io.opentelemetry.context.Scope; + +public class TraceContextMdcWrapperTest { + + private static final String TRACE_ID = "4bf92f3577b34da6a3ce929d0e0e4736"; + private static final String SPAN_ID = "00f067aa0ba902b7"; + private static final String OTHER_TRACE_ID = "d75597dcda4f6e7b9c1a2b3c4d5e6f70"; + private static final String OTHER_SPAN_ID = "aabbccddeeff0011"; + + // Minimal delegate so we test the wrapper in isolation, no real context storage. + private final ContextStorage noopDelegate = new ContextStorage() { + @Override + public Scope attach(Context toAttach) { + return () -> { }; + } + + @Override + public Context current() { + return Context.root(); + } + }; + + private final TraceContextMdcWrapper wrapper = new TraceContextMdcWrapper(noopDelegate); + + @After + public void tearDown() { + MDC.remove(LogContext.MOSAIC_TRACE_ID_KEY); + MDC.remove(LogContext.MOSAIC_SPAN_ID_KEY); + } + + private static Context contextWithSpan(String traceId, String spanId) { + return Context.root().with(Span.wrap( + SpanContext.create(traceId, spanId, TraceFlags.getSampled(), TraceState.getDefault()))); + } + + @Test + public void putsTraceContextOnMdcWhileScopeOpenAndRestoresOnClose() { + Scope scope = wrapper.attach(contextWithSpan(TRACE_ID, SPAN_ID)); + Assert.assertEquals(TRACE_ID, MDC.get(LogContext.MOSAIC_TRACE_ID_KEY)); + Assert.assertEquals(SPAN_ID, MDC.get(LogContext.MOSAIC_SPAN_ID_KEY)); + + scope.close(); + Assert.assertNull(MDC.get(LogContext.MOSAIC_TRACE_ID_KEY)); + Assert.assertNull(MDC.get(LogContext.MOSAIC_SPAN_ID_KEY)); + } + + @Test + public void leavesMdcUnsetWhenNoActiveSpan() { + Scope scope = wrapper.attach(Context.root()); + Assert.assertNull(MDC.get(LogContext.MOSAIC_TRACE_ID_KEY)); + Assert.assertNull(MDC.get(LogContext.MOSAIC_SPAN_ID_KEY)); + scope.close(); + } + + @Test + public void restoresOuterSpanWhenNestedScopeCloses() { + Scope outer = wrapper.attach(contextWithSpan(TRACE_ID, SPAN_ID)); + Scope inner = wrapper.attach(contextWithSpan(OTHER_TRACE_ID, OTHER_SPAN_ID)); + Assert.assertEquals(OTHER_TRACE_ID, MDC.get(LogContext.MOSAIC_TRACE_ID_KEY)); + + inner.close(); + Assert.assertEquals(TRACE_ID, MDC.get(LogContext.MOSAIC_TRACE_ID_KEY)); + + outer.close(); + Assert.assertNull(MDC.get(LogContext.MOSAIC_TRACE_ID_KEY)); + } +} diff --git a/client/src/main/java/org/apache/cloudstack/ServerDaemon.java b/client/src/main/java/org/apache/cloudstack/ServerDaemon.java index 06477fff8986..4cdccdf51ba6 100644 --- a/client/src/main/java/org/apache/cloudstack/ServerDaemon.java +++ b/client/src/main/java/org/apache/cloudstack/ServerDaemon.java @@ -55,6 +55,8 @@ import com.cloud.utils.PropertiesUtil; import com.cloud.utils.server.ServerProperties; +import org.apache.cloudstack.context.TraceContextMdcWrapper; + /*** * The ServerDaemon class implements the embedded server, it can be started either * using JSVC or directly from the JAR along with additional jars not shaded in the uber-jar. @@ -108,6 +110,9 @@ public class ServerDaemon implements Daemon { ////////////////////////////////////////////////// public static void main(final String... anArgs) throws Exception { + // Install the trace-context to MDC hook before the server starts, so every + // thread with an active OpenTelemetry span carries mosaic_trace_id in its logs. + TraceContextMdcWrapper.register(); final ServerDaemon daemon = new ServerDaemon(); daemon.init(null); daemon.start(); From 90b020268090b2e3a525fed244459bc4c2066091 Mon Sep 17 00:00:00 2001 From: mprokopchuk Date: Wed, 23 Sep 2026 19:06:53 +0530 Subject: [PATCH 3/4] Support OpenTelemetry distributed tracing instrumentation - Add support to API layer - All API requests get a traceId in LogContext (via ApiTraceFilter) - Read the trace and span threadcontext key names from the environment - Instrument cloudstack Agents and VM operations --- agent/conf/log4j-cloud.xml.in | 4 +- api/pom.xml | 5 + .../cloudstack/api/filter/ApiTraceFilter.java | 31 ++++- .../apache/cloudstack/context/LogContext.java | 54 ++++++++- .../context/TraceContextMdcWrapper.java | 29 ++--- .../api-config/spring-api-config-context.xml | 1 + .../context/TraceContextMdcWrapperTest.java | 24 ++-- client/conf/log4j-cloud.xml.in | 10 +- .../org/apache/cloudstack/ServerDaemon.java | 2 +- client/src/main/webapp/WEB-INF/web.xml | 10 ++ .../threadcontext/ThreadContextUtil.java | 114 ++++++++++++++++++ engine/orchestration/pom.xml | 8 ++ .../com/cloud/agent/manager/AgentAttache.java | 20 +++ .../cloud/agent/manager/AgentManagerImpl.java | 25 ++++ .../cloud/vm/VirtualMachineManagerImpl.java | 32 ++++- .../cloud/upgrade/DatabaseUpgradeChecker.java | 2 + .../upgrade/dao/Upgrade42210to42220.java | 30 +++++ .../java/com/cloud/vm/dao/VMInstanceDao.java | 2 + .../com/cloud/vm/dao/VMInstanceDaoImpl.java | 14 +++ .../db/schema-42210to42220-cleanup.sql | 20 +++ .../META-INF/db/schema-42210to42220.sql | 22 ++++ framework/jobs/pom.xml | 6 + .../cloudstack/framework/jobs/AsyncJob.java | 2 + .../framework/jobs/dao/AsyncJobDaoImpl.java | 10 ++ .../jobs/impl/AsyncJobManagerImpl.java | 41 +++++++ .../framework/jobs/impl/AsyncJobVO.java | 12 ++ framework/spring/lifecycle/pom.xml | 5 + .../CloudStackExtendedLifeCycle.java | 76 +++++++++--- framework/spring/module/pom.xml | 5 + .../impl/DefaultModuleDefinitionSet.java | 57 +++++++-- server/conf/log4j-cloud.xml.in | 2 +- server/pom.xml | 2 + .../java/com/cloud/vm/UserVmManagerImpl.java | 90 +++++++++++--- usage/conf/log4j-cloud_usage.xml.in | 2 +- .../cloudstack/trace/TracingLabels.java | 37 ++++++ 35 files changed, 713 insertions(+), 93 deletions(-) create mode 100644 core/src/main/java/org/apache/cloudstack/threadcontext/ThreadContextUtil.java create mode 100644 engine/schema/src/main/java/com/cloud/upgrade/dao/Upgrade42210to42220.java create mode 100644 engine/schema/src/main/resources/META-INF/db/schema-42210to42220-cleanup.sql create mode 100644 engine/schema/src/main/resources/META-INF/db/schema-42210to42220.sql create mode 100644 utils/src/main/java/org/apache/cloudstack/trace/TracingLabels.java diff --git a/agent/conf/log4j-cloud.xml.in b/agent/conf/log4j-cloud.xml.in index 84957edca032..d18afdb33fe7 100644 --- a/agent/conf/log4j-cloud.xml.in +++ b/agent/conf/log4j-cloud.xml.in @@ -30,7 +30,7 @@ under the License. - + @@ -39,7 +39,7 @@ under the License. - + diff --git a/api/pom.xml b/api/pom.xml index d4f9e3d10083..feb2558f5f4b 100644 --- a/api/pom.xml +++ b/api/pom.xml @@ -71,6 +71,11 @@ cloud-framework-direct-download ${project.version} + + io.opentelemetry.instrumentation + opentelemetry-instrumentation-annotations + ${cs.opentelemetry-instrumentation.version} + io.opentelemetry opentelemetry-api diff --git a/api/src/main/java/org/apache/cloudstack/api/filter/ApiTraceFilter.java b/api/src/main/java/org/apache/cloudstack/api/filter/ApiTraceFilter.java index f67cf3f7ced1..79ef58694c36 100644 --- a/api/src/main/java/org/apache/cloudstack/api/filter/ApiTraceFilter.java +++ b/api/src/main/java/org/apache/cloudstack/api/filter/ApiTraceFilter.java @@ -32,6 +32,10 @@ import javax.servlet.ServletResponse; public class ApiTraceFilter implements Filter { + + // Cap the accepted trace id length to avoid log/DB bloat from a crafted header. + private static final int MAX_TRACE_ID_LENGTH = 128; + @Override public void init(FilterConfig filterConfig) throws ServletException { } @@ -41,16 +45,37 @@ public void doFilter(ServletRequest request, ServletResponse response, FilterCha throws IOException, ServletException { try { HttpServletRequest httpReq = (HttpServletRequest) request; - String traceId = httpReq.getHeader(LogContext.X_B3_TRACEID_KEY); + String traceId = sanitizeTraceId(httpReq.getHeader(LogContext.TRACEID_KEY)); if (StringUtils.isBlank(traceId)) { traceId = UUID.randomUUID().toString(); } - LogContext.current().putContextParameter(LogContext.X_B3_TRACEID_KEY, traceId); + LogContext.current().putContextParameter(LogContext.TRACEID_KEY, traceId); chain.doFilter(request, response); } finally { - LogContext.current().removeContextParameter(LogContext.X_B3_TRACEID_KEY); + LogContext.current().removeContextParameter(LogContext.TRACEID_KEY); + } + } + + /** + * Returns the caller-supplied trace id only if it is safe to log and store: no control + * characters (prevents log forging) and within a bounded length. Otherwise returns null so a + * fresh id is generated. + */ + private String sanitizeTraceId(String traceId) { + if (traceId == null) { + return null; + } + String trimmed = traceId.trim(); + if (trimmed.isEmpty() || trimmed.length() > MAX_TRACE_ID_LENGTH) { + return null; + } + for (int i = 0; i < trimmed.length(); i++) { + if (Character.isISOControl(trimmed.charAt(i))) { + return null; + } } + return trimmed; } @Override diff --git a/api/src/main/java/org/apache/cloudstack/context/LogContext.java b/api/src/main/java/org/apache/cloudstack/context/LogContext.java index e63022f40af2..2fdb5b4cd763 100644 --- a/api/src/main/java/org/apache/cloudstack/context/LogContext.java +++ b/api/src/main/java/org/apache/cloudstack/context/LogContext.java @@ -16,11 +16,16 @@ // under the License. package org.apache.cloudstack.context; +import java.io.File; +import java.io.IOException; import java.util.ArrayList; import java.util.HashMap; import java.util.Map; +import java.util.Properties; import java.util.UUID; +import com.cloud.utils.PropertiesUtil; +import com.cloud.utils.StringUtils; import org.apache.logging.log4j.Logger; import org.apache.logging.log4j.LogManager; @@ -54,9 +59,43 @@ public class LogContext { private long userId; private final Map context = new HashMap(); - public final static String X_B3_TRACEID_KEY = "traceid"; - public final static String MOSAIC_TRACE_ID_KEY = "mosaic_trace_id"; - public final static String MOSAIC_SPAN_ID_KEY = "mosaic_span_id"; + public final static String TRACEID_KEY = "traceid"; + + /** + * MDC keys under which the active OpenTelemetry ids are published. The names are + * deployment specific, so they are read from server.properties and fall back to a + * neutral default when the property is absent or blank. + */ + public final static String TRACE_ID_KEY_PROPERTY = "otel.trace.id.mdc.key"; + public final static String SPAN_ID_KEY_PROPERTY = "otel.span.id.mdc.key"; + + public final static String DEFAULT_TRACE_ID_KEY = "otel_trace_id"; + public final static String DEFAULT_SPAN_ID_KEY = "otel_span_id"; + + private final static Properties SERVER_PROPERTIES = loadServerProperties(); + + public final static String TRACE_ID_KEY = traceKeyFromProperties(); + public final static String SPAN_ID_KEY = spanKeyFromProperties(); + + private static Properties loadServerProperties() { + try { + File file = PropertiesUtil.findConfigFile("server.properties"); + return file == null ? new Properties() : PropertiesUtil.loadFromFile(file); + } catch (IOException e) { + LOGGER.warn("Could not read server.properties, using the default MDC key names", e); + return new Properties(); + } + } + + private static String traceKeyFromProperties() { + String value = SERVER_PROPERTIES.getProperty(TRACE_ID_KEY_PROPERTY); + return StringUtils.isBlank(value) ? DEFAULT_TRACE_ID_KEY : value.trim(); + } + + private static String spanKeyFromProperties() { + String value = SERVER_PROPERTIES.getProperty(SPAN_ID_KEY_PROPERTY); + return StringUtils.isBlank(value) ? DEFAULT_SPAN_ID_KEY : value.trim(); + } static EntityManager s_entityMgr; @@ -83,12 +122,17 @@ protected LogContext(User user, Account account, String logContextId) { public void putContextParameter(String key, String value) { context.put(key, value); - MDC.put(key, value); + ThreadContext.put(key, value); + if (value == null) { + ThreadContext.remove(key); + } else { + ThreadContext.put(key, value); + } } public void removeContextParameter(String key) { context.remove(key); - MDC.remove(key); + ThreadContext.remove(key); } public void removeContextParameters() { diff --git a/api/src/main/java/org/apache/cloudstack/context/TraceContextMdcWrapper.java b/api/src/main/java/org/apache/cloudstack/context/TraceContextMdcWrapper.java index 2b44fe84e260..26fb0897aeb2 100644 --- a/api/src/main/java/org/apache/cloudstack/context/TraceContextMdcWrapper.java +++ b/api/src/main/java/org/apache/cloudstack/context/TraceContextMdcWrapper.java @@ -16,18 +16,19 @@ // under the License. package org.apache.cloudstack.context; -import org.apache.log4j.MDC; - import io.opentelemetry.api.trace.Span; import io.opentelemetry.api.trace.SpanContext; import io.opentelemetry.context.Context; import io.opentelemetry.context.ContextStorage; import io.opentelemetry.context.Scope; +import org.apache.logging.log4j.ThreadContext; /** * Mirrors the active OpenTelemetry span onto the Log4j MDC so management-server log - * lines carry mosaic_trace_id and mosaic_span_id on every thread that has an active + * lines carry trace and span ids on every thread that has an active * span (API requests, agent-command dispatch, async jobs), not just the servlet path. + * The MDC key names come from {@link LogContext#TRACE_ID_KEY} and + * {@link LogContext#SPAN_ID_KEY}, which are environment driven. * * The OpenTelemetry agent populates the log MDC automatically for Log4j2 and Logback, * but not for Log4j 1.2 (reload4j), which the management server uses. This wrapper @@ -53,29 +54,29 @@ public static void register() { @Override public Scope attach(Context toAttach) { - Object previousTraceId = MDC.get(LogContext.MOSAIC_TRACE_ID_KEY); - Object previousSpanId = MDC.get(LogContext.MOSAIC_SPAN_ID_KEY); + String previousTraceId = ThreadContext.get(LogContext.TRACE_ID_KEY); + String previousSpanId = ThreadContext.get(LogContext.SPAN_ID_KEY); SpanContext spanContext = Span.fromContext(toAttach).getSpanContext(); if (spanContext.isValid()) { - MDC.put(LogContext.MOSAIC_TRACE_ID_KEY, spanContext.getTraceId()); - MDC.put(LogContext.MOSAIC_SPAN_ID_KEY, spanContext.getSpanId()); + ThreadContext.put(LogContext.TRACE_ID_KEY, spanContext.getTraceId()); + ThreadContext.put(LogContext.SPAN_ID_KEY, spanContext.getSpanId()); } else { - MDC.remove(LogContext.MOSAIC_TRACE_ID_KEY); - MDC.remove(LogContext.MOSAIC_SPAN_ID_KEY); + ThreadContext.remove(LogContext.TRACE_ID_KEY); + ThreadContext.remove(LogContext.SPAN_ID_KEY); } Scope delegateScope = delegate.attach(toAttach); return () -> { delegateScope.close(); - restore(LogContext.MOSAIC_TRACE_ID_KEY, previousTraceId); - restore(LogContext.MOSAIC_SPAN_ID_KEY, previousSpanId); + restore(LogContext.TRACE_ID_KEY, previousTraceId); + restore(LogContext.SPAN_ID_KEY, previousSpanId); }; } - private static void restore(String key, Object previous) { + private static void restore(String key, String previous) { if (previous != null) { - MDC.put(key, previous); + ThreadContext.put(key, previous); } else { - MDC.remove(key); + ThreadContext.remove(key); } } diff --git a/api/src/main/resources/META-INF/cloudstack/api-config/spring-api-config-context.xml b/api/src/main/resources/META-INF/cloudstack/api-config/spring-api-config-context.xml index 12d3c2361acd..1d25a1976adb 100644 --- a/api/src/main/resources/META-INF/cloudstack/api-config/spring-api-config-context.xml +++ b/api/src/main/resources/META-INF/cloudstack/api-config/spring-api-config-context.xml @@ -28,5 +28,6 @@ > + diff --git a/api/src/test/java/org/apache/cloudstack/context/TraceContextMdcWrapperTest.java b/api/src/test/java/org/apache/cloudstack/context/TraceContextMdcWrapperTest.java index b1ad9dab3399..0f6b92b04eed 100644 --- a/api/src/test/java/org/apache/cloudstack/context/TraceContextMdcWrapperTest.java +++ b/api/src/test/java/org/apache/cloudstack/context/TraceContextMdcWrapperTest.java @@ -16,7 +16,7 @@ // under the License. package org.apache.cloudstack.context; -import org.apache.log4j.MDC; +import org.apache.logging.log4j.ThreadContext; import org.junit.After; import org.junit.Assert; import org.junit.Test; @@ -53,8 +53,8 @@ public Context current() { @After public void tearDown() { - MDC.remove(LogContext.MOSAIC_TRACE_ID_KEY); - MDC.remove(LogContext.MOSAIC_SPAN_ID_KEY); + ThreadContext.remove(LogContext.TRACE_ID_KEY); + ThreadContext.remove(LogContext.SPAN_ID_KEY); } private static Context contextWithSpan(String traceId, String spanId) { @@ -65,19 +65,19 @@ private static Context contextWithSpan(String traceId, String spanId) { @Test public void putsTraceContextOnMdcWhileScopeOpenAndRestoresOnClose() { Scope scope = wrapper.attach(contextWithSpan(TRACE_ID, SPAN_ID)); - Assert.assertEquals(TRACE_ID, MDC.get(LogContext.MOSAIC_TRACE_ID_KEY)); - Assert.assertEquals(SPAN_ID, MDC.get(LogContext.MOSAIC_SPAN_ID_KEY)); + Assert.assertEquals(TRACE_ID, ThreadContext.get(LogContext.TRACE_ID_KEY)); + Assert.assertEquals(SPAN_ID, ThreadContext.get(LogContext.SPAN_ID_KEY)); scope.close(); - Assert.assertNull(MDC.get(LogContext.MOSAIC_TRACE_ID_KEY)); - Assert.assertNull(MDC.get(LogContext.MOSAIC_SPAN_ID_KEY)); + Assert.assertNull(ThreadContext.get(LogContext.TRACE_ID_KEY)); + Assert.assertNull(ThreadContext.get(LogContext.SPAN_ID_KEY)); } @Test public void leavesMdcUnsetWhenNoActiveSpan() { Scope scope = wrapper.attach(Context.root()); - Assert.assertNull(MDC.get(LogContext.MOSAIC_TRACE_ID_KEY)); - Assert.assertNull(MDC.get(LogContext.MOSAIC_SPAN_ID_KEY)); + Assert.assertNull(ThreadContext.get(LogContext.TRACE_ID_KEY)); + Assert.assertNull(ThreadContext.get(LogContext.SPAN_ID_KEY)); scope.close(); } @@ -85,12 +85,12 @@ public void leavesMdcUnsetWhenNoActiveSpan() { public void restoresOuterSpanWhenNestedScopeCloses() { Scope outer = wrapper.attach(contextWithSpan(TRACE_ID, SPAN_ID)); Scope inner = wrapper.attach(contextWithSpan(OTHER_TRACE_ID, OTHER_SPAN_ID)); - Assert.assertEquals(OTHER_TRACE_ID, MDC.get(LogContext.MOSAIC_TRACE_ID_KEY)); + Assert.assertEquals(OTHER_TRACE_ID, ThreadContext.get(LogContext.TRACE_ID_KEY)); inner.close(); - Assert.assertEquals(TRACE_ID, MDC.get(LogContext.MOSAIC_TRACE_ID_KEY)); + Assert.assertEquals(TRACE_ID, ThreadContext.get(LogContext.TRACE_ID_KEY)); outer.close(); - Assert.assertNull(MDC.get(LogContext.MOSAIC_TRACE_ID_KEY)); + Assert.assertNull(ThreadContext.get(LogContext.TRACE_ID_KEY)); } } diff --git a/client/conf/log4j-cloud.xml.in b/client/conf/log4j-cloud.xml.in index 26da171269de..2226483b4aec 100755 --- a/client/conf/log4j-cloud.xml.in +++ b/client/conf/log4j-cloud.xml.in @@ -34,7 +34,7 @@ under the License. - + @@ -43,7 +43,7 @@ under the License. - + @@ -52,7 +52,7 @@ under the License. - + @@ -61,7 +61,7 @@ under the License. - + @@ -70,7 +70,7 @@ under the License. - + diff --git a/client/src/main/java/org/apache/cloudstack/ServerDaemon.java b/client/src/main/java/org/apache/cloudstack/ServerDaemon.java index 4cdccdf51ba6..3fe0d734b513 100644 --- a/client/src/main/java/org/apache/cloudstack/ServerDaemon.java +++ b/client/src/main/java/org/apache/cloudstack/ServerDaemon.java @@ -111,7 +111,7 @@ public class ServerDaemon implements Daemon { public static void main(final String... anArgs) throws Exception { // Install the trace-context to MDC hook before the server starts, so every - // thread with an active OpenTelemetry span carries mosaic_trace_id in its logs. + // thread with an active OpenTelemetry span carries the trace id in its logs. TraceContextMdcWrapper.register(); final ServerDaemon daemon = new ServerDaemon(); daemon.init(null); diff --git a/client/src/main/webapp/WEB-INF/web.xml b/client/src/main/webapp/WEB-INF/web.xml index fdb899b55562..28bff0f3a515 100644 --- a/client/src/main/webapp/WEB-INF/web.xml +++ b/client/src/main/webapp/WEB-INF/web.xml @@ -64,6 +64,16 @@ 6 + + apiTraceFilter + org.apache.cloudstack.api.filter.ApiTraceFilter + + + + apiTraceFilter + /api/* + + apiServlet /api/* diff --git a/core/src/main/java/org/apache/cloudstack/threadcontext/ThreadContextUtil.java b/core/src/main/java/org/apache/cloudstack/threadcontext/ThreadContextUtil.java new file mode 100644 index 000000000000..c0b9973c8f14 --- /dev/null +++ b/core/src/main/java/org/apache/cloudstack/threadcontext/ThreadContextUtil.java @@ -0,0 +1,114 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you 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 org.apache.cloudstack.threadcontext; + +import com.google.gson.Gson; +import com.google.gson.reflect.TypeToken; +import org.apache.commons.lang3.StringUtils; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; +import org.apache.logging.log4j.ThreadContext; + +import java.lang.reflect.Type; +import java.util.HashMap; +import java.util.Map; + +/** + * Utility class, helps to propagate {@link ThreadContext} values from parent to child threads. + * + * @author mprokopchuk + */ +public class ThreadContextUtil { + private static final Logger logger = LogManager.getLogger(ThreadContextUtil.class); + + public static final String MDC_UUID_KEY = "uuid"; + public static final String MDC_LOG_CONTEXT_ID_KEY = "logcontextid"; + + /** + * Wrap {@link Runnable} to propagate {@link ThreadContext} values. + * + * @param delegate + * @return + */ + public static Runnable wrapThreadContext(Runnable delegate) { + @SuppressWarnings("unchecked") + Map context = ThreadContext.getContext() != null ? + new HashMap<>(ThreadContext.getContext()) : null; + + return () -> { + @SuppressWarnings("unchecked") + Map oldContext = ThreadContext.getContext() != null ? + new HashMap<>(ThreadContext.getContext()) : null; + try { + ThreadContext.clearMap(); + if (context != null) { + context.forEach(ThreadContext::put); + } + delegate.run(); + } finally { + ThreadContext.clearMap(); + if (oldContext != null) { + oldContext.forEach(ThreadContext::put); + } + } + }; + } + + /** + * Set UUID in MDC context. + * + * @param uuid the UUID value to set + */ + public static void setUuid(String uuid) { + if (StringUtils.isNotEmpty(uuid)) { + ThreadContext.put(MDC_UUID_KEY, uuid); + } + } + + /** + * Set log context ID in MDC context. + * + * @param logContextId the log context ID value to set + */ + public static void setLogContextId(String logContextId) { + if (StringUtils.isNotEmpty(logContextId)) { + ThreadContext.put(MDC_LOG_CONTEXT_ID_KEY, logContextId); + } + } + + /** + * Extract UUID from JSON cmdInfo string and set it in MDC if UUID is not already present. + * This is specifically used for async job processing. + * + * @param cmdInfo the JSON string containing command info + */ + public static void extractAndSetUuidFromCmdInfo(String cmdInfo) { + if (StringUtils.isBlank((String) ThreadContext.get(MDC_UUID_KEY)) && StringUtils.isNotBlank(cmdInfo)) { + try { + Type mapType = new TypeToken>() {}.getType(); + Gson gson = new Gson(); + Map params = gson.fromJson(cmdInfo, mapType); + String entityUuid = params.get(MDC_UUID_KEY); + if (StringUtils.isNotBlank(entityUuid)) { + ThreadContext.put(MDC_UUID_KEY, entityUuid); + } + } catch (Exception e) { + logger.warn("Failed to extract UUID from cmdInfo: {}", cmdInfo, e); + } + } + } +} diff --git a/engine/orchestration/pom.xml b/engine/orchestration/pom.xml index 0f321be6bd60..4e82869a90bd 100755 --- a/engine/orchestration/pom.xml +++ b/engine/orchestration/pom.xml @@ -68,6 +68,14 @@ cloud-server ${project.version} + + io.opentelemetry + opentelemetry-api + + + io.opentelemetry.instrumentation + opentelemetry-instrumentation-annotations + org.apache.cloudstack cloud-plugin-maintenance diff --git a/engine/orchestration/src/main/java/com/cloud/agent/manager/AgentAttache.java b/engine/orchestration/src/main/java/com/cloud/agent/manager/AgentAttache.java index 402bd2b6b9b9..7290e23449f3 100644 --- a/engine/orchestration/src/main/java/com/cloud/agent/manager/AgentAttache.java +++ b/engine/orchestration/src/main/java/com/cloud/agent/manager/AgentAttache.java @@ -36,9 +36,13 @@ import com.cloud.hypervisor.Hypervisor.HypervisorType; import com.cloud.utils.Pair; import com.cloud.utils.exception.CloudRuntimeException; +import io.opentelemetry.api.trace.Span; +import io.opentelemetry.api.trace.SpanKind; +import io.opentelemetry.instrumentation.annotations.WithSpan; import org.apache.cloudstack.agent.lb.SetupMSListCommand; import org.apache.cloudstack.command.ReconcileAnswer; import org.apache.cloudstack.managed.context.ManagedContextRunnable; +import org.apache.cloudstack.trace.TracingLabels; import org.apache.cloudstack.utils.reflectiontostringbuilderutils.ReflectionToStringBuilderUtils; import org.apache.logging.log4j.Logger; import org.apache.logging.log4j.LogManager; @@ -409,7 +413,9 @@ public void send(final Request req, final Listener listener) throws AgentUnavail } } + @WithSpan(kind = SpanKind.CLIENT) public Answer[] send(final Request req, final int wait) throws AgentUnavailableException, OperationTimedoutException { + setSpanAttributes(req); SynchronousListener sl = new SynchronousListener(null); long seq = req.getSequence(); @@ -477,6 +483,20 @@ public Answer[] send(final Request req, final int wait) throws AgentUnavailableE } } + private void setSpanAttributes(final Request req) { + final Command[] spanCmds = req.getCommands(); + final String commandName = (spanCmds != null && spanCmds.length > 0 && spanCmds[0] != null) + ? spanCmds[0].getClass().getSimpleName() + : "UNKNOWN"; + + final Span span = Span.current(); + span.updateName("agent.out." + commandName); + span.setAttribute(TracingLabels.TRAFFIC, TracingLabels.TRAFFIC_HYPERVISOR); + span.setAttribute(TracingLabels.AGENT_COMMAND, commandName); + span.setAttribute(TracingLabels.HOST_ID, _id); + span.setAttribute(TracingLabels.AGENT_CALL, true); + } + private Answer[] waitForAnswerOfReconcileCommand(SynchronousListener sl, final long seq, final Command command, final int wait) { Answer[] answers = null; int waitTimeLeft = wait; diff --git a/engine/orchestration/src/main/java/com/cloud/agent/manager/AgentManagerImpl.java b/engine/orchestration/src/main/java/com/cloud/agent/manager/AgentManagerImpl.java index ecb789a15bd5..1b4fc25b1577 100644 --- a/engine/orchestration/src/main/java/com/cloud/agent/manager/AgentManagerImpl.java +++ b/engine/orchestration/src/main/java/com/cloud/agent/manager/AgentManagerImpl.java @@ -43,6 +43,9 @@ import javax.naming.ConfigurationException; import com.cloud.utils.StringUtils; +import io.opentelemetry.api.trace.Span; +import io.opentelemetry.api.trace.SpanKind; +import io.opentelemetry.instrumentation.annotations.WithSpan; import org.apache.cloudstack.agent.lb.IndirectAgentLB; import org.apache.cloudstack.ca.CAManager; import org.apache.cloudstack.command.ReconcileCommandService; @@ -60,6 +63,7 @@ import org.apache.cloudstack.managed.context.ManagedContextRunnable; import org.apache.cloudstack.management.ManagementServerHost; import org.apache.cloudstack.outofbandmanagement.dao.OutOfBandManagementDao; +import org.apache.cloudstack.trace.TracingLabels; import org.apache.cloudstack.utils.identity.ManagementServerNode; import org.apache.cloudstack.utils.reflectiontostringbuilderutils.ReflectionToStringBuilderUtils; import org.apache.commons.collections.MapUtils; @@ -1647,12 +1651,23 @@ private void processPingRoutingCommand(PingRoutingCommand pingRoutingCommand, lo processHostHealthCheckResult(hostHealthCheckResult, hostId); } + @WithSpan(kind = SpanKind.SERVER) protected void processRequest(final Link link, final Request request) { final AgentAttache attache = (AgentAttache)link.attachment(); final Command[] cmds = request.getCommands(); + if (cmds == null || cmds.length == 0) { + logger.warn("Received request with no commands: {}", request); + return; + } Command cmd = cmds[0]; boolean logD = true; + if (cmd != null && cmd.getContextParam("logid") != null) { + ThreadContext.put("logcontextid", cmd.getContextParam("logid")); + } + + setSpanAttributes(cmd, attache); + if (attache == null) { if (!(cmd instanceof StartupCommand)) { logger.warn("Throwing away a request because it came through as the first command on a connect: {}", request); @@ -1782,6 +1797,16 @@ protected void processRequest(final Link link, final Request request) { } } + private void setSpanAttributes(Command cmd, AgentAttache attache) { + final Span span = Span.current(); + final String commandName = cmd != null ? cmd.getClass().getSimpleName() : "UNKNOWN"; + span.updateName("agent.in." + commandName); + span.setAttribute(TracingLabels.TRAFFIC, TracingLabels.TRAFFIC_HYPERVISOR); + span.setAttribute(TracingLabels.AGENT_COMMAND, commandName); + span.setAttribute(TracingLabels.HOST_ID, attache != null ? attache.getId() : -1L); + span.setAttribute(TracingLabels.AGENT_CALL, true); + } + protected void processResponse(final Link link, final Response response) { final AgentAttache attache = (AgentAttache)link.attachment(); if (attache == null) { diff --git a/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java b/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java index c98391a654db..e9d512b83aef 100755 --- a/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java +++ b/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java @@ -50,6 +50,10 @@ import javax.naming.ConfigurationException; import javax.persistence.EntityExistsException; +import io.opentelemetry.api.baggage.Baggage; +import io.opentelemetry.api.trace.Span; +import io.opentelemetry.context.Scope; +import io.opentelemetry.instrumentation.annotations.WithSpan; import org.apache.cloudstack.affinity.dao.AffinityGroupVMMapDao; import org.apache.cloudstack.annotation.AnnotationService; import org.apache.cloudstack.annotation.dao.AnnotationDao; @@ -99,6 +103,7 @@ import org.apache.cloudstack.storage.datastore.db.SnapshotDataStoreDao; import org.apache.cloudstack.storage.datastore.db.StoragePoolVO; import org.apache.cloudstack.storage.to.VolumeObjectTO; +import org.apache.cloudstack.trace.TracingLabels; import org.apache.cloudstack.utils.cache.SingleCache; import org.apache.cloudstack.utils.identity.ManagementServerNode; import org.apache.cloudstack.utils.reflectiontostringbuilderutils.ReflectionToStringBuilderUtils; @@ -6159,8 +6164,33 @@ private Pair orchestrateStorageMigration(final VmWorkSto } @Override + @WithSpan public Pair handleVmWorkJob(final VmWork work) throws Exception { - return _jobHandlerProxy.handleVmWorkJob(work); + Span span = setSpanAttributes(work); + final String op = work.getClass().getSimpleName(); + final String vmId = String.valueOf(work.getVmId()); + try (Scope ignored = Baggage.current().toBuilder() + .put(TracingLabels.TRAFFIC, TracingLabels.TRAFFIC_HYPERVISOR) + .put(TracingLabels.VM_OP, op) + .put(TracingLabels.VM_ID, vmId) + .build().makeCurrent()) { + final Pair result = _jobHandlerProxy.handleVmWorkJob(work); + final JobInfo.Status status = (result != null) ? result.first() : null; + span.setAttribute(TracingLabels.JOB_RESULT, status != null ? status.name() : "UNKNOWN"); + return result; + } + } + + private Span setSpanAttributes(final VmWork work) { + final Span span = Span.current(); + final String op = work.getClass().getSimpleName(); + final String vmId = String.valueOf(work.getVmId()); + span.updateName(op); + span.setAttribute(TracingLabels.TRAFFIC, TracingLabels.TRAFFIC_HYPERVISOR); + span.setAttribute(TracingLabels.VM_OP, op); + span.setAttribute(TracingLabels.VM_ID, vmId); + span.setAttribute(TracingLabels.OP_ROOT, true); + return span; } private VmWorkJobVO createPlaceHolderWork(final long instanceId) { diff --git a/engine/schema/src/main/java/com/cloud/upgrade/DatabaseUpgradeChecker.java b/engine/schema/src/main/java/com/cloud/upgrade/DatabaseUpgradeChecker.java index 3868ca960e06..ee19b89268cb 100644 --- a/engine/schema/src/main/java/com/cloud/upgrade/DatabaseUpgradeChecker.java +++ b/engine/schema/src/main/java/com/cloud/upgrade/DatabaseUpgradeChecker.java @@ -33,6 +33,7 @@ import javax.inject.Inject; +import com.cloud.upgrade.dao.Upgrade42210to42220; import com.cloud.utils.FileUtil; import org.apache.cloudstack.utils.CloudStackVersion; import org.apache.commons.lang3.StringUtils; @@ -246,6 +247,7 @@ public DatabaseUpgradeChecker() { .next("4.20.4.0", new Upgrade42040to42100()) .next("4.21.0.0", new Upgrade42100to42200()) .next("4.22.0.0", new Upgrade42200to42210()) + .next("4.22.1.0", new Upgrade42210to42220()) .build(); } diff --git a/engine/schema/src/main/java/com/cloud/upgrade/dao/Upgrade42210to42220.java b/engine/schema/src/main/java/com/cloud/upgrade/dao/Upgrade42210to42220.java new file mode 100644 index 000000000000..392dc4d4a3b5 --- /dev/null +++ b/engine/schema/src/main/java/com/cloud/upgrade/dao/Upgrade42210to42220.java @@ -0,0 +1,30 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you 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.cloud.upgrade.dao; + +public class Upgrade42210to42220 extends DbUpgradeAbstractImpl implements DbUpgrade, DbUpgradeSystemVmTemplate { + + @Override + public String[] getUpgradableVersionRange() { + return new String[] {"4.22.1.0", "4.22.2.0"}; + } + + @Override + public String getUpgradedVersion() { + return "4.22.2.0"; + } +} diff --git a/engine/schema/src/main/java/com/cloud/vm/dao/VMInstanceDao.java b/engine/schema/src/main/java/com/cloud/vm/dao/VMInstanceDao.java index 4fd3e729e0d2..fbc4ea669e51 100755 --- a/engine/schema/src/main/java/com/cloud/vm/dao/VMInstanceDao.java +++ b/engine/schema/src/main/java/com/cloud/vm/dao/VMInstanceDao.java @@ -197,4 +197,6 @@ List searchRemovedByRemoveDate(final Date startDate, final Date en List listDeleteProtectedVmsByAccountId(long accountId); List listDeleteProtectedVmsByDomainIds(Set domainIds); + + List listByIds(List ids); } diff --git a/engine/schema/src/main/java/com/cloud/vm/dao/VMInstanceDaoImpl.java b/engine/schema/src/main/java/com/cloud/vm/dao/VMInstanceDaoImpl.java index d8c9b9253c89..af4b247154b0 100755 --- a/engine/schema/src/main/java/com/cloud/vm/dao/VMInstanceDaoImpl.java +++ b/engine/schema/src/main/java/com/cloud/vm/dao/VMInstanceDaoImpl.java @@ -21,6 +21,7 @@ import java.sql.SQLException; import java.util.ArrayList; import java.util.Collection; +import java.util.Collections; import java.util.Date; import java.util.HashMap; import java.util.List; @@ -1336,4 +1337,17 @@ public List listDeleteProtectedVmsByDomainIds(Set domainIds) Filter filter = new Filter(VMInstanceVO.class, null, false, 0L, 10L); return listBy(sc, filter); } + + @Override + public List listByIds(List ids) { + if (CollectionUtils.isEmpty(ids)) { + return Collections.emptyList(); + } + SearchBuilder sb = createSearchBuilder(); + sb.and("id", sb.entity().getId(), Op.IN); + sb.done(); + SearchCriteria sc = sb.create(); + sc.setParameters("id", ids.toArray()); + return listBy(sc); + } } diff --git a/engine/schema/src/main/resources/META-INF/db/schema-42210to42220-cleanup.sql b/engine/schema/src/main/resources/META-INF/db/schema-42210to42220-cleanup.sql new file mode 100644 index 000000000000..85563b6daf03 --- /dev/null +++ b/engine/schema/src/main/resources/META-INF/db/schema-42210to42220-cleanup.sql @@ -0,0 +1,20 @@ +-- Licensed to the Apache Software Foundation (ASF) under one +-- or more contributor license agreements. See the NOTICE file +-- distributed with this work for additional information +-- regarding copyright ownership. The ASF licenses this file +-- to you 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. + +--; +-- Schema upgrade cleanup from 4.22.1.0 to 4.22.2.0 +--; diff --git a/engine/schema/src/main/resources/META-INF/db/schema-42210to42220.sql b/engine/schema/src/main/resources/META-INF/db/schema-42210to42220.sql new file mode 100644 index 000000000000..fdc6b3d0e38a --- /dev/null +++ b/engine/schema/src/main/resources/META-INF/db/schema-42210to42220.sql @@ -0,0 +1,22 @@ +-- Licensed to the Apache Software Foundation (ASF) under one +-- or more contributor license agreements. See the NOTICE file +-- distributed with this work for additional information +-- regarding copyright ownership. The ASF licenses this file +-- to you 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. + +--; +-- Schema upgrade from 4.22.1.0 to 4.22.2.0 +--; + +ALTER TABLE `cloud`.`async_job` ADD COLUMN context TEXT; diff --git a/framework/jobs/pom.xml b/framework/jobs/pom.xml index f584af90c6b9..6be36965e394 100644 --- a/framework/jobs/pom.xml +++ b/framework/jobs/pom.xml @@ -73,5 +73,11 @@ commons-io test + + org.apache.cloudstack + cloud-core + ${project.version} + compile + diff --git a/framework/jobs/src/main/java/org/apache/cloudstack/framework/jobs/AsyncJob.java b/framework/jobs/src/main/java/org/apache/cloudstack/framework/jobs/AsyncJob.java index bde9b4af1671..69783e3ad742 100644 --- a/framework/jobs/src/main/java/org/apache/cloudstack/framework/jobs/AsyncJob.java +++ b/framework/jobs/src/main/java/org/apache/cloudstack/framework/jobs/AsyncJob.java @@ -119,4 +119,6 @@ public static interface Constants { void setSyncSource(SyncQueueItem item); String getRelated(); + + String getContextJson(); } diff --git a/framework/jobs/src/main/java/org/apache/cloudstack/framework/jobs/dao/AsyncJobDaoImpl.java b/framework/jobs/src/main/java/org/apache/cloudstack/framework/jobs/dao/AsyncJobDaoImpl.java index 81cc5d4f2a8c..8d923d519cac 100644 --- a/framework/jobs/src/main/java/org/apache/cloudstack/framework/jobs/dao/AsyncJobDaoImpl.java +++ b/framework/jobs/src/main/java/org/apache/cloudstack/framework/jobs/dao/AsyncJobDaoImpl.java @@ -21,7 +21,9 @@ import java.util.Date; import java.util.List; +import com.google.gson.Gson; import org.apache.cloudstack.api.ApiConstants; +import org.apache.cloudstack.context.LogContext; import org.apache.commons.collections.CollectionUtils; import org.apache.commons.lang3.StringUtils; @@ -299,4 +301,12 @@ public long countPendingJobs(String havingInfo, String... cmds) { List results = customSearch(sc, null); return results.get(0); } + + @Override + public AsyncJobVO persist(AsyncJobVO job) { + if (job.getContextJson() == null) { + job.setContextJson(new Gson().toJson(LogContext.current().getContextParameters())); + } + return super.persist(job); + } } diff --git a/framework/jobs/src/main/java/org/apache/cloudstack/framework/jobs/impl/AsyncJobManagerImpl.java b/framework/jobs/src/main/java/org/apache/cloudstack/framework/jobs/impl/AsyncJobManagerImpl.java index 1cb1cb4e309f..37698f317b1a 100644 --- a/framework/jobs/src/main/java/org/apache/cloudstack/framework/jobs/impl/AsyncJobManagerImpl.java +++ b/framework/jobs/src/main/java/org/apache/cloudstack/framework/jobs/impl/AsyncJobManagerImpl.java @@ -20,6 +20,7 @@ import static com.cloud.utils.HumanReadableJson.getHumanReadableBytesJson; import java.io.Serializable; +import java.lang.reflect.Type; import java.util.Arrays; import java.util.Collections; import java.util.Date; @@ -35,10 +36,14 @@ import javax.inject.Inject; import javax.naming.ConfigurationException; +import com.google.gson.Gson; +import com.google.gson.JsonParseException; +import com.google.gson.reflect.TypeToken; import org.apache.cloudstack.api.ApiCommandResourceType; import org.apache.cloudstack.api.ApiErrorCode; import org.apache.cloudstack.command.ReconcileCommandService; import org.apache.cloudstack.context.CallContext; +import org.apache.cloudstack.context.LogContext; import org.apache.cloudstack.engine.orchestration.service.NetworkOrchestrationService; import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotDataFactory; import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotInfo; @@ -64,6 +69,7 @@ import org.apache.cloudstack.jobs.JobInfo.Status; import org.apache.cloudstack.managed.context.ManagedContextRunnable; import org.apache.cloudstack.management.ManagementServerHost; +import org.apache.cloudstack.threadcontext.ThreadContextUtil; import org.apache.cloudstack.utils.identity.ManagementServerNode; import org.apache.logging.log4j.ThreadContext; @@ -659,6 +665,7 @@ protected void runInContext() { AsyncJobExecutionContext.setCurrentExecutionContext(new AsyncJobExecutionContext(job)); String related = job.getRelated(); String logContext = job.getShortUuid(); + String contextJson = job.getContextJson(); if (related != null && !related.isEmpty()) { AsyncJob relatedJob = _jobDao.findByIdIncludingRemoved(Long.parseLong(related)); if (relatedJob != null) { @@ -667,6 +674,34 @@ protected void runInContext() { } ThreadContext.put("logcontextid", logContext); + if (StringUtils.isNotBlank(contextJson)) { + try { + Type type = new TypeToken>() { + }.getType(); + Map ctx = new Gson().fromJson(contextJson, type); + LogContext.current().putContextParameters(ctx); + // don't fail the job due to logs + } catch (JsonParseException e) { + logger.warn(String.format("Failed to parse %s, log context won't be updated", contextJson), e); + } + } + + if (StringUtils.isBlank((String) ThreadContext.get(ThreadContextUtil.MDC_UUID_KEY))) { + AsyncJob jobToCheck = job; + logger.debug("Updating UUID MDC value"); + + // If current job has no cmdInfo and has a related parent job, check parent instead + if (StringUtils.isNotBlank(related)) { + AsyncJob parentJob = _jobDao.findByIdIncludingRemoved(Long.parseLong(related)); + if (parentJob != null && StringUtils.isNotBlank(parentJob.getCmdInfo())) { + jobToCheck = parentJob; + } + } + + // Extract entity UUID from the selected job + ThreadContextUtil.extractAndSetUuidFromCmdInfo(jobToCheck.getCmdInfo()); + } + // execute the job if (logger.isDebugEnabled()) { logger.debug("Executing " + StringUtils.cleanString(job.toString())); @@ -721,6 +756,12 @@ protected void runInContext() { AsyncJobExecutionContext.unregister(); _jobMonitor.unregisterActiveTask(runNumber); + LogContext.current().removeContextParameters(); + // These MDC keys are set directly (not via LogContext), so clear them here + // as well; otherwise a value set for one job leaks into later jobs on this + // pooled worker thread. + ThreadContext.remove(ThreadContextUtil.MDC_UUID_KEY); + ThreadContext.remove("logcontextid"); } catch (Throwable e) { logger.error("Double exception", e); } diff --git a/framework/jobs/src/main/java/org/apache/cloudstack/framework/jobs/impl/AsyncJobVO.java b/framework/jobs/src/main/java/org/apache/cloudstack/framework/jobs/impl/AsyncJobVO.java index 4ef7876f8030..dae413775cbf 100644 --- a/framework/jobs/src/main/java/org/apache/cloudstack/framework/jobs/impl/AsyncJobVO.java +++ b/framework/jobs/src/main/java/org/apache/cloudstack/framework/jobs/impl/AsyncJobVO.java @@ -129,6 +129,9 @@ public class AsyncJobVO implements AsyncJob, JobInfo { @Column(name = "uuid") private String uuid; + @Column(name = "context", length = 65535) + private String contextJson; + @Transient private SyncQueueItem syncSource = null; @@ -384,6 +387,15 @@ public void setRemoved(final Date removed) { this.removed = removed; } + @Override + public String getContextJson() { + return contextJson; + } + + public void setContextJson(String contextJson) { + this.contextJson = contextJson; + } + @Override public String toString() { return String.format("AsyncJob %s", diff --git a/framework/spring/lifecycle/pom.xml b/framework/spring/lifecycle/pom.xml index af3dca3047e4..9d8101921e8f 100644 --- a/framework/spring/lifecycle/pom.xml +++ b/framework/spring/lifecycle/pom.xml @@ -38,5 +38,10 @@ cloud-framework-config ${project.version} + + io.opentelemetry + opentelemetry-api + ${cs.opentelemetry.version} + diff --git a/framework/spring/lifecycle/src/main/java/org/apache/cloudstack/spring/lifecycle/CloudStackExtendedLifeCycle.java b/framework/spring/lifecycle/src/main/java/org/apache/cloudstack/spring/lifecycle/CloudStackExtendedLifeCycle.java index bd3e424f7673..82053603be40 100644 --- a/framework/spring/lifecycle/src/main/java/org/apache/cloudstack/spring/lifecycle/CloudStackExtendedLifeCycle.java +++ b/framework/spring/lifecycle/src/main/java/org/apache/cloudstack/spring/lifecycle/CloudStackExtendedLifeCycle.java @@ -29,6 +29,10 @@ import javax.management.NotCompliantMBeanException; import javax.naming.ConfigurationException; +import io.opentelemetry.api.GlobalOpenTelemetry; +import io.opentelemetry.api.trace.Span; +import io.opentelemetry.api.trace.Tracer; +import io.opentelemetry.context.Context; import com.cloud.utils.component.ComponentLifecycle; import com.cloud.utils.component.SystemIntegrityChecker; @@ -38,6 +42,7 @@ public class CloudStackExtendedLifeCycle extends AbstractBeanCollector { + private static final Tracer tracer = GlobalOpenTelemetry.getTracer("org.apache.cloudstack.spring.lifecycle"); Map> sorted = new TreeMap<>(); @@ -66,29 +71,60 @@ protected void checkIntegrity() { public void startBeans() { logger.info("Starting CloudStack Components"); - with(new WithComponentLifeCycle() { - @Override - public void with(ComponentLifecycle lifecycle) { - logger.info("starting bean {}.", lifecycle.getName()); - try { - lifecycle.start(); - } catch (Exception e) { - logger.error("Error on starting bean [{}] due to: {}", lifecycle.getName(), e); - throw new CloudRuntimeException("Failed to start bean [" + lifecycle.getName() + "]"); - } - - if (lifecycle instanceof ManagementBean) { - ManagementBean mbean = (ManagementBean)lifecycle; + // Boot spans are tagged cloudstack.phase=startup so a stateless collector + // filter can extract the boot trace to the debug view. They are deliberately + // NEVER made current: several beans schedule periodic DB pollers during + // start(), and the OTel agent captures the current context at schedule time. + // If this span were current, every future poll would re-parent under the boot + // trace and it would never close. We thread the parent Context explicitly + // (setParent) so our own child spans link correctly without the context + // leaking onto those background executors. Do NOT add makeCurrent() here. + Span rootSpan = tracer.spanBuilder("startup.beans.start") + .setAttribute("cloudstack.phase", "startup") + .startSpan(); + final Context beansCtx = Context.current().with(rootSpan); + + try { + with(new WithComponentLifeCycle() { + @Override + public void with(ComponentLifecycle lifecycle) { + String beanName = lifecycle.getName(); + if (beanName == null) { + beanName = lifecycle.getClass().getSimpleName(); + } + logger.info("starting bean {}.", beanName); + Span span = tracer.spanBuilder("startup.bean.start") + .setParent(beansCtx) + .setAttribute("cloudstack.phase", "startup") + .setAttribute("bean.name", beanName) + .startSpan(); + long start = System.currentTimeMillis(); try { - JmxUtil.registerMBean(mbean); - } catch (MalformedObjectNameException | InstanceAlreadyExistsException | - MBeanRegistrationException | NotCompliantMBeanException e) { - logger.warn("Unable to register MBean: " + mbean.getName(), e); + lifecycle.start(); + } catch (Exception e) { + logger.error("Error on starting bean [{}] due to: {}", beanName, e.getMessage(), e); + throw new CloudRuntimeException("Failed to start bean [" + beanName + "]"); + } finally { + span.end(); + } + logger.info("bean [{}] started in {} ms", beanName, System.currentTimeMillis() - start); + + if (lifecycle instanceof ManagementBean) { + ManagementBean mbean = (ManagementBean)lifecycle; + try { + JmxUtil.registerMBean(mbean); + } catch (MalformedObjectNameException | InstanceAlreadyExistsException | + MBeanRegistrationException | NotCompliantMBeanException e) { + logger.warn("Unable to register MBean: {}", mbean.getName(), e); + } + logger.info("Registered MBean: {}", mbean.getName()); } - logger.info("Registered MBean: " + mbean.getName()); } - } - }); + }); + } finally { + rootSpan.end(); + } + logger.info("Done Starting CloudStack Components"); } diff --git a/framework/spring/module/pom.xml b/framework/spring/module/pom.xml index ccd2c5efb161..90bd07a46fdb 100644 --- a/framework/spring/module/pom.xml +++ b/framework/spring/module/pom.xml @@ -47,5 +47,10 @@ provided true + + io.opentelemetry + opentelemetry-api + ${cs.opentelemetry.version} + diff --git a/framework/spring/module/src/main/java/org/apache/cloudstack/spring/module/model/impl/DefaultModuleDefinitionSet.java b/framework/spring/module/src/main/java/org/apache/cloudstack/spring/module/model/impl/DefaultModuleDefinitionSet.java index 78693f72140c..ddcfc7227a7e 100644 --- a/framework/spring/module/src/main/java/org/apache/cloudstack/spring/module/model/impl/DefaultModuleDefinitionSet.java +++ b/framework/spring/module/src/main/java/org/apache/cloudstack/spring/module/model/impl/DefaultModuleDefinitionSet.java @@ -32,6 +32,11 @@ import java.util.Set; import java.util.Stack; +import io.opentelemetry.api.GlobalOpenTelemetry; +import io.opentelemetry.api.trace.Span; +import io.opentelemetry.api.trace.Tracer; +import io.opentelemetry.context.Context; + import org.apache.commons.io.IOUtils; import org.apache.logging.log4j.Logger; import org.apache.logging.log4j.LogManager; @@ -51,6 +56,8 @@ public class DefaultModuleDefinitionSet implements ModuleDefinitionSet { protected Logger logger = LogManager.getLogger(getClass()); + private static final Tracer tracer = GlobalOpenTelemetry.getTracer("org.apache.cloudstack.spring.module"); + public static final String DEFAULT_CONFIG_RESOURCES = "DefaultConfigResources"; public static final String DEFAULT_CONFIG_PROPERTIES = "DefaultConfigProperties"; public static final String MODULES_EXCLUDE = "modules.exclude"; @@ -64,6 +71,7 @@ public class DefaultModuleDefinitionSet implements ModuleDefinitionSet { ApplicationContext rootContext = null; Set excludes = new HashSet(); Properties configProperties = null; + Context loadCtx = null; public DefaultModuleDefinitionSet(Map modules, String root) { super(); @@ -72,11 +80,26 @@ public DefaultModuleDefinitionSet(Map modules, String } public void load() throws IOException { - if (!loadRootContext()) - return; + // Tagged cloudstack.phase=startup for the boot-trace debug filter, and + // deliberately never made current — parent Context is threaded explicitly via + // setParent below. Making startup spans current re-parents beans' periodic DB + // pollers under the boot trace so it never closes (see CloudStackExtendedLifeCycle + // .startBeans). Do NOT add makeCurrent() here. + Span loadSpan = tracer.spanBuilder("startup.modules.load") + .setAttribute("cloudstack.phase", "startup") + .startSpan(); + loadCtx = Context.current().with(loadSpan); + try { + if (!loadRootContext()) + return; + + printHierarchy(); + loadContexts(); + } finally { + loadSpan.end(); + loadCtx = null; + } - printHierarchy(); - loadContexts(); startContexts(); } @@ -161,18 +184,28 @@ protected ApplicationContext loadContext(ModuleDefinition def, ApplicationContex context.setParent(parent); context.setClassLoader(def.getClassLoader()); + Context parentCtx = loadCtx != null ? loadCtx : Context.current(); + Span span = tracer.spanBuilder("startup.module.load") + .setParent(parentCtx) + .setAttribute("cloudstack.phase", "startup") + .setAttribute("module.name", def.getName()) + .startSpan(); long start = System.currentTimeMillis(); - if (logger.isInfoEnabled()) { - for (Resource resource : resources) { - logger.info("Loading module context [" + def.getName() + "] from " + resource); + try { + if (logger.isInfoEnabled()) { + for (Resource resource : resources) { + logger.info("Loading module context [{}] from {}", def.getName(), resource); + } } - } - context.refresh(); - logger.info("Loaded module context [" + def.getName() + "] in " + (System.currentTimeMillis() - start) + " ms"); + context.refresh(); + logger.info("Loaded module context [{}] in {} ms", def.getName(), System.currentTimeMillis() - start); - contexts.put(def.getName(), context); + contexts.put(def.getName(), context); - return context; + return context; + } finally { + span.end(); + } } protected boolean shouldLoad(ModuleDefinition def) { diff --git a/server/conf/log4j-cloud.xml.in b/server/conf/log4j-cloud.xml.in index 9a8e5dc7bf33..5f6412610d20 100755 --- a/server/conf/log4j-cloud.xml.in +++ b/server/conf/log4j-cloud.xml.in @@ -40,7 +40,7 @@ under the License. - + diff --git a/server/pom.xml b/server/pom.xml index 19cc0ca4583d..57469027e288 100644 --- a/server/pom.xml +++ b/server/pom.xml @@ -200,10 +200,12 @@ io.opentelemetry.instrumentation opentelemetry-instrumentation-annotations + ${cs.opentelemetry-instrumentation.version} io.opentelemetry opentelemetry-api + ${cs.opentelemetry.version} diff --git a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java index 7636ef6b152c..14aab77599f7 100644 --- a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java +++ b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java @@ -60,6 +60,13 @@ import javax.xml.parsers.DocumentBuilder; import javax.xml.parsers.ParserConfigurationException; +import com.cloud.utils.Profiler; +import io.opentelemetry.api.GlobalOpenTelemetry; +import io.opentelemetry.api.baggage.Baggage; +import io.opentelemetry.api.trace.Span; +import io.opentelemetry.api.trace.Tracer; +import io.opentelemetry.context.Scope; + import org.apache.cloudstack.acl.ControlledEntity; import org.apache.cloudstack.acl.ControlledEntity.ACLType; import org.apache.cloudstack.acl.SecurityChecker.AccessType; @@ -416,6 +423,8 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, VirtualMachineGuru, Configurable { + private static final Tracer tracer = GlobalOpenTelemetry.getTracer("com.cloud.vm"); + /** * The number of seconds to wait before timing out when trying to acquire a global lock. */ @@ -2513,28 +2522,77 @@ public boolean start() { } private void loadVmDetailsInMapForExternalDhcpIp() { - - List networks = _networkDao.listByGuestType(Network.GuestType.Shared); - networks.addAll(_networkDao.listByGuestType(Network.GuestType.L2)); - - for (NetworkVO network: networks) { - if (GuestType.L2.equals(network.getGuestType()) || _networkModel.isSharedNetworkWithoutServices(network.getId())) { - List nics = _nicDao.listByNetworkId(network.getId()); - - for (NicVO nic : nics) { - if (nic.getIPv4Address() == null) { - long nicId = nic.getId(); - long vmId = nic.getInstanceId(); - VMInstanceVO vmInstance = _vmInstanceDao.findById(vmId); + try { + Profiler profiler = new Profiler(); + profiler.start(); + + Span methodSpan = tracer.spanBuilder("startup.loadVmDetailsForExternalDhcpIp") + .setAttribute("cloudstack.phase", "startup") + .startSpan(); + // Unlike the bean/module startup spans (never made current — see + // CloudStackExtendedLifeCycle.startBeans), we DO make phase=startup baggage + // current here so the OTel agent copies it onto the auto-instrumented DB child + // spans below (via BaggageSpanProcessor), tying this slow ~35-min scan's queries + // to the boot trace. Safe because this method schedules no periodic pollers of + // its own, and start()'s executors were scheduled before this scope, so no + // background task captured this baggage. + try (Scope methodScope = methodSpan.makeCurrent(); + Scope phaseScope = Baggage.current().toBuilder() + .put("cloudstack.phase", "startup").build().makeCurrent()) { + List networks = _networkDao.listByGuestType(Network.GuestType.Shared); + networks.addAll(_networkDao.listByGuestType(Network.GuestType.L2)); + methodSpan.setAttribute("shared.network.count", networks.size()); + Map offeringWithoutServices = new HashMap<>(); + int networksScanned = 0; + int nicsAdded = 0; + + for (NetworkVO network: networks) { + boolean withoutServices = GuestType.L2.equals(network.getGuestType()) + || _networkModel.isSharedNetworkWithoutServices(network.getId()) + || offeringWithoutServices.computeIfAbsent(network.getNetworkOfferingId(), + offeringId -> _networkModel.listNetworkOfferingServices(offeringId).isEmpty()); + if (!withoutServices) { + continue; + } + networksScanned++; + + Span networkSpan = tracer.spanBuilder("startup.loadVmDetails.network") + .setAttribute("cloudstack.phase", "startup") + .setAttribute("network.id", network.getId()) + .startSpan(); + try (Scope networkScope = networkSpan.makeCurrent()) { + List nullIpNics = _nicDao.listByNetworkId(network.getId()).stream() + .filter(nic -> nic.getIPv4Address() == null) + .collect(Collectors.toList()); + if (nullIpNics.isEmpty()) { + continue; + } // only load running vms. For stopped vms get loaded on starting - if (vmInstance != null && vmInstance.getState() == State.Running) { - VmAndCountDetails vmAndCount = new VmAndCountDetails(vmId, VmIpFetchTrialMax.value()); - vmIdCountMap.put(nicId, vmAndCount); + List vmIds = nullIpNics.stream().map(NicVO::getInstanceId).distinct().collect(Collectors.toList()); + Map runningVmsById = _vmInstanceDao.listByIds(vmIds).stream() + .filter(vm -> vm != null && vm.getState() == State.Running) + .collect(Collectors.toMap(VMInstanceVO::getId, vm -> vm)); + + for (NicVO nic : nullIpNics) { + if (runningVmsById.containsKey(nic.getInstanceId())) { + vmIdCountMap.put(nic.getId(), new VmAndCountDetails(nic.getInstanceId(), VmIpFetchTrialMax.value())); + nicsAdded++; + } } + } finally { + networkSpan.end(); } } + + profiler.stop(); + logger.info("External-DHCP VM-IP map seeded: {} shared-without-service networks, {} nics added, took {} ms", + networksScanned, nicsAdded, profiler.getDurationInMillis()); + } finally { + methodSpan.end(); } + } catch (Exception e) { + logger.error("Failed to seed external-DHCP VM-IP retrieval map", e); } } diff --git a/usage/conf/log4j-cloud_usage.xml.in b/usage/conf/log4j-cloud_usage.xml.in index 871d6fb5a7a6..2f9689cec664 100644 --- a/usage/conf/log4j-cloud_usage.xml.in +++ b/usage/conf/log4j-cloud_usage.xml.in @@ -39,7 +39,7 @@ under the License. - + diff --git a/utils/src/main/java/org/apache/cloudstack/trace/TracingLabels.java b/utils/src/main/java/org/apache/cloudstack/trace/TracingLabels.java new file mode 100644 index 000000000000..be445172f0a2 --- /dev/null +++ b/utils/src/main/java/org/apache/cloudstack/trace/TracingLabels.java @@ -0,0 +1,37 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you 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 org.apache.cloudstack.trace; + +/** + * Shared OpenTelemetry span attribute and baggage keys (and common values) + * used by CloudStack tracing instrumentation. + */ +public final class TracingLabels { + private TracingLabels() { + } + + public static final String TRAFFIC = "cloudstack.traffic"; + public static final String AGENT_COMMAND = "cloudstack.agent.command"; + public static final String HOST_ID = "cloudstack.host.id"; + public static final String AGENT_CALL = "cloudstack.agent.call"; + public static final String VM_OP = "cloudstack.vm.op"; + public static final String VM_ID = "cloudstack.vm.id"; + public static final String OP_ROOT = "cloudstack.op.root"; + public static final String JOB_RESULT = "cloudstack.job.result"; + + public static final String TRAFFIC_HYPERVISOR = "hypervisor"; +} From f96c8577f2cfdfbf3035c922cc0a261134c8dcf9 Mon Sep 17 00:00:00 2001 From: Suresh Kumar Anaparti Date: Fri, 25 Sep 2026 11:41:04 +0530 Subject: [PATCH 4/4] log4j2 and otel trace config changes --- api/pom.xml | 9 ++ .../context/TraceContextMdcWrapper.java | 87 ----------------- .../context/ThreadContextInheritanceTest.java | 63 ++++++++++++ .../context/TraceContextMdcWrapperTest.java | 96 ------------------- client/conf/log4j-cloud.xml.in | 10 +- client/pom.xml | 4 + .../org/apache/cloudstack/ServerDaemon.java | 5 - client/src/main/webapp/WEB-INF/web.xml | 10 -- server/conf/log4j-cloud.xml.in | 2 +- 9 files changed, 82 insertions(+), 204 deletions(-) delete mode 100644 api/src/main/java/org/apache/cloudstack/context/TraceContextMdcWrapper.java create mode 100644 api/src/test/java/org/apache/cloudstack/context/ThreadContextInheritanceTest.java delete mode 100644 api/src/test/java/org/apache/cloudstack/context/TraceContextMdcWrapperTest.java diff --git a/api/pom.xml b/api/pom.xml index feb2558f5f4b..c9b4b2c15fa0 100644 --- a/api/pom.xml +++ b/api/pom.xml @@ -83,6 +83,15 @@ + + org.apache.maven.plugins + maven-surefire-plugin + + + true + + + org.apache.maven.plugins maven-jar-plugin diff --git a/api/src/main/java/org/apache/cloudstack/context/TraceContextMdcWrapper.java b/api/src/main/java/org/apache/cloudstack/context/TraceContextMdcWrapper.java deleted file mode 100644 index 26fb0897aeb2..000000000000 --- a/api/src/main/java/org/apache/cloudstack/context/TraceContextMdcWrapper.java +++ /dev/null @@ -1,87 +0,0 @@ -// Licensed to the Apache Software Foundation (ASF) under one -// or more contributor license agreements. See the NOTICE file -// distributed with this work for additional information -// regarding copyright ownership. The ASF licenses this file -// to you 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 org.apache.cloudstack.context; - -import io.opentelemetry.api.trace.Span; -import io.opentelemetry.api.trace.SpanContext; -import io.opentelemetry.context.Context; -import io.opentelemetry.context.ContextStorage; -import io.opentelemetry.context.Scope; -import org.apache.logging.log4j.ThreadContext; - -/** - * Mirrors the active OpenTelemetry span onto the Log4j MDC so management-server log - * lines carry trace and span ids on every thread that has an active - * span (API requests, agent-command dispatch, async jobs), not just the servlet path. - * The MDC key names come from {@link LogContext#TRACE_ID_KEY} and - * {@link LogContext#SPAN_ID_KEY}, which are environment driven. - * - * The OpenTelemetry agent populates the log MDC automatically for Log4j2 and Logback, - * but not for Log4j 1.2 (reload4j), which the management server uses. This wrapper - * fills that gap by hooking the OpenTelemetry context lifecycle: whenever a span - * becomes current on a thread it copies the ids into the MDC, and restores the - * previous values when that scope closes. Install once at startup via {@link #register()}. - */ -public class TraceContextMdcWrapper implements ContextStorage { - - private final ContextStorage delegate; - - TraceContextMdcWrapper(ContextStorage delegate) { - this.delegate = delegate; - } - - /** - * Install the wrapper. Must be called before the first OpenTelemetry context is - * used, i.e. at management-server startup, before the server accepts requests. - */ - public static void register() { - ContextStorage.addWrapper(TraceContextMdcWrapper::new); - } - - @Override - public Scope attach(Context toAttach) { - String previousTraceId = ThreadContext.get(LogContext.TRACE_ID_KEY); - String previousSpanId = ThreadContext.get(LogContext.SPAN_ID_KEY); - SpanContext spanContext = Span.fromContext(toAttach).getSpanContext(); - if (spanContext.isValid()) { - ThreadContext.put(LogContext.TRACE_ID_KEY, spanContext.getTraceId()); - ThreadContext.put(LogContext.SPAN_ID_KEY, spanContext.getSpanId()); - } else { - ThreadContext.remove(LogContext.TRACE_ID_KEY); - ThreadContext.remove(LogContext.SPAN_ID_KEY); - } - Scope delegateScope = delegate.attach(toAttach); - return () -> { - delegateScope.close(); - restore(LogContext.TRACE_ID_KEY, previousTraceId); - restore(LogContext.SPAN_ID_KEY, previousSpanId); - }; - } - - private static void restore(String key, String previous) { - if (previous != null) { - ThreadContext.put(key, previous); - } else { - ThreadContext.remove(key); - } - } - - @Override - public Context current() { - return delegate.current(); - } -} diff --git a/api/src/test/java/org/apache/cloudstack/context/ThreadContextInheritanceTest.java b/api/src/test/java/org/apache/cloudstack/context/ThreadContextInheritanceTest.java new file mode 100644 index 000000000000..e94c7ac6ed67 --- /dev/null +++ b/api/src/test/java/org/apache/cloudstack/context/ThreadContextInheritanceTest.java @@ -0,0 +1,63 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you 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 org.apache.cloudstack.context; + +import java.util.concurrent.atomic.AtomicReference; + +import org.apache.logging.log4j.ThreadContext; +import org.junit.After; +import org.junit.Assert; +import org.junit.Test; + +/** + * Log4j 1.x backed the MDC with an InheritableThreadLocal, so a thread spawned while an API + * request was being served saw the request's trace id for free. Log4j2 uses a plain ThreadLocal + * unless log4j2.isThreadContextMapInheritable is set, and the ids would silently vanish from + * every thread a request spawns. The management server sets the flag in JAVA_OPTS + * (packaging/systemd/cloudstack-management.default) and surefire sets it for this module; this + * test fails if either is dropped. + */ +public class ThreadContextInheritanceTest { + + private static final String TRACE_ID = "trace-from-parent"; + + @After + public void tearDown() { + ThreadContext.clearMap(); + } + + @Test + public void childThreadInheritsContextOfSpawningThread() throws InterruptedException { + ThreadContext.put(LogContext.TRACEID_KEY, TRACE_ID); + + AtomicReference seenByChild = new AtomicReference<>(); + Thread child = new Thread(() -> seenByChild.set(ThreadContext.get(LogContext.TRACEID_KEY))); + child.start(); + child.join(); + + Assert.assertEquals(TRACE_ID, seenByChild.get()); + } + + @Test + public void childThreadDoesNotLeakContextBackToParent() throws InterruptedException { + Thread child = new Thread(() -> ThreadContext.put(LogContext.TRACEID_KEY, "trace-from-child")); + child.start(); + child.join(); + + Assert.assertNull(ThreadContext.get(LogContext.TRACEID_KEY)); + } +} diff --git a/api/src/test/java/org/apache/cloudstack/context/TraceContextMdcWrapperTest.java b/api/src/test/java/org/apache/cloudstack/context/TraceContextMdcWrapperTest.java deleted file mode 100644 index 0f6b92b04eed..000000000000 --- a/api/src/test/java/org/apache/cloudstack/context/TraceContextMdcWrapperTest.java +++ /dev/null @@ -1,96 +0,0 @@ -// Licensed to the Apache Software Foundation (ASF) under one -// or more contributor license agreements. See the NOTICE file -// distributed with this work for additional information -// regarding copyright ownership. The ASF licenses this file -// to you 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 org.apache.cloudstack.context; - -import org.apache.logging.log4j.ThreadContext; -import org.junit.After; -import org.junit.Assert; -import org.junit.Test; - -import io.opentelemetry.api.trace.Span; -import io.opentelemetry.api.trace.SpanContext; -import io.opentelemetry.api.trace.TraceFlags; -import io.opentelemetry.api.trace.TraceState; -import io.opentelemetry.context.Context; -import io.opentelemetry.context.ContextStorage; -import io.opentelemetry.context.Scope; - -public class TraceContextMdcWrapperTest { - - private static final String TRACE_ID = "4bf92f3577b34da6a3ce929d0e0e4736"; - private static final String SPAN_ID = "00f067aa0ba902b7"; - private static final String OTHER_TRACE_ID = "d75597dcda4f6e7b9c1a2b3c4d5e6f70"; - private static final String OTHER_SPAN_ID = "aabbccddeeff0011"; - - // Minimal delegate so we test the wrapper in isolation, no real context storage. - private final ContextStorage noopDelegate = new ContextStorage() { - @Override - public Scope attach(Context toAttach) { - return () -> { }; - } - - @Override - public Context current() { - return Context.root(); - } - }; - - private final TraceContextMdcWrapper wrapper = new TraceContextMdcWrapper(noopDelegate); - - @After - public void tearDown() { - ThreadContext.remove(LogContext.TRACE_ID_KEY); - ThreadContext.remove(LogContext.SPAN_ID_KEY); - } - - private static Context contextWithSpan(String traceId, String spanId) { - return Context.root().with(Span.wrap( - SpanContext.create(traceId, spanId, TraceFlags.getSampled(), TraceState.getDefault()))); - } - - @Test - public void putsTraceContextOnMdcWhileScopeOpenAndRestoresOnClose() { - Scope scope = wrapper.attach(contextWithSpan(TRACE_ID, SPAN_ID)); - Assert.assertEquals(TRACE_ID, ThreadContext.get(LogContext.TRACE_ID_KEY)); - Assert.assertEquals(SPAN_ID, ThreadContext.get(LogContext.SPAN_ID_KEY)); - - scope.close(); - Assert.assertNull(ThreadContext.get(LogContext.TRACE_ID_KEY)); - Assert.assertNull(ThreadContext.get(LogContext.SPAN_ID_KEY)); - } - - @Test - public void leavesMdcUnsetWhenNoActiveSpan() { - Scope scope = wrapper.attach(Context.root()); - Assert.assertNull(ThreadContext.get(LogContext.TRACE_ID_KEY)); - Assert.assertNull(ThreadContext.get(LogContext.SPAN_ID_KEY)); - scope.close(); - } - - @Test - public void restoresOuterSpanWhenNestedScopeCloses() { - Scope outer = wrapper.attach(contextWithSpan(TRACE_ID, SPAN_ID)); - Scope inner = wrapper.attach(contextWithSpan(OTHER_TRACE_ID, OTHER_SPAN_ID)); - Assert.assertEquals(OTHER_TRACE_ID, ThreadContext.get(LogContext.TRACE_ID_KEY)); - - inner.close(); - Assert.assertEquals(TRACE_ID, ThreadContext.get(LogContext.TRACE_ID_KEY)); - - outer.close(); - Assert.assertNull(ThreadContext.get(LogContext.TRACE_ID_KEY)); - } -} diff --git a/client/conf/log4j-cloud.xml.in b/client/conf/log4j-cloud.xml.in index 2226483b4aec..60ef7648fb22 100755 --- a/client/conf/log4j-cloud.xml.in +++ b/client/conf/log4j-cloud.xml.in @@ -34,7 +34,7 @@ under the License. - + @@ -43,7 +43,7 @@ under the License. - + @@ -52,7 +52,7 @@ under the License. - + @@ -61,7 +61,7 @@ under the License. - + @@ -70,7 +70,7 @@ under the License. - + diff --git a/client/pom.xml b/client/pom.xml index 85519e16c2c5..aee856a96276 100644 --- a/client/pom.xml +++ b/client/pom.xml @@ -750,6 +750,10 @@ log4j2.configurationFile log4j-cloud.xml + + log4j2.isThreadContextMapInheritable + true + diff --git a/client/src/main/java/org/apache/cloudstack/ServerDaemon.java b/client/src/main/java/org/apache/cloudstack/ServerDaemon.java index 3fe0d734b513..06477fff8986 100644 --- a/client/src/main/java/org/apache/cloudstack/ServerDaemon.java +++ b/client/src/main/java/org/apache/cloudstack/ServerDaemon.java @@ -55,8 +55,6 @@ import com.cloud.utils.PropertiesUtil; import com.cloud.utils.server.ServerProperties; -import org.apache.cloudstack.context.TraceContextMdcWrapper; - /*** * The ServerDaemon class implements the embedded server, it can be started either * using JSVC or directly from the JAR along with additional jars not shaded in the uber-jar. @@ -110,9 +108,6 @@ public class ServerDaemon implements Daemon { ////////////////////////////////////////////////// public static void main(final String... anArgs) throws Exception { - // Install the trace-context to MDC hook before the server starts, so every - // thread with an active OpenTelemetry span carries the trace id in its logs. - TraceContextMdcWrapper.register(); final ServerDaemon daemon = new ServerDaemon(); daemon.init(null); daemon.start(); diff --git a/client/src/main/webapp/WEB-INF/web.xml b/client/src/main/webapp/WEB-INF/web.xml index 28bff0f3a515..fdb899b55562 100644 --- a/client/src/main/webapp/WEB-INF/web.xml +++ b/client/src/main/webapp/WEB-INF/web.xml @@ -64,16 +64,6 @@ 6 - - apiTraceFilter - org.apache.cloudstack.api.filter.ApiTraceFilter - - - - apiTraceFilter - /api/* - - apiServlet /api/* diff --git a/server/conf/log4j-cloud.xml.in b/server/conf/log4j-cloud.xml.in index 5f6412610d20..30cbdc3c7c80 100755 --- a/server/conf/log4j-cloud.xml.in +++ b/server/conf/log4j-cloud.xml.in @@ -40,7 +40,7 @@ under the License. - +