Skip to content

Commit b1351bc

Browse files
daguimudaguimu
authored
Fix httpclient-4.x plugin injecting sw8 headers into excluded ports (#800)
 Add PROPAGATION_EXCLUDE_PORTS config to httpclient-4.x plugin, mirroring the existing httpclient-5.x feature. When configured, requests to listed ports skip span creation and header injection entirely, preventing HTTP 400 from servers like ClickHouse that reject unknown headers. Default is empty (opt-in). The ClickHouse E2E test sets it to 8123 explicitly.
1 parent ac0df43 commit b1351bc

File tree

7 files changed

+293
-13
lines changed

7 files changed

+293
-13
lines changed

‎apm-sniffer/apm-sdk-plugin/httpClient-4.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/httpClient/v4/HttpClientExecuteInterceptor.java‎

Lines changed: 68 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,10 @@
2222
import java.net.MalformedURLException;
2323
import java.net.URI;
2424
import java.net.URL;
25+
import java.util.Arrays;
26+
import java.util.Collections;
27+
import java.util.Set;
28+
import java.util.stream.Collectors;
2529

2630
import org.apache.http.HttpHost;
2731
import org.apache.http.HttpRequest;
@@ -34,6 +38,8 @@
3438
import org.apache.skywalking.apm.agent.core.context.tag.Tags;
3539
import org.apache.skywalking.apm.agent.core.context.trace.AbstractSpan;
3640
import org.apache.skywalking.apm.agent.core.context.trace.SpanLayer;
41+
import org.apache.skywalking.apm.agent.core.logging.api.ILog;
42+
import org.apache.skywalking.apm.agent.core.logging.api.LogManager;
3743
import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.EnhancedInstance;
3844
import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.InstanceMethodsAroundInterceptor;
3945
import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.MethodInterceptResult;
@@ -43,12 +49,19 @@
4349

4450
public class HttpClientExecuteInterceptor implements InstanceMethodsAroundInterceptor {
4551
private static final String ERROR_URI = "/_blank";
52+
private static final ILog LOGGER = LogManager.getLogger(HttpClientExecuteInterceptor.class);
53+
54+
/**
55+
* Lazily-resolved set of ports that must not receive SkyWalking
56+
* propagation headers. Built once from
57+
* {@link HttpClientPluginConfig.Plugin.HttpClient#PROPAGATION_EXCLUDE_PORTS}.
58+
*/
59+
private volatile Set<Integer> excludePortsCache;
4660

4761
@Override
4862
public void beforeMethod(EnhancedInstance objInst, Method method, Object[] allArguments, Class<?>[] argumentsTypes,
4963
MethodInterceptResult result) throws Throwable {
50-
if (allArguments[0] == null || allArguments[1] == null) {
51-
// illegal args, can't trace. ignore.
64+
if (skipIntercept(allArguments)) {
5265
return;
5366
}
5467
final HttpHost httpHost = (HttpHost) allArguments[0];
@@ -82,7 +95,7 @@ public void beforeMethod(EnhancedInstance objInst, Method method, Object[] allAr
8295
@Override
8396
public Object afterMethod(EnhancedInstance objInst, Method method, Object[] allArguments, Class<?>[] argumentsTypes,
8497
Object ret) throws Throwable {
85-
if (allArguments[0] == null || allArguments[1] == null) {
98+
if (skipIntercept(allArguments)) {
8699
return ret;
87100
}
88101

@@ -111,10 +124,62 @@ public Object afterMethod(EnhancedInstance objInst, Method method, Object[] allA
111124
@Override
112125
public void handleMethodException(EnhancedInstance objInst, Method method, Object[] allArguments,
113126
Class<?>[] argumentsTypes, Throwable t) {
127+
if (skipIntercept(allArguments)) {
128+
return;
129+
}
114130
AbstractSpan activeSpan = ContextManager.activeSpan();
115131
activeSpan.log(t);
116132
}
117133

134+
private boolean skipIntercept(Object[] allArguments) {
135+
if (allArguments[0] == null || allArguments[1] == null) {
136+
return true;
137+
}
138+
HttpHost httpHost = (HttpHost) allArguments[0];
139+
return isExcludedPort(port(httpHost));
140+
}
141+
142+
/**
143+
* Returns {@code true} when {@code port} is listed in
144+
* {@link HttpClientPluginConfig.Plugin.HttpClient#PROPAGATION_EXCLUDE_PORTS}.
145+
*
146+
* <p>The config value is parsed lazily and cached so that it is read after
147+
* the agent has fully initialised its configuration subsystem.
148+
*/
149+
private boolean isExcludedPort(int port) {
150+
if (port <= 0) {
151+
return false;
152+
}
153+
if (excludePortsCache == null) {
154+
synchronized (this) {
155+
if (excludePortsCache == null) {
156+
excludePortsCache = parseExcludePorts(
157+
HttpClientPluginConfig.Plugin.HttpClient.PROPAGATION_EXCLUDE_PORTS);
158+
}
159+
}
160+
}
161+
return excludePortsCache.contains(port);
162+
}
163+
164+
private static Set<Integer> parseExcludePorts(String raw) {
165+
if (raw == null || raw.trim().isEmpty()) {
166+
return Collections.emptySet();
167+
}
168+
return Arrays.stream(raw.split(","))
169+
.map(String::trim)
170+
.filter(s -> !s.isEmpty())
171+
.map(s -> {
172+
try {
173+
return Integer.parseInt(s);
174+
} catch (NumberFormatException e) {
175+
LOGGER.warn("Ignoring invalid port in PROPAGATION_EXCLUDE_PORTS: {}", s);
176+
return -1;
177+
}
178+
})
179+
.filter(p -> p > 0)
180+
.collect(Collectors.toSet());
181+
}
182+
118183
private String getRequestURI(String uri) {
119184
if (isUrl(uri)) {
120185
String requestPath;

‎apm-sniffer/apm-sdk-plugin/httpClient-4.x-plugin/src/test/java/org/apache/skywalking/apm/plugin/httpClient/v4/HttpClientPropagationExcludePortTest.java‎

Lines changed: 203 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,203 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one or more
3+
* contributor license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright ownership.
5+
* The ASF licenses this file to You under the Apache License, Version 2.0
6+
* (the "License"); you may not use this file except in compliance with
7+
* the License. You may obtain a copy of the License at
8+
*
9+
* http://www.apache.org/licenses/LICENSE-2.0
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
*
17+
*/
18+
19+
package org.apache.skywalking.apm.plugin.httpClient.v4;
20+
21+
import static org.hamcrest.CoreMatchers.is;
22+
import static org.hamcrest.MatcherAssert.assertThat;
23+
import static org.mockito.ArgumentMatchers.anyString;
24+
import static org.mockito.Mockito.never;
25+
import static org.mockito.Mockito.times;
26+
import static org.mockito.Mockito.verify;
27+
import static org.mockito.Mockito.when;
28+
import static org.mockito.Mockito.mock;
29+
30+
import java.util.List;
31+
32+
import org.apache.http.HttpHost;
33+
import org.apache.http.HttpResponse;
34+
import org.apache.http.ProtocolVersion;
35+
import org.apache.http.RequestLine;
36+
import org.apache.http.StatusLine;
37+
import org.apache.http.client.methods.HttpGet;
38+
import org.apache.skywalking.apm.agent.core.boot.ServiceManager;
39+
import org.apache.skywalking.apm.agent.core.context.trace.TraceSegment;
40+
import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.EnhancedInstance;
41+
import org.apache.skywalking.apm.agent.test.tools.AgentServiceRule;
42+
import org.apache.skywalking.apm.agent.test.tools.SegmentStorage;
43+
import org.apache.skywalking.apm.agent.test.tools.SegmentStoragePoint;
44+
import org.apache.skywalking.apm.agent.test.tools.TracingSegmentRunner;
45+
import org.apache.skywalking.apm.plugin.httpclient.HttpClientPluginConfig;
46+
import org.junit.Before;
47+
import org.junit.Rule;
48+
import org.junit.Test;
49+
import org.junit.runner.RunWith;
50+
import org.mockito.Mock;
51+
import org.mockito.junit.MockitoJUnit;
52+
import org.mockito.junit.MockitoRule;
53+
54+
/**
55+
* Verifies that requests to ports listed in
56+
* {@link HttpClientPluginConfig.Plugin.HttpClient#PROPAGATION_EXCLUDE_PORTS}
57+
* are silently skipped (no span created, no sw8 header injected).
58+
*/
59+
@RunWith(TracingSegmentRunner.class)
60+
public class HttpClientPropagationExcludePortTest {
61+
62+
@SegmentStoragePoint
63+
private SegmentStorage segmentStorage;
64+
65+
@Rule
66+
public AgentServiceRule agentServiceRule = new AgentServiceRule();
67+
@Rule
68+
public MockitoRule rule = MockitoJUnit.rule();
69+
70+
@Mock
71+
private HttpHost clickHouseHost;
72+
@Mock
73+
private HttpHost regularHost;
74+
@Mock
75+
private HttpGet request;
76+
@Mock
77+
private HttpResponse httpResponse;
78+
@Mock
79+
private StatusLine statusLine;
80+
@Mock
81+
private EnhancedInstance enhancedInstance;
82+
83+
private HttpClientExecuteInterceptor interceptor;
84+
85+
private Object[] clickHouseArgs;
86+
private Object[] regularArgs;
87+
private Class<?>[] argumentsType;
88+
89+
@Before
90+
public void setUp() throws Exception {
91+
ServiceManager.INSTANCE.boot();
92+
93+
HttpClientPluginConfig.Plugin.HttpClient.PROPAGATION_EXCLUDE_PORTS = "8123";
94+
interceptor = new HttpClientExecuteInterceptor();
95+
96+
when(statusLine.getStatusCode()).thenReturn(200);
97+
when(httpResponse.getStatusLine()).thenReturn(statusLine);
98+
99+
RequestLine requestLine = new RequestLine() {
100+
@Override
101+
public String getMethod() {
102+
return "GET";
103+
}
104+
105+
@Override
106+
public ProtocolVersion getProtocolVersion() {
107+
return new ProtocolVersion("http", 1, 1);
108+
}
109+
110+
@Override
111+
public String getUri() {
112+
return "http://my-service:8080/api/ping";
113+
}
114+
};
115+
when(request.getRequestLine()).thenReturn(requestLine);
116+
117+
when(clickHouseHost.getHostName()).thenReturn("clickhouse-server");
118+
when(clickHouseHost.getSchemeName()).thenReturn("http");
119+
when(clickHouseHost.getPort()).thenReturn(8123);
120+
121+
when(regularHost.getHostName()).thenReturn("my-service");
122+
when(regularHost.getSchemeName()).thenReturn("http");
123+
when(regularHost.getPort()).thenReturn(8080);
124+
125+
clickHouseArgs = new Object[]{clickHouseHost, request};
126+
regularArgs = new Object[]{regularHost, request};
127+
argumentsType = new Class[]{HttpHost.class, HttpGet.class};
128+
}
129+
130+
@Test
131+
public void requestToExcludedPort_noSpanAndNoHeaderInjected() throws Throwable {
132+
interceptor.beforeMethod(enhancedInstance, null, clickHouseArgs, argumentsType, null);
133+
interceptor.afterMethod(enhancedInstance, null, clickHouseArgs, argumentsType, httpResponse);
134+
135+
List<TraceSegment> segments = segmentStorage.getTraceSegments();
136+
assertThat(segments.size(), is(0));
137+
verify(request, never()).setHeader(anyString(), anyString());
138+
}
139+
140+
@Test
141+
public void requestToRegularPort_spanCreatedAndHeadersInjected() throws Throwable {
142+
interceptor.beforeMethod(enhancedInstance, null, regularArgs, argumentsType, null);
143+
interceptor.afterMethod(enhancedInstance, null, regularArgs, argumentsType, httpResponse);
144+
145+
List<TraceSegment> segments = segmentStorage.getTraceSegments();
146+
assertThat(segments.size(), is(1));
147+
verify(request, times(3)).setHeader(anyString(), anyString());
148+
}
149+
150+
@Test
151+
public void whenExcludePortsEmpty_allPortsAreTraced() throws Throwable {
152+
HttpClientPluginConfig.Plugin.HttpClient.PROPAGATION_EXCLUDE_PORTS = "";
153+
154+
HttpClientExecuteInterceptor freshInterceptor = new HttpClientExecuteInterceptor();
155+
freshInterceptor.beforeMethod(enhancedInstance, null, clickHouseArgs, argumentsType, null);
156+
freshInterceptor.afterMethod(enhancedInstance, null, clickHouseArgs, argumentsType, httpResponse);
157+
158+
List<TraceSegment> segments = segmentStorage.getTraceSegments();
159+
assertThat(segments.size(), is(1));
160+
}
161+
162+
@Test
163+
public void multipleExcludedPorts_allSkippedAndNonExcludedStillTraced() throws Throwable {
164+
HttpClientPluginConfig.Plugin.HttpClient.PROPAGATION_EXCLUDE_PORTS = "8123,9200";
165+
166+
HttpClientExecuteInterceptor freshInterceptor = new HttpClientExecuteInterceptor();
167+
168+
// 8123 should be excluded
169+
freshInterceptor.beforeMethod(enhancedInstance, null, clickHouseArgs, argumentsType, null);
170+
freshInterceptor.afterMethod(enhancedInstance, null, clickHouseArgs, argumentsType, httpResponse);
171+
assertThat(segmentStorage.getTraceSegments().size(), is(0));
172+
173+
// 9200 should also be excluded
174+
HttpHost esHost = mock(HttpHost.class);
175+
when(esHost.getHostName()).thenReturn("es-server");
176+
when(esHost.getSchemeName()).thenReturn("http");
177+
when(esHost.getPort()).thenReturn(9200);
178+
Object[] esArgs = new Object[]{esHost, request};
179+
180+
freshInterceptor = new HttpClientExecuteInterceptor();
181+
HttpClientPluginConfig.Plugin.HttpClient.PROPAGATION_EXCLUDE_PORTS = "8123,9200";
182+
freshInterceptor.beforeMethod(enhancedInstance, null, esArgs, argumentsType, null);
183+
freshInterceptor.afterMethod(enhancedInstance, null, esArgs, argumentsType, httpResponse);
184+
assertThat(segmentStorage.getTraceSegments().size(), is(0));
185+
186+
// 8080 should still be traced
187+
freshInterceptor = new HttpClientExecuteInterceptor();
188+
HttpClientPluginConfig.Plugin.HttpClient.PROPAGATION_EXCLUDE_PORTS = "8123,9200";
189+
freshInterceptor.beforeMethod(enhancedInstance, null, regularArgs, argumentsType, null);
190+
freshInterceptor.afterMethod(enhancedInstance, null, regularArgs, argumentsType, httpResponse);
191+
assertThat(segmentStorage.getTraceSegments().size(), is(1));
192+
}
193+
194+
@Test
195+
public void excludedPort_handleMethodExceptionSkipped() throws Throwable {
196+
interceptor.beforeMethod(enhancedInstance, null, clickHouseArgs, argumentsType, null);
197+
interceptor.handleMethodException(enhancedInstance, null, clickHouseArgs, argumentsType, new RuntimeException("test"));
198+
interceptor.afterMethod(enhancedInstance, null, clickHouseArgs, argumentsType, httpResponse);
199+
200+
List<TraceSegment> segments = segmentStorage.getTraceSegments();
201+
assertThat(segments.size(), is(0));
202+
}
203+
}

‎apm-sniffer/apm-sdk-plugin/httpclient-commons/src/main/java/org/apache/skywalking/apm/plugin/httpclient/HttpClientPluginConfig.java‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,22 @@ public static class HttpClient {
2828
* This config item controls that whether the HttpClient plugin should collect the parameters of the request.
2929
*/
3030
public static boolean COLLECT_HTTP_PARAMS = false;
31+
32+
/**
33+
* Comma-separated list of destination ports whose outbound HTTP requests
34+
* will be completely skipped by the httpclient-4.x interceptor: no exit
35+
* span is created and no SkyWalking propagation headers are injected.
36+
*
37+
* <p>Some HTTP-based database protocols (e.g. ClickHouse on port 8123)
38+
* reject requests that contain unknown HTTP headers, returning HTTP 400.
39+
* Adding such ports here prevents the agent from creating exit spans
40+
* and from injecting the {@code sw8} tracing headers into those outbound
41+
* requests.
42+
*
43+
* <p>Example – exclude ClickHouse and Elasticsearch ports:
44+
* {@code plugin.httpclient.propagation_exclude_ports=8123,9200}
45+
*/
46+
public static String PROPAGATION_EXCLUDE_PORTS = "";
3147
}
3248

3349
@PluginConfig(root = HttpClientPluginConfig.class)

‎apm-sniffer/config/agent.config‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -218,8 +218,10 @@ plugin.jdkthreading.threading_class_prefixes=${SW_PLUGIN_JDKTHREADING_THREADING_
218218
plugin.tomcat.collect_http_params=${SW_PLUGIN_TOMCAT_COLLECT_HTTP_PARAMS:false}
219219
# This config item controls that whether the SpringMVC plugin should collect the parameters of the request, when your Spring application is based on Tomcat, consider only setting either `plugin.tomcat.collect_http_params` or `plugin.springmvc.collect_http_params`. Also, activate implicitly in the profiled trace.
220220
plugin.springmvc.collect_http_params=${SW_PLUGIN_SPRINGMVC_COLLECT_HTTP_PARAMS:false}
221-
# This config item controls that whether the HttpClient plugin should collect the parameters of the request
221+
# This config item controls that whether the HttpClient plugin should collect the parameters of the request
222222
plugin.httpclient.collect_http_params=${SW_PLUGIN_HTTPCLIENT_COLLECT_HTTP_PARAMS:false}
223+
# Comma-separated list of destination ports whose outbound HTTP requests will be completely skipped by the httpclient-4.x interceptor (no exit span created, no sw8 headers injected).
224+
plugin.httpclient.propagation_exclude_ports=${SW_PLUGIN_HTTPCLIENT_PROPAGATION_EXCLUDE_PORTS:}
223225
# When `COLLECT_HTTP_PARAMS` is enabled, how many characters to keep and send to the OAP backend, use negative values to keep and send the complete parameters, NB. this config item is added for the sake of performance.
224226
plugin.http.http_params_length_threshold=${SW_PLUGIN_HTTP_HTTP_PARAMS_LENGTH_THRESHOLD:1024}
225227
# When `include_http_headers` declares header names, this threshold controls the length limitation of all header values. use negative values to keep and send the complete headers. Note. this config item is added for the sake of performance.

‎changes/changes-9.5.0.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,8 @@ Release Notes.
2121
enhanced by both SkyWalking and Spring AOP.
2222
* Build: Centralized plugin version management in the root POM and remove redundant declarations.
2323
* Support Spring Cloud Gateway 4.3.x.
24+
* Add `PROPAGATION_EXCLUDE_PORTS` config to httpclient-4.x plugin to skip tracing and header injection for
25+
specified ports (default: 8123), fixing ClickHouse HTTP 400 caused by injected sw8 headers.
2426

2527
All issues and pull requests are [here](https://github.com/apache/skywalking/milestone/236?closed=1)
2628

0 commit comments

Comments
 (0)