fix: add PROPAGATION_EXCLUDE_PORTS to httpclient-4.x plugin by daguimu · Pull Request #800 · apache/skywalking-java

daguimu · 2026-03-26T01:59:13Z

Problem

When the SkyWalking Java agent enhances an application that uses ClickHouse JDBC via HTTP (port 8123), the httpclient-4.x plugin unconditionally injects sw8, sw8-correlation, and sw8-x propagation headers into every outbound HTTP request. ClickHouse rejects requests containing unknown headers, returning HTTP 400 Bad Request, which breaks all JDBC queries.

Root Cause

HttpClientExecuteInterceptor.beforeMethod() iterates over all ContextCarrier items and calls httpRequest.setHeader() without checking whether the target server can tolerate extra headers. The httpclient-5.x plugin already solved this with a PROPAGATION_EXCLUDE_PORTS config, but httpclient-4.x was missing this feature.

Fix

Add PROPAGATION_EXCLUDE_PORTS config to HttpClientPluginConfig.Plugin.HttpClient (default: "8123")
Add skipIntercept() logic to HttpClientExecuteInterceptor that checks the destination port against the exclusion list before creating spans or injecting headers
When the port is excluded, beforeMethod, afterMethod, and handleMethodException all return early, leaving the HTTP request completely untouched
Port parsing is lazy and cached (double-checked locking) for zero overhead on the hot path
Add the config entry to agent.config

Tests Added

requestToExcludedPort_noSpanAndNoHeaderInjected - verifies port 8123 produces no trace segment and no header injection
requestToRegularPort_spanCreatedAndHeadersInjected - verifies port 8080 is still traced normally
whenExcludePortsEmpty_allPortsAreTraced - verifies empty config traces everything including 8123
multipleExcludedPorts_allSkippedAndNonExcludedStillTraced - verifies multi-port config (8123,9200) works correctly
excludedPort_handleMethodExceptionSkipped - verifies exception handling is also skipped for excluded ports

All existing httpclient-4.x tests continue to pass.

Impact

Only affects httpclient-4.x plugin users whose target servers reject unknown HTTP headers (e.g. ClickHouse)
Default config (8123) matches the httpclient-5.x plugin behavior
No behavioral change for non-excluded ports
Configurable via plugin.httpclient.propagation_exclude_ports or env var SW_PLUGIN_HTTPCLIENT_PROPAGATION_EXCLUDE_PORTS

Fixes apache/skywalking#13700

Some HTTP-based protocols (e.g. ClickHouse on port 8123) reject requests containing unknown HTTP headers such as sw8, returning HTTP 400. The httpclient-5.x plugin already has this feature but httpclient-4.x was missing it. Add a PROPAGATION_EXCLUDE_PORTS config (default: "8123") to the httpclient-4.x plugin so that requests to excluded ports skip span creation and header injection entirely. Fixes apache/skywalking#13700

wu-sheng · 2026-03-26T02:29:16Z

#797 should have fixed this? Is CK using 4.x and 5.x?

Since httpclient-4.x now skips port 8123, the nested HTTP exit span is no longer created, so the http.status_code tag no longer appears on ClickHouse JDBC exit spans. Update expected test data accordingly.

daguimu · 2026-03-26T03:50:06Z

Thanks for the review! PR #797 added PROPAGATION_EXCLUDE_PORTS to the httpclient-5.x plugin. However, ClickHouse JDBC v0.3.1 (used in the clickhouse-0.3.1-scenario E2E test) uses Apache HttpClient 4.5.13 (org.apache.http.impl.client.CloseableHttpClient) as its HTTP transport, which is intercepted by the httpclient-4.x plugin — a separate plugin that doesn't have this feature yet.

This PR adds the same PROPAGATION_EXCLUDE_PORTS support to the httpclient-4.x plugin to cover this case.

wu-sheng · 2026-03-26T03:52:39Z

e2e doesn't matter at all. They passed anyway. the point is, is 0.3.1 ck client still used?

daguimu · 2026-03-26T03:57:28Z

The issue reporter in apache/skywalking#13700 confirmed the problem is caused by the httpclient-4.x plugin — their workaround was SW_EXCLUDE_PLUGINS=httpclient-4.x.

This fix is not specific to ClickHouse JDBC v0.3.1 — any application using Apache HttpClient 4.x to communicate with a server that rejects unknown headers (e.g., ClickHouse on port 8123) would be affected. Apache HttpClient 4.x (org.apache.httpcomponents:httpclient) remains widely used in the Java ecosystem.

Copilot

Pull request overview

Adds a PROPAGATION_EXCLUDE_PORTS configuration to the httpclient-4.x plugin so the interceptor can skip span creation and SkyWalking header injection for destination ports (defaulting to ClickHouse’s 8123), preventing HTTP 400s from servers that reject unknown headers.

Changes:

Introduce plugin.httpclient.propagation_exclude_ports config (default 8123) and document it in agent.config + release notes.
Implement lazy, cached excluded-port parsing in HttpClientExecuteInterceptor and short-circuit before/after/exception paths when excluded.
Add unit tests covering excluded vs regular ports and update ClickHouse scenario expected data.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
`test/plugin/scenarios/clickhouse-0.3.1-scenario/config/expectedData.yaml`	Updates scenario expectations to match new behavior (no HTTP status tag emitted via httpclient-4.x path).
`changes/changes-9.5.0.md`	Adds release note entry for the new exclusion-port config.
`apm-sniffer/config/agent.config`	Documents and wires the new `plugin.httpclient.propagation_exclude_ports` setting + env var.
`apm-sniffer/apm-sdk-plugin/httpclient-commons/src/main/java/org/apache/skywalking/apm/plugin/httpclient/HttpClientPluginConfig.java`	Adds `PROPAGATION_EXCLUDE_PORTS` plugin config field and Javadoc.
`apm-sniffer/apm-sdk-plugin/httpClient-4.x-plugin/src/test/java/org/apache/skywalking/apm/plugin/httpClient/v4/HttpClientPropagationExcludePortTest.java`	New test suite validating skip behavior and edge cases.
`apm-sniffer/apm-sdk-plugin/httpClient-4.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/httpClient/v4/HttpClientExecuteInterceptor.java`	Implements skip-intercept logic with lazy parsing + caching of excluded ports.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-26T04:03:34Z

...ommons/src/main/java/org/apache/skywalking/apm/plugin/httpclient/HttpClientPluginConfig.java

+             * <p>Default: {@code "8123"} (ClickHouse HTTP interface).
+             *
+             * <p>Example – also exclude port 9200 (Elasticsearch):
+             * {@code plugin.httpclient.PROPAGATION_EXCLUDE_PORTS=8123,9200}


The Javadoc example uses plugin.httpclient.PROPAGATION_EXCLUDE_PORTS, but config keys are lowercased by ConfigInitializer (e.g. COLLECT_HTTP_PARAMS maps to plugin.httpclient.collect_http_params). This example should be updated to plugin.httpclient.propagation_exclude_ports=8123,9200 (and/or mention the env var) so users don’t set a key that will be ignored.

Suggested change

* {@code plugin.httpclient.PROPAGATION_EXCLUDE_PORTS=8123,9200}

* {@code plugin.httpclient.propagation_exclude_ports=8123,9200}

Fixes Copilot review comment: ConfigInitializer maps fields to lowercase keys, so the example should read plugin.httpclient.propagation_exclude_ports instead of uppercase.

wu-sheng · 2026-03-26T07:14:07Z

apm-sniffer/config/agent.config

+#  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). Default: 8123 (ClickHouse HTTP interface).
+plugin.httpclient.propagation_exclude_ports=${SW_PLUGIN_HTTPCLIENT_PROPAGATION_EXCLUDE_PORTS:8123}


This should not be added as a default. This config should only exist in ck test cases.

Done. Removed the default from agent.config and moved SW_PLUGIN_HTTPCLIENT_PROPAGATION_EXCLUDE_PORTS=8123 to the clickhouse-0.3.1 E2E test scenario only.

Move the PROPAGATION_EXCLUDE_PORTS=8123 config from the global agent.config default to the clickhouse-0.3.1 E2E test scenario only, as requested in code review.

wu-sheng · 2026-03-26T08:27:15Z

...ommons/src/main/java/org/apache/skywalking/apm/plugin/httpclient/HttpClientPluginConfig.java

+             * <p>Example – also exclude port 9200 (Elasticsearch):
+             * {@code plugin.httpclient.propagation_exclude_ports=8123,9200}
+             */
+            public static String PROPAGATION_EXCLUDE_PORTS = "8123";


This default value should be removed. No default to exclude. The config item plugin.httpclient.propagation_exclude_ports should be a part of config file, but also have an empty value.

wu-sheng · 2026-03-26T08:32:53Z

test/plugin/scenarios/clickhouse-0.3.1-scenario/configuration.yml

 startScript: ./bin/startup.sh
 environment:
  - SW_JDBC_TRACE_SQL_PARAMETERS=true
+  - SW_PLUGIN_HTTPCLIENT_PROPAGATION_EXCLUDE_PORTS=8123


This never works, if you don't put the config in the agent.config. The plugin test passed just because you hard coded default value.

…ion_exclude_ports - Change PROPAGATION_EXCLUDE_PORTS default from "8123" to "" (no ports excluded by default) - Add config entry back to agent.config with empty default so env var SW_PLUGIN_HTTPCLIENT_PROPAGATION_EXCLUDE_PORTS works correctly - E2E clickhouse test sets the env var to "8123" explicitly

daguimu · 2026-03-26T08:41:31Z

Fixed both issues in the latest commit (6d822b2):

Removed default value: PROPAGATION_EXCLUDE_PORTS default changed from "8123" to "" — no ports excluded by default.
Restored agent.config entry with empty default so the env var SW_PLUGIN_HTTPCLIENT_PROPAGATION_EXCLUDE_PORTS works correctly via ConfigInitializer:
```
plugin.httpclient.propagation_exclude_ports=${SW_PLUGIN_HTTPCLIENT_PROPAGATION_EXCLUDE_PORTS:}
```
The clickhouse-0.3.1 E2E test sets SW_PLUGIN_HTTPCLIENT_PROPAGATION_EXCLUDE_PORTS=8123 explicitly, which now flows through agent.config → ConfigInitializer → HttpClientPluginConfig as expected.

fix: update clickhouse E2E expected data for excluded port

bd53bbd

Since httpclient-4.x now skips port 8123, the nested HTTP exit span is no longer created, so the http.status_code tag no longer appears on ClickHouse JDBC exit spans. Update expected test data accordingly.

wu-sheng added this to the 9.7.0 milestone Mar 26, 2026

wu-sheng added enhancement New feature or request plugin labels Mar 26, 2026

wu-sheng requested a review from Copilot March 26, 2026 03:58

Copilot started reviewing on behalf of wu-sheng March 26, 2026 03:59 View session

Copilot AI reviewed Mar 26, 2026

View reviewed changes

fix: use lowercase config key in Javadoc example

aad37d2

Fixes Copilot review comment: ConfigInitializer maps fields to lowercase keys, so the example should read plugin.httpclient.propagation_exclude_ports instead of uppercase.

wu-sheng reviewed Mar 26, 2026

View reviewed changes

fix: remove default propagation_exclude_ports from agent.config

8c53417

Move the PROPAGATION_EXCLUDE_PORTS=8123 config from the global agent.config default to the clickhouse-0.3.1 E2E test scenario only, as requested in code review.

wu-sheng reviewed Mar 26, 2026

View reviewed changes

wu-sheng approved these changes Mar 26, 2026

View reviewed changes

wu-sheng merged commit b1351bc into apache:main Mar 26, 2026
206 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: add PROPAGATION_EXCLUDE_PORTS to httpclient-4.x plugin#800

fix: add PROPAGATION_EXCLUDE_PORTS to httpclient-4.x plugin#800
wu-sheng merged 5 commits intoapache:mainfrom
daguimu:fix/httpclient4-propagation-exclude-ports-issue13700

daguimu commented Mar 26, 2026

Uh oh!

wu-sheng commented Mar 26, 2026

Uh oh!

daguimu commented Mar 26, 2026

Uh oh!

wu-sheng commented Mar 26, 2026

Uh oh!

daguimu commented Mar 26, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 26, 2026

Uh oh!

wu-sheng Mar 26, 2026

Uh oh!

daguimu Mar 26, 2026

Uh oh!

wu-sheng Mar 26, 2026

Uh oh!

wu-sheng Mar 26, 2026

Uh oh!

daguimu commented Mar 26, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	* {@code plugin.httpclient.PROPAGATION_EXCLUDE_PORTS=8123,9200}
	* {@code plugin.httpclient.propagation_exclude_ports=8123,9200}

		# 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). Default: 8123 (ClickHouse HTTP interface).
		plugin.httpclient.propagation_exclude_ports=${SW_PLUGIN_HTTPCLIENT_PROPAGATION_EXCLUDE_PORTS:8123}

Conversation

daguimu commented Mar 26, 2026

Problem

Root Cause

Fix

Tests Added

Impact

Uh oh!

wu-sheng commented Mar 26, 2026

Uh oh!

daguimu commented Mar 26, 2026

Uh oh!

wu-sheng commented Mar 26, 2026

Uh oh!

daguimu commented Mar 26, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

Uh oh!

wu-sheng Mar 26, 2026

Choose a reason for hiding this comment

Uh oh!

daguimu Mar 26, 2026

Choose a reason for hiding this comment

Uh oh!

wu-sheng Mar 26, 2026

Choose a reason for hiding this comment

Uh oh!

wu-sheng Mar 26, 2026

Choose a reason for hiding this comment

Uh oh!

daguimu commented Mar 26, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants