diff --git a/CHANGELOG.md b/CHANGELOG.md index 81e1ef49d..2fb05c317 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,3 +11,4 @@ - [fix: prepend context-path to contract reporter API paths](https://github.com/Tencent/spring-cloud-tencent/pull/1803) - [feat: support overrideHost configuration for ratelimit, event reporter and stat modules](https://github.com/Tencent/spring-cloud-tencent/pull/1804) - [fix: split contract base-package for springdoc scan](https://github.com/Tencent/spring-cloud-tencent/pull/1807) +- [fix: inject baggage as W3C header for reactive clients to avoid OTel scope leak](https://github.com/Tencent/spring-cloud-tencent/pull/1810) diff --git a/spring-cloud-tencent-plugin-starters/spring-cloud-starter-tencent-trace-plugin/src/main/java/com/tencent/cloud/plugin/trace/TraceClientPreEnhancedPlugin.java b/spring-cloud-tencent-plugin-starters/spring-cloud-starter-tencent-trace-plugin/src/main/java/com/tencent/cloud/plugin/trace/TraceClientPreEnhancedPlugin.java index 52cc7e348..0a3fb6f88 100644 --- a/spring-cloud-tencent-plugin-starters/spring-cloud-starter-tencent-trace-plugin/src/main/java/com/tencent/cloud/plugin/trace/TraceClientPreEnhancedPlugin.java +++ b/spring-cloud-tencent-plugin-starters/spring-cloud-starter-tencent-trace-plugin/src/main/java/com/tencent/cloud/plugin/trace/TraceClientPreEnhancedPlugin.java @@ -23,6 +23,7 @@ import java.util.Map; import com.tencent.cloud.plugin.trace.attribute.SpanAttributesProvider; import com.tencent.cloud.polaris.context.PolarisSDKContextManager; +import com.tencent.cloud.rpc.enhancement.plugin.EnhancedContextKeys; import com.tencent.cloud.rpc.enhancement.plugin.EnhancedPlugin; import com.tencent.cloud.rpc.enhancement.plugin.EnhancedPluginContext; import com.tencent.cloud.rpc.enhancement.plugin.EnhancedPluginType; @@ -58,7 +59,21 @@ public class TraceClientPreEnhancedPlugin implements EnhancedPlugin { } } } + if (CollectionUtils.isEmpty(attributes)) { + return; + } + + // Reactive scenarios (SCG / WebClient): only stage the baggage attributes to be written; the + // downstream interceptor injects them as a W3C baggage header on the outgoing request, so no + // OTel Scope is attached here and there is no ThreadLocal lifecycle to close across async + // boundaries. + if (isReactiveScenario(context)) { + context.getExtraData().put(EnhancedContextKeys.PENDING_BAGGAGE_ATTRIBUTES_KEY, attributes); + return; + } + // Blocking scenarios (Feign / RestTemplate): keep the original semantics, attaching to the + // current thread so the FINALLY stage can close it. TraceAttributes traceAttributes = new TraceAttributes(); traceAttributes.setAttributes(attributes); traceAttributes.setAttributeLocation(TraceAttributes.AttributeLocation.BAGGAGE); @@ -71,6 +86,27 @@ public class TraceClientPreEnhancedPlugin implements EnhancedPlugin { } } + /** + * Whether current invocation is a reactive client scenario (SCG / WebClient). Matched by class + * name prefix to avoid a hard dependency on spring-webflux / spring-cloud-gateway. + * + * @param context enhanced plugin context + * @return true if the current call is a reactive scenario + */ + private boolean isReactiveScenario(EnhancedPluginContext context) { + Object origin = context.getOriginRequest(); + if (origin == null) { + return false; + } + String className = origin.getClass().getName(); + // SCG: org.springframework.web.server.ServerWebExchange implementation + // WebClient: org.springframework.web.reactive.function.client.ClientRequest implementation + return className.startsWith("org.springframework.web.server.") + || className.startsWith("org.springframework.mock.web.server.") + || className.startsWith("org.springframework.web.reactive.function.client.") + || className.startsWith("org.springframework.cloud.gateway."); + } + @Override public int getOrder() { return PluginOrderConstant.ClientPluginOrder.TRACE_CLIENT_PLUGIN_ORDER; diff --git a/spring-cloud-tencent-rpc-enhancement/src/main/java/com/tencent/cloud/rpc/enhancement/instrument/scg/EnhancedGatewayGlobalFilter.java b/spring-cloud-tencent-rpc-enhancement/src/main/java/com/tencent/cloud/rpc/enhancement/instrument/scg/EnhancedGatewayGlobalFilter.java index 721e8fd64..003916a0e 100644 --- a/spring-cloud-tencent-rpc-enhancement/src/main/java/com/tencent/cloud/rpc/enhancement/instrument/scg/EnhancedGatewayGlobalFilter.java +++ b/spring-cloud-tencent-rpc-enhancement/src/main/java/com/tencent/cloud/rpc/enhancement/instrument/scg/EnhancedGatewayGlobalFilter.java @@ -31,6 +31,7 @@ import com.tencent.cloud.rpc.enhancement.plugin.EnhancedPluginType; import com.tencent.cloud.rpc.enhancement.plugin.EnhancedRequestContext; import com.tencent.cloud.rpc.enhancement.plugin.EnhancedResponseContext; import com.tencent.cloud.rpc.enhancement.util.EnhancedPluginUtils; +import com.tencent.cloud.rpc.enhancement.util.OtelBaggageScopeHelper; import com.tencent.polaris.api.utils.CollectionUtils; import com.tencent.polaris.api.utils.StringUtils; import com.tencent.polaris.circuitbreak.client.exception.CallAbortedException; @@ -163,11 +164,23 @@ public class EnhancedGatewayGlobalFilter implements GlobalFilter, Ordered { } // Exchange may be changed in plugin ServerWebExchange exchange = (ServerWebExchange) enhancedPluginContext.getOriginRequest(); - return chain.filter(exchange) - .doOnSubscribe(v -> { - setTargetServiceInstance(exchange, enhancedPluginContext, serviceId); - pluginRunner.run(EnhancedPluginType.Client.BEFORE_CALLING, enhancedPluginContext); - }) + + // Run BEFORE_CALLING synchronously, before chain.filter, instead of inside doOnSubscribe as + // before. TraceClientPreEnhancedPlugin stages the baggage attributes into + // enhancedPluginContext.extraData (PENDING_BAGGAGE_ATTRIBUTES_KEY) here so they can be written + // into the downstream request headers below, which must happen before the request is sent. + // Moving it earlier is safe today: setTargetServiceInstance only needs GATEWAY_REQUEST_URL_ATTR, + // already rewritten by ReactiveLoadBalancerClientFilter (lower order, runs first), and the only + // reactive BEFORE_CALLING plugin is the trace plugin. A future plugin that relies on + // subscribe-time context would observe a different timing. + setTargetServiceInstance(exchange, enhancedPluginContext, serviceId); + pluginRunner.run(EnhancedPluginType.Client.BEFORE_CALLING, enhancedPluginContext); + + // Inject the pending baggage as a W3C baggage header on the downstream request before + // subscribing. This avoids any dependency on the OTel Context ThreadLocal lifecycle and + // prevents Scope.close from failing across async boundaries. + ServerWebExchange downstreamExchange = injectPendingBaggageHeader(exchange, enhancedPluginContext); + return chain.filter(downstreamExchange) .doOnSuccess(v -> { MetadataContext metadataContextOnSuccess = originExchange.getAttribute( MetadataConstant.HeaderName.METADATA_CONTEXT); @@ -177,8 +190,8 @@ public class EnhancedGatewayGlobalFilter implements GlobalFilter, Ordered { enhancedPluginContext.setDelay(System.currentTimeMillis() - startTime); EnhancedResponseContext enhancedResponseContext = EnhancedResponseContext.builder() - .httpStatus(exchange.getResponse().getRawStatusCode()) - .httpHeaders(exchange.getResponse().getHeaders()) + .httpStatus(downstreamExchange.getResponse().getRawStatusCode()) + .httpHeaders(downstreamExchange.getResponse().getHeaders()) .build(); enhancedPluginContext.setResponse(enhancedResponseContext); @@ -210,6 +223,26 @@ public class EnhancedGatewayGlobalFilter implements GlobalFilter, Ordered { }); } + /** + * Inject pending baggage attributes into the downstream request as a W3C baggage HTTP header. + * This avoids any dependency on the OTel Context ThreadLocal lifecycle and prevents Scope.close + * from failing across async boundaries. + * + * @param exchange original ServerWebExchange + * @param ctx enhanced plugin context (holds the baggage attributes to write) + * @return the original exchange when there is nothing to write, otherwise a mutated exchange + */ + private ServerWebExchange injectPendingBaggageHeader(ServerWebExchange exchange, EnhancedPluginContext ctx) { + String existing = exchange.getRequest().getHeaders().getFirst(OtelBaggageScopeHelper.BAGGAGE_HEADER); + String merged = OtelBaggageScopeHelper.resolvePendingBaggageHeader(ctx, existing); + if (merged == null) { + return exchange; + } + return exchange.mutate().request(builder -> builder.headers(headers -> + headers.set(OtelBaggageScopeHelper.BAGGAGE_HEADER, merged) + )).build(); + } + @Override public int getOrder() { return OrderConstant.Client.Scg.ENHANCED_FILTER_ORDER; diff --git a/spring-cloud-tencent-rpc-enhancement/src/main/java/com/tencent/cloud/rpc/enhancement/instrument/webclient/EnhancedWebClientExchangeFilterFunction.java b/spring-cloud-tencent-rpc-enhancement/src/main/java/com/tencent/cloud/rpc/enhancement/instrument/webclient/EnhancedWebClientExchangeFilterFunction.java index 1cfc6bf8e..39716beaf 100644 --- a/spring-cloud-tencent-rpc-enhancement/src/main/java/com/tencent/cloud/rpc/enhancement/instrument/webclient/EnhancedWebClientExchangeFilterFunction.java +++ b/spring-cloud-tencent-rpc-enhancement/src/main/java/com/tencent/cloud/rpc/enhancement/instrument/webclient/EnhancedWebClientExchangeFilterFunction.java @@ -30,6 +30,7 @@ import com.tencent.cloud.rpc.enhancement.plugin.EnhancedPluginType; import com.tencent.cloud.rpc.enhancement.plugin.EnhancedRequestContext; import com.tencent.cloud.rpc.enhancement.plugin.EnhancedResponseContext; import com.tencent.cloud.rpc.enhancement.util.EnhancedPluginUtils; +import com.tencent.cloud.rpc.enhancement.util.OtelBaggageScopeHelper; import com.tencent.polaris.api.utils.CollectionUtils; import com.tencent.polaris.circuitbreak.client.exception.CallAbortedException; import com.tencent.polaris.fault.client.exception.FaultInjectionException; @@ -134,7 +135,11 @@ public class EnhancedWebClientExchangeFilterFunction implements ExchangeFilterFu } // request may be changed by plugin ClientRequest request = (ClientRequest) enhancedPluginContext.getOriginRequest(); - return next.exchange(request) + // Inject the baggage attributes staged in the pre stage as a W3C baggage header on the + // downstream request. This avoids any dependency on the OTel Context ThreadLocal lifecycle + // and prevents Scope.close from failing across async boundaries. + ClientRequest downstreamRequest = injectPendingBaggageHeader(request, enhancedPluginContext); + return next.exchange(downstreamRequest) .doOnSuccess(response -> { enhancedPluginContext.setDelay(System.currentTimeMillis() - startTime); @@ -160,6 +165,26 @@ public class EnhancedWebClientExchangeFilterFunction implements ExchangeFilterFu }); } + /** + * Inject pending baggage attributes into the downstream ClientRequest as a W3C baggage HTTP + * header. This avoids any dependency on the OTel Context ThreadLocal lifecycle and prevents + * Scope.close from failing across async boundaries. + * + * @param request original ClientRequest + * @param ctx enhanced plugin context (holds the baggage attributes to write) + * @return the original request when there is nothing to write, otherwise a mutated ClientRequest + */ + private ClientRequest injectPendingBaggageHeader(ClientRequest request, EnhancedPluginContext ctx) { + String existing = request.headers().getFirst(OtelBaggageScopeHelper.BAGGAGE_HEADER); + String merged = OtelBaggageScopeHelper.resolvePendingBaggageHeader(ctx, existing); + if (merged == null) { + return request; + } + return ClientRequest.from(request) + .headers(headers -> headers.set(OtelBaggageScopeHelper.BAGGAGE_HEADER, merged)) + .build(); + } + private URI getServiceUri(ClientRequest clientRequest) { Object instance = MetadataContextHolder.get() .getLoadbalancerMetadata().get(LOAD_BALANCER_SERVICE_INSTANCE); diff --git a/spring-cloud-tencent-rpc-enhancement/src/main/java/com/tencent/cloud/rpc/enhancement/plugin/EnhancedContextKeys.java b/spring-cloud-tencent-rpc-enhancement/src/main/java/com/tencent/cloud/rpc/enhancement/plugin/EnhancedContextKeys.java new file mode 100644 index 000000000..e37f3a4ff --- /dev/null +++ b/spring-cloud-tencent-rpc-enhancement/src/main/java/com/tencent/cloud/rpc/enhancement/plugin/EnhancedContextKeys.java @@ -0,0 +1,41 @@ +/* + * Tencent is pleased to support the open source community by making spring-cloud-tencent available. + * + * Copyright (C) 2021 Tencent. All rights reserved. + * + * Licensed under the BSD 3-Clause License (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://opensource.org/licenses/BSD-3-Clause + * + * 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.tencent.cloud.rpc.enhancement.plugin; + +/** + * Common key constants stored in {@link EnhancedPluginContext#getExtraData()}. + * + *
These constants live in the rpc-enhancement module so they can be referenced by both the + * SCG / WebClient reactive client interceptors and the upper trace-plugin, without creating a + * module cycle.
+ * + * @author Haotian Zhang + */ +public final class EnhancedContextKeys { + + /** + * Pending baggage attributes (Map of string) staged by the trace plugin in the BEFORE_CALLING + * stage for reactive clients (SCG / WebClient). The SCG / WebClient interceptor removes them and + * injects them as a W3C baggage HTTP header on the outgoing request. No OTel Scope is attached, + * so there is no ThreadLocal lifecycle to close across async boundaries. + */ + public static final String PENDING_BAGGAGE_ATTRIBUTES_KEY = "PENDING_BAGGAGE_ATTRIBUTES_KEY"; + + private EnhancedContextKeys() { + } +} diff --git a/spring-cloud-tencent-rpc-enhancement/src/main/java/com/tencent/cloud/rpc/enhancement/util/OtelBaggageScopeHelper.java b/spring-cloud-tencent-rpc-enhancement/src/main/java/com/tencent/cloud/rpc/enhancement/util/OtelBaggageScopeHelper.java new file mode 100644 index 000000000..f1f7994d9 --- /dev/null +++ b/spring-cloud-tencent-rpc-enhancement/src/main/java/com/tencent/cloud/rpc/enhancement/util/OtelBaggageScopeHelper.java @@ -0,0 +1,208 @@ +/* + * Tencent is pleased to support the open source community by making spring-cloud-tencent available. + * + * Copyright (C) 2021 Tencent. All rights reserved. + * + * Licensed under the BSD 3-Clause License (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://opensource.org/licenses/BSD-3-Clause + * + * 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.tencent.cloud.rpc.enhancement.util; + +import java.io.ByteArrayOutputStream; +import java.nio.charset.StandardCharsets; +import java.util.BitSet; +import java.util.LinkedHashMap; +import java.util.Map; + +import com.tencent.cloud.rpc.enhancement.plugin.EnhancedContextKeys; +import com.tencent.cloud.rpc.enhancement.plugin.EnhancedPluginContext; + +/** + * Helper that builds a W3C baggage HTTP header value from a map of attributes. + * + *The previous approach reflectively called {@code Baggage.makeCurrent()} to attach baggage + * to the current thread's OTel Context and closed it after the downstream request was sent. In + * Reactor / Netty async scenarios, attach and close are not guaranteed to run on the same thread, + * and the OTel Agent also restores the ThreadLocal when its own Scope closes. Both cause the + * {@code current() != toAttach} check inside {@code ScopeImpl.close()} to fail, so the scope is + * never actually released.
+ * + *To avoid this entirely, this helper does not touch {@code ThreadLocalContextStorage} at all. + * Instead it produces a W3C baggage compliant header value. Callers write it into the downstream + * request headers before sending; the OTel Agent on the server side parses it back into + * {@code Baggage}, which is semantically identical to attach but has no ThreadLocal lifecycle.
+ * + * @author Haotian Zhang + */ +public final class OtelBaggageScopeHelper { + + /** + * Standard W3C baggage HTTP header name. + */ + public static final String BAGGAGE_HEADER = "baggage"; + + private static final char[] HEX = "0123456789ABCDEF".toCharArray(); + + /** + * The {@code baggage-octet} set from the W3C Baggage spec: US-ASCII characters excluding CTLs, + * whitespace, DQUOTE, comma, semicolon and backslash. Any code point outside this set must be + * percent-encoded. This intentionally differs from {@code x-www-form-urlencoded}: a space becomes + * {@code %20}, not {@code +}, because OTel's {@code BaggageCodec} only decodes {@code %XX} and + * keeps {@code +} as a literal plus. + */ + private static final BitSet BAGGAGE_OCTET = new BitSet(128); + + static { + for (int c = 0x21; c <= 0x7E; c++) { + BAGGAGE_OCTET.set(c); + } + BAGGAGE_OCTET.clear('"'); + BAGGAGE_OCTET.clear(','); + BAGGAGE_OCTET.clear(';'); + BAGGAGE_OCTET.clear('\\'); + } + + private OtelBaggageScopeHelper() { + } + + /** + * Encode the given attributes as a W3C baggage header value. + * + *Format is {@code key1=value1,key2=value2}; both key and value are percent-encoded per + * RFC 3986. Returns {@code null} when the input is empty.
+ * + * @param attributes attributes to encode + * @return W3C baggage header value or null when input is empty + */ + public static String encodeBaggage(MapExisting keys are overwritten by the new value; other keys are preserved. Returns the + * input unchanged when no attributes are provided.
+ * + * @param existing existing baggage header value (nullable) + * @param attributes attributes to merge in + * @return merged baggage header value, or null when both inputs empty + */ + public static String mergeIntoBaggageHeader(String existing, Map