Skip to content

Add the Timestream Output Plugin#8211

Closed
piotrwest wants to merge 1 commit intoinfluxdata:release-1.15from
aws:telegraf_v1.15.3_with_Timestream
Closed

Add the Timestream Output Plugin#8211
piotrwest wants to merge 1 commit intoinfluxdata:release-1.15from
aws:telegraf_v1.15.3_with_Timestream

Conversation

@piotrwest
Copy link
Copy Markdown
Contributor

@piotrwest piotrwest commented Oct 1, 2020

Required for all PRs:

  • Signed CLA.
  • Associated README.md updated.
  • Has appropriate unit tests.

We have created a new Telegraf output plugin that is able to write metrics to Amazon Timestream. Please see the details in README.md file.

Closes #5539

@sjwang90
Copy link
Copy Markdown
Contributor

sjwang90 commented Oct 1, 2020

Hey @piotrwest, if you're an employee of Amazon we also need you and any other employees involved in building this plugin to sign the Corporate CLA.

Amazon.com Services, Inc has executed a CCLA in the past but we would still need you to be covered on the Schedule A portion.

Please let us know if you have any questions!

@sjwang90
Copy link
Copy Markdown
Contributor

sjwang90 commented Oct 1, 2020

Amazon Timestream Issue #5539

@sjwang90 sjwang90 added area/aws AWS plugins including cloudwatch, ecs, kinesis new plugin labels Oct 1, 2020
Copy link
Copy Markdown
Contributor

@ssoroka ssoroka left a comment

Choose a reason for hiding this comment

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

Thanks for the PR. I have a couple suggestions.
Looks like the build is failing due to module versions. might need to rebase and go mod tidy

Execute unit tests with:

```
go test -v ./plugins/outputs/timestream/timestream_test.go ./plugins/outputs/timestream/timestream_internal_test.go ./plugins/outputs/timestream/timestream.go
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
go test -v ./plugins/outputs/timestream/timestream_test.go ./plugins/outputs/timestream/timestream_internal_test.go ./plugins/outputs/timestream/timestream.go
go test -v ./plugins/outputs/timestream/...


func (t *Timestream) writeToTimestream(writeRecordsInput *timestreamwrite.WriteRecordsInput, resourceNotFoundRetry bool) error {
if t.Debug {
log.Printf("I! Timestream: Writing to Timestream: '%v' with ResourceNotFoundRetry: '%t'", writeRecordsInput, resourceNotFoundRetry)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

instead of adding a Debug parameter and using log.Printf, you can just use the Log telegraf.Logger you already added to the struct, which has a Debug method, and respects the global debug flags. For tests, set the logger to testutil.Logger{}. Then you can also remove the LogLevel and plugin name from the log lines and they will conform to the Telegraf standards.

@sjwang90
Copy link
Copy Markdown
Contributor

sjwang90 commented Oct 6, 2020

Hey @piotrwest! We were able to get you on Amazon's Corporate CLA. Once @ssoroka's comments are addressed we can look at getting this merged.

@piotrwest piotrwest mentioned this pull request Oct 7, 2020
3 tasks
@piotrwest
Copy link
Copy Markdown
Contributor Author

That's great, thank you @sjwang90 .

I had created a new pull request: #8239 . Closing this one.

I will continue with applying feedback from @ssoroka there, which was not possible here, as the source branch name (telegraf_v1.15.3_with_Timestream) was fixed on a specific version. I didn't want to confuse the customers by reusing it for a pull request purposes.

@piotrwest piotrwest closed this Oct 7, 2020
@piotrwest piotrwest deleted the telegraf_v1.15.3_with_Timestream branch January 13, 2021 16:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/aws AWS plugins including cloudwatch, ecs, kinesis new plugin

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants