feat(output plugin): Add new output plugin to support Apache IoTDB #11557

citrusreticulata · 2022-07-27T13:30:32Z

Updated associated README.md.
Wrote appropriate unit tests.
Pull request title or commits are in conventional commit format

resolves #

Added an output plugin to support Apache IoTDB.
Modify file "go.mod", added external dependency: iotdb-client-go.
Added readme of Apache IoTDB output plugin in English and Chinese.
Information about relevant database: Apache IoTDB github, Apache IoTDB client for Golang

By the way, unit test of IoTDB output plugin requires an external running database, on 'localhost:6667' by default.

Related issue #11561

localize makefile to fit windows init iotdb plugin

add iotdb config sample in Chinese; add iotdb config sample in Chinese; treat negative number as 0 in 'timeout' complete iotdb.go and it's sample.conf 1. complete iotdb.go， fix datatype array declare fault. 2. complete sample.conf go-mod updated; fix iotdb datatype bug 1. use thrift v0.14.2 to fit iotdb dependency 2. fix a bug in iotdb.go, it a var type mistake use nanosecond as defualt TimeStampUnit; add iotdb_test.go 1. use nanosecond as defualt TimeStampUnit 2. add iotdb_test.go use iotdb-client-go 0.12.1 use iotdb-client-go 0.12.1 but it's deppened on thrift 0.14.1 a pr of supporting thrift 0.15.0 is submitted, waitting for merge. add github.com/stretchr/testify v1.8.0 to go.sum make success update iotdb-client-go thrift to 0.15.0 backup makefile for local windows OS deal with 'Tags' in different methods 1. create function WriteRecordsWithTags to deal with 'Tags' in different methods 2. add config parameter treateTagsAs 3. add struct RecordsWithTags 4. fix typo 'invaild' update sample.conf, use better example update sample.conf, use better example Split function ‘WriteRecordsWithTags’ into multiple； complete tests 1. Split function ‘WriteRecordsWithTags’ into multiple: ConvertMetricsToRecordsWithTags -> ModifiyRecordsWithTags -> WriteRecordsWithTags 2. complete tests solve deep copy problem solve deep copy problem Optimize testing change defualt treateTagsAs; update test cases; update readme_zh 1. change defualt treateTagsAs, now it's "DeviceID_subtree" 2. update test cases, fix bug datatype mismatch. this bug occured becuase same mesurement use different datatype in different test cases. 3. update readme in chinese

1. delete makefile.bak, etc 2. update gitignore

run `go mod tidy`.

update license of dependencies; fix readme 1. update license of dependencies; 2. fix readme to fit lint

update readme.md, delete long url optimize code style, local lint test has passed optimizing rules: 1. do not use underscores in Go names 2. avoid unnecessary conversion 3. use better structure of if-else, and encapsulate complex structures

1. the CI recognize language option as the first section. Of course it's very short so checks failed.

srebhan

Thank you very much @citrusreticulata for this contribution. I have some requests in the code. Please take a look. Please also remove the non-English documentation and configuration examples as we only support English documents and comments.

Looking forward to the next round!

plugins/outputs/iotdb/README.md

plugins/outputs/iotdb/iotdb.go

plugins/outputs/iotdb/iotdb_test.go

1. Code optimized 2. Using container in testing is NOT supported yet

fix some typo

srebhan

Thank you @citrusreticulata for the nice and fast update! This is a huge improvement already. I have some more comments and suggestions, but I feel we are coming closer with the PR. Please also convert your integration test-cases to use containers instead of relying on IoTDB being installed. Also switch to table based tests (see comment) and clearly separate into integration tests and short-tests.

plugins/outputs/iotdb/sample.conf

plugins/outputs/iotdb/iotdb_test.go

citrusreticulata · 2022-07-29T12:04:34Z

Thank you @citrusreticulata for the nice and fast update! This is a huge improvement already. I have some more comments and suggestions, but I feel we are coming closer with the PR. Please also convert your integration test-cases to use containers instead of relying on IoTDB being installed. Also switch to table based tests (see comment) and clearly separate into integration tests and short-tests.

Thank you for your appreciation and quick reply! I will complete the above modification suggestions.

Code optimized according to [review](influxdata#11557 (review))

srebhan

Some small things left. Please do not wrap the functions in the tests as this will hide the location of the actual error. Rather live with code duplication for easier debugging.

plugins/outputs/iotdb/README.md

plugins/outputs/iotdb/iotdb.go

plugins/outputs/iotdb/iotdb_test.go

Code optimized; waiting for final review

fix docker image version

citrusreticulata · 2022-08-02T06:03:46Z

I don't know why but ALL integration tests failed on my local machine.
Not just for iotdb output plugin:

But also for other output plugins,. For example, mongoDB:

They're all stuck at Start ()

This makes it difficult for me to do local debugging

citrusreticulata · 2022-08-02T06:35:03Z

I don't know why testcontainers didn't work.
The CI error log says:

2022/08/02 05:51:17 Starting container id: e50fcba886ae image: testcontainers/ryuk:0.3.3
2022/08/02 05:51:18 Waiting for container id e50fcba886ae image: testcontainers/ryuk:0.3.3
2022/08/02 05:51:18 Container is ready id: e50fcba886ae image: testcontainers/ryuk:0.3.3
2022/08/02 05:51:28 Starting container id: 49995306a977 image: apache/iotdb:0.13.0-node
2022/08/02 05:51:29 Waiting for container id 49995306a977 image: apache/iotdb:0.13.0-node
--- FAIL: TestIntegrationInserts (71.85s)
    iotdb_test.go:275: 
        	Error Trace:	/home/circleci/project/plugins/outputs/iotdb/iotdb_test.go:275
        	Error:      	Received unexpected error:
        	            	container failed to start: failed to start container: context deadline exceeded
        	Test:       	TestIntegrationInserts
        	Messages:   	failed to start IoTDB container
FAIL
FAIL	github.com/influxdata/telegraf/plugins/outputs/iotdb	71.911s

So I reuse the original test method, that is, assume that there is an running instance locally, just like clickhouse , opentsdb or other plugins not using testcontainers.

I know it's not friendly for CI, and testing is a little more troublesome than using testcontainers, but I really can't handle the abnormal situation of testcontainer on my local machines(See last comment).

citrusreticulata · 2022-08-02T13:54:09Z

So maybe dividing this group of tests into two functions is better for understand... I'd better do this.

1. After this commit, Metric Conversion and Tags-Handling are tested in one function. But this commit divided them into two parts. This makes code easier to read.

citrusreticulata · 2022-08-03T03:31:05Z

I have solved the problem that the last test function is not easy to read. All CI tests passed.
And according to actual functionality, I divided Metric Conversion tests into two unit-tests, which are reasonable and meet actual handling process. I think it's time to do final review.

Thank you @srebhan very much for your support and help these days. We have had nearly 100 conversations and gradually improved the code to meet the standards and become excellent, which is inseparable from your help. I am very grateful for this! I also learned a lot from the reviews these days.

srebhan · 2022-08-03T18:44:19Z

I designed it to do unit testing. Because IoTDB is likely to support other tags processing methods in the future, in this case modifyRecordsWithTags() will show different behaviors in this case.

We exactly want to get failing tests in this case! If the behavior changes, we want to know, as this might break existing use-cases. That's my point, your processing will always run both, so your tests (at least some) should also test this!

citrusreticulata · 2022-08-03T19:02:49Z

I designed it to do unit testing. Because IoTDB is likely to support other tags processing methods in the future, in this case modifyRecordsWithTags() will show different behaviors in this case.

We exactly want to get failing tests in this case! If the behavior changes, we want to know, as this might break existing use-cases. That's my point, your processing will always run both, so your tests (at least some) should also test this!

All right. Entire conversion tests and failure test cases will be added in next commit.

1. Added entire metrics conversion tests 2. support failure test cases.

srebhan

Thanks @citrusreticulata for the update and sorry for the late reply. Two very small things and we are good for the final review IMO...

plugins/outputs/iotdb/iotdb_test.go

Update iotdb_test.go; fix some typo

srebhan · 2022-08-05T15:16:48Z

And one merge conflict to be resolved...

srebhan

Looks good to me. Thanks for your contribution @citrusreticulata and also for the nice review process!

citrusreticulata · 2022-08-05T15:41:23Z

Emm... I think the "go.sum" conflict is not for me to solve. I have run go mod tidy so "go.sum" is automatically generated. I think I'd better not edit it manually. Maybe it's because some other PRs didn't run go mod tidy or more dependence was added.

citrusreticulata · 2022-08-05T15:51:06Z

The conflict is that there is a row in the master branch that does not exist in this branch.

github.com/stretchr/testify v1.7.2/go.mod h1:R6va5+xMeoiuVRoj+gSkQ7d3FALtqAAGI1FQKckRals=

I think it's because someone forget to run go mod tidy or more dependencies were added.
But in either case, I think I'd better not modify this file manually, because it's generated automatically. If I delete it, some plugins in master may go wrong, but if I retain it, CI tests will raise errors(I tried it a few minutes ago).

And I run go mod tidy in master branch locally, this row is not removed....
To be honest, I don't know how to deal with this conflict.

make tidy

citrusreticulata · 2022-08-08T03:39:27Z

Problem solved!
I merged upstream breach master into branch apache-iotdb, then run go mod tidy and make docs.
Conflict is resolved and PR is ready to merge now.
Thanks a lot for team's help and support!

reimda

I just have a few suggestions on the readme. Thanks!

plugins/outputs/iotdb/README.md

1. using subheadings instead of unordered lists. 2. uses better words 3. also update relevant comments in sample.conf

telegraf-tiger · 2022-08-09T02:52:30Z

Download PR build artifacts for linux_amd64.tar.gz, darwin_amd64.tar.gz, and windows_amd64.zip.
Downloads for additional architectures and packages are available below.

⚠️ This pull request increases the Telegraf binary size by 1.06 % for linux amd64 (new size: 150.0 MB, nightly size 148.5 MB)

📦 Click here to get additional PR build artifacts

Artifact URLs

DEB	RPM	TAR GZ	ZIP
amd64.deb	aarch64.rpm	darwin_amd64.tar.gz	windows_amd64.zip
arm64.deb	armel.rpm	darwin_arm64.tar.gz	windows_i386.zip
armel.deb	armv6hl.rpm	freebsd_amd64.tar.gz
armhf.deb	i386.rpm	freebsd_armv7.tar.gz
i386.deb	ppc64le.rpm	freebsd_i386.tar.gz
mips.deb	riscv64.rpm	linux_amd64.tar.gz
mipsel.deb	s390x.rpm	linux_arm64.tar.gz
ppc64el.deb	x86_64.rpm	linux_armel.tar.gz
riscv64.deb		linux_armhf.tar.gz
s390x.deb		linux_i386.tar.gz
		linux_mips.tar.gz
		linux_mipsel.tar.gz
		linux_ppc64le.tar.gz
		linux_riscv64.tar.gz
		linux_s390x.tar.gz
		static_linux_amd64.tar.gz

citrusreticulata added 3 commits July 27, 2022 21:13

init iotdb plugin

560c91d

localize makefile to fit windows init iotdb plugin

to fit linux style.

e0e454c

1. delete makefile.bak, etc 2. update gitignore

telegraf-tiger bot added the feat Improvement on an existing feature such as adding a new setting/mode to an existing plugin label Jul 27, 2022

citrusreticulata marked this pull request as draft July 27, 2022 13:45

citrusreticulata added 2 commits July 27, 2022 21:51

make tidy

6af6993

run `go mod tidy`.

optimize readme and testing

d63c1f3

citrusreticulata mentioned this pull request Jul 28, 2022

Add output plugin to support Apache IoTDB #11561

Closed

update license of dependencies; fix readme

5e39fda

update license of dependencies; fix readme 1. update license of dependencies; 2. fix readme to fit lint

citrusreticulata force-pushed the apache-iotdb branch from 0175dd9 to 5e39fda Compare July 28, 2022 02:35

optimize code style

f4cc0ee

update readme.md, delete long url optimize code style, local lint test has passed optimizing rules: 1. do not use underscores in Go names 2. avoid unnecessary conversion 3. use better structure of if-else, and encapsulate complex structures

citrusreticulata force-pushed the apache-iotdb branch from 5b049f9 to f4cc0ee Compare July 28, 2022 04:59

update readme, added a longer first section

a6d1789

1. the CI recognize language option as the first section. Of course it's very short so checks failed.

citrusreticulata marked this pull request as ready for review July 28, 2022 05:20

srebhan requested changes Jul 28, 2022

View reviewed changes

srebhan self-assigned this Jul 28, 2022

srebhan added area/iot New plugins or features relating to IoT monitoring plugin/output 1. Request for new output plugins 2. Issues/PRs that are related to out plugins labels Jul 28, 2022

citrusreticulata added 2 commits July 29, 2022 13:38

Code optimized; waiting to support container

021900a

1. Code optimized 2. Using container in testing is NOT supported yet

fix some typo

181d437

fix some typo

srebhan reviewed Jul 29, 2022

View reviewed changes

Code optimized; waiting to test

1e5d002

Code optimized according to [review](influxdata#11557 (review))

citrusreticulata requested a review from srebhan July 29, 2022 16:42

srebhan reviewed Aug 1, 2022

View reviewed changes

citrusreticulata added 2 commits August 2, 2022 12:21

Code optimized; fix README.md typo

0832eaa

Code optimized; waiting for final review

fix docker image version

83f6711

fix docker image version

citrusreticulata force-pushed the apache-iotdb branch from 8a831c5 to 83f6711 Compare August 2, 2022 05:43

divided Conversion and Tags-Handling into unit tests

17f0e4f

1. After this commit, Metric Conversion and Tags-Handling are tested in one function. But this commit divided them into two parts. This makes code easier to read.

citrusreticulata requested a review from srebhan August 3, 2022 03:31

Added entire metrics conversion tests

2658193

1. Added entire metrics conversion tests 2. support failure test cases.

srebhan reviewed Aug 5, 2022

View reviewed changes

plugins/outputs/iotdb/iotdb_test.go Outdated Show resolved Hide resolved

plugins/outputs/iotdb/iotdb_test.go Outdated Show resolved Hide resolved

Update iotdb_test.go; fix some typo

15cef01

Update iotdb_test.go; fix some typo

citrusreticulata requested a review from srebhan August 5, 2022 14:59

srebhan approved these changes Aug 5, 2022

View reviewed changes

srebhan added the ready for final review This pull request has been reviewed and/or tested by multiple users and is ready for a final review. label Aug 5, 2022

citrusreticulata force-pushed the apache-iotdb branch from c46fd34 to 15cef01 Compare August 5, 2022 15:37

citrusreticulata added 2 commits August 8, 2022 10:08

Merge remote-tracking branch 'upstream/master' into apache-iotdb

bd08e84

make tidy

28210db

make tidy

srebhan approved these changes Aug 8, 2022

View reviewed changes

reimda reviewed Aug 8, 2022

View reviewed changes

plugins/outputs/iotdb/README.md Outdated Show resolved Hide resolved

citrusreticulata added 2 commits August 9, 2022 10:26

update README.md and sample.conf;

cddafdb

1. using subheadings instead of unordered lists. 2. uses better words 3. also update relevant comments in sample.conf

fix long line bug...

8c9432e

srebhan approved these changes Aug 9, 2022

View reviewed changes

powersj approved these changes Aug 10, 2022

View reviewed changes

powersj merged commit 5d669e2 into influxdata:master Aug 10, 2022

citrusreticulata deleted the apache-iotdb branch August 26, 2022 03:53

citrusreticulata restored the apache-iotdb branch August 26, 2022 03:53

sjwang90 mentioned this pull request Sep 7, 2022

Telegraf 1.24 influxdata/docs-v2#4422

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(output plugin): Add new output plugin to support Apache IoTDB #11557

feat(output plugin): Add new output plugin to support Apache IoTDB #11557

citrusreticulata commented Jul 27, 2022 •

edited

Loading

srebhan left a comment

srebhan left a comment

citrusreticulata commented Jul 29, 2022

srebhan left a comment

citrusreticulata commented Aug 2, 2022

citrusreticulata commented Aug 2, 2022

citrusreticulata commented Aug 2, 2022

citrusreticulata commented Aug 3, 2022

srebhan commented Aug 3, 2022

citrusreticulata commented Aug 3, 2022

srebhan left a comment

srebhan commented Aug 5, 2022

srebhan left a comment

citrusreticulata commented Aug 5, 2022

citrusreticulata commented Aug 5, 2022

citrusreticulata commented Aug 8, 2022

reimda left a comment

telegraf-tiger bot commented Aug 9, 2022

Artifact URLs

feat(output plugin): Add new output plugin to support Apache IoTDB #11557

feat(output plugin): Add new output plugin to support Apache IoTDB #11557

Conversation

citrusreticulata commented Jul 27, 2022 • edited Loading

srebhan left a comment

Choose a reason for hiding this comment

srebhan left a comment

Choose a reason for hiding this comment

citrusreticulata commented Jul 29, 2022

srebhan left a comment

Choose a reason for hiding this comment

citrusreticulata commented Aug 2, 2022

citrusreticulata commented Aug 2, 2022

citrusreticulata commented Aug 2, 2022

citrusreticulata commented Aug 3, 2022

srebhan commented Aug 3, 2022

citrusreticulata commented Aug 3, 2022

srebhan left a comment

Choose a reason for hiding this comment

srebhan commented Aug 5, 2022

srebhan left a comment

Choose a reason for hiding this comment

citrusreticulata commented Aug 5, 2022

citrusreticulata commented Aug 5, 2022

citrusreticulata commented Aug 8, 2022

reimda left a comment

Choose a reason for hiding this comment

telegraf-tiger bot commented Aug 9, 2022

Artifact URLs

citrusreticulata commented Jul 27, 2022 •

edited

Loading