Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add starlark processor #7660

Merged
merged 30 commits into from
Jun 23, 2020
Merged

Add starlark processor #7660

merged 30 commits into from
Jun 23, 2020

Conversation

danielnelson
Copy link
Contributor

Ready for review at long last, special thanks to @srebhan for the help.

closes #7194
closes #3709

Required for all PRs:

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

@danielnelson danielnelson added this to the 1.15.0 milestone Jun 10, 2020
Copy link
Contributor

@reimda reimda left a comment

Choose a reason for hiding this comment

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

If there's an error when calling the script we write the details to the logfile. Many cases in the unit tests (TestApply and TestMetric) have "expected: []telegraf.Metric{}" when there is an error in the script but they don't check the error in the log. Could we have the table tests verify that the error was sent to the logfile? The table test structs would need to add a field like expected_error_message and then the test code would need to compare it with plugin.Log.

For example, the test at starlark_test.go line 562 should check that the log says "Error calling Starlark: line 2: tag value must be a string" or something similar.

plugins/processors/starlark/README.md Outdated Show resolved Hide resolved
plugins/processors/starlark/README.md Outdated Show resolved Hide resolved
plugins/processors/starlark/starlark_test.go Show resolved Hide resolved
plugins/processors/starlark/starlark_test.go Outdated Show resolved Hide resolved
Copy link
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.

Very cool. Included some feedback

plugins/processors/starlark/README.md Show resolved Hide resolved
plugins/processors/starlark/starlark.go Outdated Show resolved Hide resolved
plugins/processors/starlark/starlark.go Outdated Show resolved Hide resolved
plugins/processors/starlark/starlark.go Outdated Show resolved Hide resolved
plugins/processors/starlark/field_dict.go Show resolved Hide resolved
plugins/processors/starlark/starlark_test.go Outdated Show resolved Hide resolved
plugins/processors/starlark/starlark_test.go Show resolved Hide resolved
plugins/processors/starlark/builtins.go Show resolved Hide resolved
plugins/processors/starlark/builtins.go Show resolved Hide resolved
@danielnelson danielnelson mentioned this pull request Jun 13, 2020
3 tasks
Copy link
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.

🎉

@danielnelson danielnelson merged commit c7cce96 into master Jun 23, 2020
@danielnelson danielnelson deleted the starlark branch June 23, 2020 21:15
rhajek pushed a commit to bonitoo-io/telegraf that referenced this pull request Jul 13, 2020
idohalevi pushed a commit to idohalevi/telegraf that referenced this pull request Sep 29, 2020
arstercz pushed a commit to arstercz/telegraf that referenced this pull request Mar 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add a general purpose processor based on the Starlark language Create a math processor plugin
3 participants