added a timestamp.timezone configuration in SplunkSinkConnectorConfig by mo-mosh · Pull Request #403 · splunk/kafka-connect-splunk

mo-mosh · 2023-08-28T02:57:57Z

I have the same issue "Add support for timezone override #398", so I added a timestamp.timezone.
as a default it's the UTC, but in my case I needed to change UTC to Asia/Seoul.

… for the connector config

VihasMakwana · 2023-08-31T10:59:10Z

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

            }
        } else {
-            SimpleDateFormat df = new SimpleDateFormat(connectorConfig.timestampFormat);
+            SimpleDateFormat df = new SimpleDateFormat(connectorConfig.timestampFormat,Locale.ENGLISH);


Why do we need locale change?

without Locale information, in my local build, I get failed from test proceed, because orginally locale set up from system(os) as ko_KR. so I get exception error like this "Unparseable date: "Jun 13 2010 23:11:52.454 UTC".
(there's a English Jun)
I believe that the timestamp format of testcode should be changed or this code needs to be added Locale.ENGLISH.

but that's not my testcode, so I will remove Locale.ENGLISH as it was

VihasMakwana · 2023-08-31T10:59:45Z

src/test/java/com/splunk/kafka/connect/SplunkSinkTaskTest.java

        config.put(SplunkSinkConnectorConfig.ENABLE_TIMESTAMP_EXTRACTION_CONF, String.valueOf(true));
        config.put(SplunkSinkConnectorConfig.REGEX_CONF, "\\\"time\\\":\\s*\\\"(?<time>.*?)\"");
        config.put(SplunkSinkConnectorConfig.TIMESTAMP_FORMAT_CONF, "MMM dd yyyy HH:mm:ss.SSS zzz");
+//        config.put(SplunkSinkConnectorConfig.TIMESTAMP_TIMEZONE_CONF, "");


I think we can remove it, as it defaults to empty string.

yes I agree =)

github-actions · 2023-08-31T11:01:05Z

Unit Test Results

167 tests 167 ✔️ 38s ⏱️
  23 suites     0 💤
  23 files     0 ❌

Results for commit 1c6a738.

♻️ This comment has been updated with latest results.

VihasMakwana · 2023-08-31T12:06:26Z

Can you add test cases covering your config?

VihasMakwana · 2023-08-31T12:56:17Z

@mo-mosh sync develop with your branch, it involves a fix for failing pipeline.

VihasMakwana · 2023-09-02T06:59:58Z

@mo-mosh I can add test cases, if you're occupied.
I'm pushing for this because we (splunk) are planning to release a new minor version with some fixes.
I would love to have this feature in our latest release.

VihasMakwana · 2023-09-02T07:00:18Z

Hope you don't mind if I raise a PR against your repo?

mo-mosh · 2023-09-03T23:56:05Z

I don't mind =) and I'll update repo and gonna add test cases
sorry for the late reply !

mo-mosh · 2023-09-04T07:35:30Z

I updated the code like I added only what I need from the origin develop branch
please review the code @VihasMakwana

VihasMakwana

@mo-mosh some comments.

VihasMakwana · 2023-09-04T11:01:20Z

src/test/java/com/splunk/kafka/connect/SplunkSinkTaskTest.java

+        config.put(SplunkSinkConnectorConfig.ENABLE_TIMESTAMP_EXTRACTION_CONF, String.valueOf(true));
+        config.put(SplunkSinkConnectorConfig.REGEX_CONF, "\\\"REQ_TIME\\\":\\s*\\\"(?<time>.*?)\"");
+        config.put(SplunkSinkConnectorConfig.TIMESTAMP_FORMAT_CONF, "yyyyMMddHHmmssSSS");
+        config.put(SplunkSinkConnectorConfig.TIMESTAMP_TIMEZONE_CONF, "Asis/Seoul");


Suggested change

config.put(SplunkSinkConnectorConfig.TIMESTAMP_TIMEZONE_CONF, "Asis/Seoul");

config.put(SplunkSinkConnectorConfig.TIMESTAMP_TIMEZONE_CONF, "Asia/Seoul");

Oh my typo 😭

src/test/java/com/splunk/kafka/connect/SplunkSinkTaskTest.java

VihasMakwana

LGTM

mo-mosh added 3 commits August 28, 2023 11:25

added a timestamp.timezone configuration in SplunkSinkConnectorConfig…

9576ca7

… for the connector config

added a timestamp.timezone configuration in SplunkSinkConnectorConfig

07d16eb

code sync

e9d1ad2

mo-mosh temporarily deployed to workflow-approval August 28, 2023 02:58 — with GitHub Actions Inactive

VihasMakwana reviewed Aug 31, 2023

View reviewed changes

Merge branch 'splunk:develop' into develop

16945bd

mo-mosh had a problem deploying to workflow-approval August 31, 2023 23:20 — with GitHub Actions Failure

mo-mosh added 3 commits September 4, 2023 10:12

Merge remote-tracking branch 'upstream/develop' into develop

cce5a5d

added a timestamp.timezone configuration in SplunkSinkConnectorConfig

4d7e611

Merge remote-tracking branch 'origin' into develop

8673160

mo-mosh had a problem deploying to workflow-approval September 4, 2023 07:27 — with GitHub Actions Failure

added a timestamp.timezone configuration in SplunkSinkConnectorConfig

ad9b600

mo-mosh temporarily deployed to workflow-approval September 4, 2023 07:31 — with GitHub Actions Inactive

VihasMakwana self-requested a review September 4, 2023 10:59

VihasMakwana reviewed Sep 4, 2023

View reviewed changes

updated testcases

1c6a738

mo-mosh temporarily deployed to workflow-approval September 5, 2023 02:30 — with GitHub Actions Inactive

VihasMakwana self-requested a review September 5, 2023 06:23

VihasMakwana approved these changes Sep 5, 2023

View reviewed changes

VihasMakwana merged commit 44c9d9a into splunk:develop Sep 5, 2023

	config.put(SplunkSinkConnectorConfig.TIMESTAMP_TIMEZONE_CONF, "Asis/Seoul");
	config.put(SplunkSinkConnectorConfig.TIMESTAMP_TIMEZONE_CONF, "Asia/Seoul");

Conversation

mo-mosh commented Aug 28, 2023

Uh oh!

VihasMakwana Aug 31, 2023

Choose a reason for hiding this comment

Uh oh!

mo-mosh Sep 4, 2023 • 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!

VihasMakwana Aug 31, 2023

Choose a reason for hiding this comment

Uh oh!

mo-mosh Sep 4, 2023

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Aug 31, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Unit Test Results

Uh oh!

VihasMakwana commented Aug 31, 2023

Uh oh!

VihasMakwana commented Aug 31, 2023

Uh oh!

VihasMakwana commented Sep 2, 2023

Uh oh!

VihasMakwana commented Sep 2, 2023

Uh oh!

mo-mosh commented Sep 3, 2023

Uh oh!

mo-mosh commented Sep 4, 2023

Uh oh!

VihasMakwana left a comment

Choose a reason for hiding this comment

Uh oh!

VihasMakwana Sep 4, 2023

Choose a reason for hiding this comment

Uh oh!

mo-mosh Sep 5, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

VihasMakwana left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

mo-mosh Sep 4, 2023 •

edited

Loading

github-actions bot commented Aug 31, 2023 •

edited

Loading