Skip to content

Fix issue 434: Add connector level proxy configurations#436

Open
wangshu3000 wants to merge 1 commit into
splunk:developfrom
fidelity-contributions:fix-issue-434
Open

Fix issue 434: Add connector level proxy configurations#436
wangshu3000 wants to merge 1 commit into
splunk:developfrom
fidelity-contributions:fix-issue-434

Conversation

@wangshu3000

Copy link
Copy Markdown

This is a PR to fix issue: #434

In this PR, i added 2 parameters to the connector configurations.
splunk.hec.http.proxy.host - The proxy host
splunk.hec.http.proxy.port - The proxy port

This proxy is setup at the connector level, the splunk connector can use it's own proxy.
If these 2 parameters were configured, it'll use this proxy when connecting to HEC endpoint.
Otherwise, it'll use the JVM level proxy setting in JVM_OPTS.

This provides us the capability to use a different proxy for this connector. No matter how the global JVM_OPTS proxy was configured.

@wangshu3000 wangshu3000 changed the title Fix issue 434 Fix issue 434: Add connector level proxy configurations Jun 24, 2024
@wangshu3000

Copy link
Copy Markdown
Author

Hi @wojtekzyla @VihasMakwana
Could you please hep to review this PR if you have time?
This is a PR to add proxy related configuration settings, so that we can configure connector level proxy.

Thanks.

@wojtekzyla

Copy link
Copy Markdown
Contributor

Hi @wangshu3000 sure I'll review this

Comment thread README.md Outdated
@wangshu3000

Copy link
Copy Markdown
Author

@wojtekzyla Not sure if you got chance to review my change, i have updated the description based on your suggestion. Could you please have a review? Thanks.

@shijiadong2022

Copy link
Copy Markdown

@wojtekzyla I wonder if you could help review this one when you have a chance?

@wojtekzyla wojtekzyla self-requested a review October 22, 2024 09:14
@wojtekzyla

Copy link
Copy Markdown
Contributor

Could you add some empty commit to restart the workflow?

Signed-off-by: Wang, Shu <shu.wang@fmr.com>
@wangshu3000

Copy link
Copy Markdown
Author

Thanks @wojtekzyla for your approval, i have retriggered the CI Build Test, could you please approve the flow? Thanks.

@github-actions

Copy link
Copy Markdown

Unit Test Results

175 tests   175 ✅  38s ⏱️
 24 suites    0 💤
 24 files      0 ❌

Results for commit b45a55d.

@wangshu3000

Copy link
Copy Markdown
Author

Thanks @wojtekzyla
Looks like the CI Build was failed at fossa-scan step, looks like some dependencies are not compliant, not sure if i should bump the version in my PR or it can be bypassed?

@wojtekzyla

Copy link
Copy Markdown
Contributor

Thanks @wojtekzyla Looks like the CI Build was failed at fossa-scan step, looks like some dependencies are not compliant, not sure if i should bump the version in my PR or it can be bypassed?

Yes, if you could do that it would be helpful. You could also add it to PR description

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR addresses issue #434 by adding connector-level HTTP proxy configuration for the Splunk HEC client, allowing a specific connector to use a dedicated outbound proxy instead of relying solely on JVM-wide proxy settings.

Changes:

  • Added new connector configs splunk.hec.http.proxy.host / splunk.hec.http.proxy.port and plumbed them through to HecConfig.
  • Updated the HEC HTTP client construction to apply a per-connector proxy via Apache HttpClient RequestConfig.
  • Added/updated unit tests and documentation, and refreshed the JaCoCo coverage report artifacts.

Reviewed changes

Copilot reviewed 11 out of 12 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
target/site/jacoco/jacoco.csv Updates committed coverage metrics after code/test changes.
src/test/java/com/splunk/kafka/connect/UnitUtil.java Adds proxy settings into generated test task configs.
src/test/java/com/splunk/kafka/connect/SplunkSinkConnectorConfigTest.java Adds a unit test asserting proxy config is propagated into HecConfig.
src/test/java/com/splunk/kafka/connect/ConfigProfile.java Extends test config profiles with proxy fields and string representation updates.
src/test/java/com/splunk/hecclient/HttpClientBuilderTest.java Adds a smoke test ensuring HttpClientBuilder can be built with proxy inputs.
src/test/java/com/splunk/hecclient/HecConfigTest.java Extends getter/setter coverage for new proxy fields on HecConfig.
src/main/java/com/splunk/kafka/connect/SplunkSinkConnectorConfig.java Introduces new connector config keys/docs and passes proxy settings into HecConfig.
src/main/java/com/splunk/hecclient/HttpClientBuilder.java Adds proxy fields/setters and applies proxy in RequestConfig when configured.
src/main/java/com/splunk/hecclient/HecConfig.java Adds new proxy host/port fields with getters/setters.
src/main/java/com/splunk/hecclient/Hec.java Plumbs HecConfig proxy settings into HTTP client creation paths.
README.md Documents the new connector properties and defaults.

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

.build();
RequestConfig.Builder requestConfigBuilder = RequestConfig.custom()
.setCookieSpec(CookieSpecs.STANDARD);
if (StringUtils.isNotEmpty(this.httpProxyHost) && this.httpProxyPort != 0) {
Comment on lines 288 to +292
sourcetypes = getString(SOURCETYPE_CONF);
sources = getString(SOURCE_CONF);
httpKeepAlive = getBoolean(HTTP_KEEPALIVE_CONF);
httpProxyHost = getString(HTTP_PROXY_HOST_CONF);
httpProxyPort = getInt(HTTP_PROXY_PORT_CONF);
Comment on lines 283 to 288
return new HttpClientBuilder().setDisableSSLCertVerification(config.getDisableSSLCertVerification())
.setMaxConnectionPoolSizePerDestination(poolSizePerDest)
.setMaxConnectionPoolSize(poolSizePerDest * config.getUris().size())
.setHttpProxyHost(config.getHttpProxyHost())
.setHttpProxyPort(config.getHttpProxyPort())
.build();
Comment thread README.md
Comment on lines +172 to +173
| `splunk.hec.http.proxy.host` | This setting is the http proxy server hostname. Configure it to use connector level proxy when connecting to HEC endpoint, otherwise, it'll use JVM level proxy setting in JVM_OPTS. | `""` |
| `splunk.hec.http.proxy.port` | This setting is the http proxy server port. Configure it to use connector level proxy when connecting to HEC endpoint, otherwise, it'll use JVM level proxy setting in JVM_OPTS. | `0` |

@Override public String toString() {
return "ConfigProfile{" + "topics='" + topics + '\'' + ", topics.regex='" + topicsRegex + '\'' + ", token='" + token + '\'' + ", uri='" + uri + '\'' + ", raw=" + raw + ", ack=" + ack + ", indexes='" + indexes + '\'' + ", sourcetypes='" + sourcetypes + '\'' + ", sources='" + sources + '\'' + ", httpKeepAlive=" + httpKeepAlive + ", validateCertificates=" + validateCertificates + ", hasTrustStorePath=" + hasTrustStorePath + ", trustStorePath='" + trustStorePath + '\'' + ", trustStoreType='" + trustStoreType + '\'' + ", trustStorePassword='" + trustStorePassword + '\'' + ", eventBatchTimeout=" + eventBatchTimeout + ", ackPollInterval=" + ackPollInterval + ", ackPollThreads=" + ackPollThreads + ", maxHttpConnPerChannel=" + maxHttpConnPerChannel + ", totalHecChannels=" + totalHecChannels + ", socketTimeout=" + socketTimeout + ", enrichements='" + enrichements + '\'' + ", enrichementMap=" + enrichementMap + ", trackData=" + trackData + ", maxBatchSize=" + maxBatchSize + ", numOfThreads=" + numOfThreads + '}';
return "ConfigProfile{" + "topics='" + topics + '\'' + ", topics.regex='" + topicsRegex + '\'' + ", token='" + token + '\'' + ", uri='" + uri + '\'' + ", raw=" + raw + ", ack=" + ack + ", indexes='" + indexes + '\'' + ", sourcetypes='" + sourcetypes + '\'' + ", sources='" + sources + '\'' + ", httpKeepAlive=" + httpKeepAlive + ", httpProxyHost=" + httpProxyHost + ", httpProxyPort=" + httpProxyPort + ", validateCertificates=" + validateCertificates + ", hasTrustStorePath=" + hasTrustStorePath + ", trustStorePath='" + trustStorePath + '\'' + ", " + "trustStoreType='" + trustStoreType + '\'' + ", trustStorePassword='" + trustStorePassword + '\'' + ", eventBatchTimeout=" + eventBatchTimeout + ", ackPollInterval=" + ackPollInterval + ", ackPollThreads=" + ackPollThreads + ", maxHttpConnPerChannel=" + maxHttpConnPerChannel + ", totalHecChannels=" + totalHecChannels + ", socketTimeout=" + socketTimeout + ", enrichements='" + enrichements + '\'' + ", enrichementMap=" + enrichementMap + ", trackData=" + trackData + ", maxBatchSize=" + maxBatchSize + ", numOfThreads=" + numOfThreads + '}';
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants