Issue 138/header support #168

dtregonning · 2018-08-27T19:39:05Z

Resubmitted PR including Header work and re=categorization function defined in issue-165.

PR encompasses all Header related work to date.

Inclusion of config parameters:

"splunk.header.support";
"splunk.header.custom";
"splunk.header.index";
"splunk.header.source";
"splunk.header.sourcetype";
"splunk.header.host";
Inclusion of splunk.hec.json.event.formatted for events store in Kafka already configured in the HEC Format to ensure correct metadata extraction.

Modified test for headers to work update version

…list

Issue 165 modify record sort for raw

chenziliang · 2018-08-27T20:12:41Z

src/main/java/com/splunk/kafka/connect/SplunkSinkTask.java

+            if(records != null) { handleRecordsWithHeader(records); }
+        }
+
+        else if (connectorConfig.hasMetaDataConfigured()) {


Let's move else if to line 145

chenziliang · 2018-08-27T20:13:27Z

src/main/java/com/splunk/hecclient/RawEventBatch.java

+            .append(source, other.source)
+            .append(host, other.host)
+            .isEquals();
+        } else {


don't need else, a direct return false shall be fine

chenziliang · 2018-08-27T20:15:01Z

src/main/java/com/splunk/kafka/connect/SplunkSinkRecord.java

+        this.connectorConfig = connectorConfig;
+        this.headers = record.headers();
+        if(this.headers != null) {
+            log.info("not null headers");


can we get rid of this log as it will be too many and didn't provide much valuable information

haha - yeah :)

chenziliang · 2018-08-27T20:15:52Z

src/main/java/com/splunk/kafka/connect/SplunkSinkRecord.java

+        String source = headers.lastWithName(connectorConfig.headerSource).value().toString();
+        String sourcetype = headers.lastWithName(connectorConfig.headerSourcetype).value().toString();
+
+        if(splunkHeaderIndex.equals(index) && splunkHeaderHost.equals(host) &&


can we just

return splunkHeaderIndex.equals(index) && splunkHeaderHost.equals(host) && splunkHeaderSource.equals(source) && splunkHeaderSourcetype.equals(sourcetype);

chenziliang · 2018-08-27T20:17:20Z

src/main/java/com/splunk/kafka/connect/SplunkSinkRecord.java

+    }
+
+    private void setMetadataValues() {
+        log.info("Made it to setMetadataValues");


let's get rid of this log

chenziliang · 2018-08-27T20:29:18Z

src/main/java/com/splunk/kafka/connect/SplunkSinkTask.java

+
+        if (headers.lastWithName(connectorConfig.headerIndex) != null) {
+        //log.debug("splunk_index header detected, value is: " + headers.lastWithName(connectorConfig.headerIndex).value().toString());
+            event.setIndex(headers.lastWithName(connectorConfig.headerIndex).value().toString());


let's pay attention to the indent in this func ?

Can we cache headers.lastWithName(connectorConfig.headerIndex) to avoid looking up again ?

chenziliang · 2018-08-27T20:33:11Z

src/main/java/com/splunk/kafka/connect/SplunkSinkTask.java

+            event.setSourcetype(headers.lastWithName(connectorConfig.headerSourcetype).value().toString());
+        }
+
+        // Custom headers are configured with a comma separated list passed in configuration


Let's make sure, these are header keys, not headers and we don't support key containing ","

We will support a comma separated list of custom headers provided to the configuration to index as extra fields

chenziliang · 2018-08-27T20:33:56Z

src/main/java/com/splunk/kafka/connect/SplunkSinkTask.java

+            Map<String, String> headerMap = new HashMap<>();
+            for (String header : customHeaders) {
+                if (headers.lastWithName(header) != null) {
+                    log.debug(header + " header detected, value is: " + headers.lastWithName(header).value().toString());


can we get rid of this debug log ?

chenziliang · 2018-08-27T20:37:24Z

src/main/java/com/splunk/kafka/connect/SplunkSinkTask.java

+            String[] customHeaders = connectorConfig.headerCustom.split(",");
+            Map<String, String> headerMap = new HashMap<>();
+            for (String header : customHeaders) {
+                if (headers.lastWithName(header) != null) {


can we cache headers.lastWithName(header) and we don't need to do lookup everytime ?

chenziliang · 2018-08-27T20:39:37Z

src/main/java/com/splunk/hecclient/JsonEventBatch.java

+    @Override
+    public boolean equals(Object obj) {
+        if (obj instanceof JsonEventBatch) {
+            final JsonEventBatch other = (JsonEventBatch) obj;


what is the difference just doing this.endpoint.equals(other.endpoint)

chenziliang · 2018-08-27T20:51:46Z

src/main/java/com/splunk/hecclient/RawEventBatch.java

+            .append(sourcetype, other.sourcetype)
+            .append(source, other.source)
+            .append(host, other.host)
+            .isEquals();


Do the following 2 meta tuple equal ?

(idx1, sourcetype2, source3, host4) (idx, 1sourcetype2, source, 3host4)

No. Added Test to check condition

public void checkEquals() { RawEventBatch batchOne = RawEventBatch.factory() .setSource("source3") .setIndex("idx1") .setSourcetype("sourcetype2") .setHost("host4") .build(); RawEventBatch batchTwo = RawEventBatch.factory() .setSource("source") .setIndex("idx") .setSourcetype("1sourcetype2") .setHost("3host4") .build(); Assert.assertFalse(batchOne.equals(batchTwo)); } }```

chenziliang · 2018-08-29T18:56:04Z

src/main/java/com/splunk/kafka/connect/SplunkSinkConnectorConfig.java

-               .setTrustStorePath(trustStorePath)
-               .setTrustStorePassword(trustStorePassword)
-               .setHasCustomTrustStore(hasTrustStorePath);
+                .setSocketTimeout(socketTimeout)


Identing problem ?

chenziliang · 2018-08-29T18:57:36Z

src/main/java/com/splunk/kafka/connect/SplunkSinkRecord.java

+    }
+
+    private void setMetadataValues() {
+        if(this.headers.lastWithName(connectorConfig.headerIndex) != null) {


I would introduce a variable to hold this.headers.lastWithName(connectorConfig.headerIndex) to avoid re-computation

chenziliang · 2018-08-29T18:58:20Z

src/main/java/com/splunk/kafka/connect/SplunkSinkRecord.java

+    }
+    public String getSplunkHeaderSource() {
+        return splunkHeaderSource;
+    }


Let's separate each function with an empty line :)

chenziliang · 2018-08-29T18:59:46Z

src/main/java/com/splunk/kafka/connect/SplunkSinkTask.java

+
+        StringBuilder headerString = new StringBuilder();
+
+        if(headers.lastWithName(connectorConfig.headerIndex) != null) {


can we use a variable to hold the result of headers.lastWithName(connectorConfig.headerIndex) to avoid call this again ?

chenziliang · 2018-08-29T19:00:10Z

src/main/java/com/splunk/kafka/connect/SplunkSinkTask.java

+    }
+
+    public String insertHeaderToken() {
+        return "$$$";


we can make this a const string

chenziliang · 2018-08-29T19:01:01Z

src/main/java/com/splunk/kafka/connect/SplunkSinkTask.java

+                if (customHeader != null) {
+                    headerMap.put(header, customHeader.value().toString());
+                } else {
+                    log.debug(header + " header value not present in event.");


i would like to get rid of this in prod :)

Don Tregonning added 9 commits August 21, 2018 13:23

issue-138 - Add Header support to Connector

5e79d8a

Modified test for headers to work update version

Merge branch 'develop' into issue-138/header-support

8c709a3

Updates based on review

8953a55

issue-165 Modify raw batching functionality to use hashmap instad of …

4e7ba60

…list

WIP

4e1372a

wip

d40d844

issue-165 - fixed categorization of like Headers

7906cc0

Merge pull request #167 from splunk/issue-165-modify-record-sort-for-raw

7180b33

Issue 165 modify record sort for raw

merging diff

7777e45

dtregonning requested review from chaitanyaphalak and chenziliang August 27, 2018 19:39

remove unused test

f1bd1b5

chenziliang reviewed Aug 27, 2018

View reviewed changes

Header work after review

9378d9f

chenziliang reviewed Aug 29, 2018

View reviewed changes

More changes after Header work review

8252964

chenziliang previously approved these changes Aug 29, 2018

View reviewed changes

fix potential null value for compareRecordHeaders

fe2a90e

dtregonning dismissed chenziliang’s stale review via fe2a90e August 29, 2018 21:07

chenziliang approved these changes Aug 29, 2018

View reviewed changes

dtregonning merged commit d540004 into develop Aug 29, 2018

dtregonning deleted the issue-138/header-support branch August 29, 2018 22:14


		StringBuilder headerString = new StringBuilder();

		if(headers.lastWithName(connectorConfig.headerIndex) != null) {

Issue 138/header support #168

Issue 138/header support #168

Uh oh!

Conversation

dtregonning commented Aug 27, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dtregonning Aug 29, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

dtregonning Aug 29, 2018 •

edited

Loading