feat: Add input plugin for ctrlX Data Layer
Required for all PRs:
- [x] Updated associated README.md.
- [x] Wrote appropriate unit tests.
- [x] Pull request title or commits are in conventional commit format
Thanks so much for the pull request!
:handshake: :black_nib: Just a reminder that the CLA has not yet been signed, and we'll need it before merging. Please sign the CLA when you get a chance, then post a comment here saying !signed-cla
Thanks so much for the pull request!
:handshake: :black_nib: Just a reminder that the CLA has not yet been signed, and we'll need it before merging. Please sign the CLA when you get a chance, then post a comment here saying !signed-cla
Thanks so much for the pull request!
:handshake: :black_nib: Just a reminder that the CLA has not yet been signed, and we'll need it before merging. Please sign the CLA when you get a chance, then post a comment here saying !signed-cla
Thanks so much for the pull request!
:handshake: :black_nib: Just a reminder that the CLA has not yet been signed, and we'll need it before merging. Please sign the CLA when you get a chance, then post a comment here saying !signed-cla
Thanks so much for the pull request!
:handshake: :black_nib: Just a reminder that the CLA has not yet been signed, and we'll need it before merging. Please sign the CLA when you get a chance, then post a comment here saying !signed-cla
Thanks so much for the pull request!
:handshake: :black_nib: Just a reminder that the CLA has not yet been signed, and we'll need it before merging. Please sign the CLA when you get a chance, then post a comment here saying !signed-cla
Thanks so much for the pull request!
:handshake: :black_nib: Just a reminder that the CLA has not yet been signed, and we'll need it before merging. Please sign the CLA when you get a chance, then post a comment here saying !signed-cla
Thanks so much for the pull request!
:handshake: :black_nib: Just a reminder that the CLA has not yet been signed, and we'll need it before merging. Please sign the CLA when you get a chance, then post a comment here saying !signed-cla
Thanks so much for the pull request!
:handshake: :black_nib: Just a reminder that the CLA has not yet been signed, and we'll need it before merging. Please sign the CLA when you get a chance, then post a comment here saying !signed-cla
Thanks so much for the pull request!
:handshake: :black_nib: Just a reminder that the CLA has not yet been signed, and we'll need it before merging. Please sign the CLA when you get a chance, then post a comment here saying !signed-cla
Thanks so much for the pull request!
:handshake: :black_nib: Just a reminder that the CLA has not yet been signed, and we'll need it before merging. Please sign the CLA when you get a chance, then post a comment here saying !signed-cla
Thanks @srebhan for your comprehensive review of the plugin. Indeed, this led in an enormous improvement of our implementation. Please find all comments above.
Regarding your major points:
- Done
- Done
- Still open, see our comment above.
- Done
- Will be implemented as a next feature after this merge with separate PR.
- We think the current implementation is lean and sufficient and we do not need many of the features of those 3rd party dependencies. E.g. we actually started with https://github.com/r3labs/sse but later we noticed it is far too complex and we need only a client and no server implementation.
CLA and CCLA are on its way. Will sign as soon as we are done with the code.
Thanks again, we are also looking forward to the next round ;-)
Hi @srebhan, one question regarding input format parsing. We want to use a built-in parser in order to stream all data types from ctrlX DataLayer to the InfluxDB. In addition we want to use the parsers available with the property 'data_format' for advanced users. Is there already such a mechanism implemented in any input plugin? If not, how should we name the property, which switches between built-in parsing and 'data_format' parsing?
Our first approach is like this:
built-in formatting is true (default) means, data_format parser is not used:
[[inputs.ctrlx_datalayer_sse]]
builtin_formatting =true
built-in formatting is false means, data_format parser is used:
[[inputs.ctrlx_datalayer_sse]]
builtin_formatting =false
data_format ="value"
data_type ="string"
Well the best way would be to write a parser and let telegraf do the magic. Otherwise, you might run into issues where use_builtin_parser is set to true, but the user also specified a data_format...
If this is not an option, I would add a use_builtin_parser option that takes a boolean value...
Hi @srebhan , here is a new commit with the suggest improvements. Please give it a go. Thanks for your patience and helpful comments. :-)
Pushed the first commit with the obvious fixes. More to come in the next days. Have a nice weekend.
Just committed fixes and comments for the remaining review findings. Please note, that we also have a little functional enhancement in the pipeline to make the field-names customizable. Pretty similar to how it is realized in the OPC UA plugin. Will push the feature as soon as you are fine with the current state of findings.
Thanks again for your help.
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.
:relaxed: This pull request doesn't significantly change the Telegraf binary size (less than 1%)
:package: Click here to get additional PR build artifacts
Artifact URLs
@srebhan @powersj Short status info: We are currently in work to refactor the sse implementation into an external repository as you proposed. This protocol repository will be maintained by us. You may expect a new commit soon.
@krauskopf any news on this PR?
@srebhan Sry, took some time to align development of the refactorings. We are working on the topic and will provide a commit soon.
Hi @srebhan , we updated the PR and you can have a look at it now. SSE is refactored to own repository as you suggested. Thanks!
@srebhan as mentioned we have updated the PR. Do you have a chance to look at the PR? Thanks
FYI March 13 is the scheduled date for v1.26.0, so I'd like to see this land if possible before then! :)
Hi @albrecht-j,
Looks like it has been a bit. @srebhan has left some comments and was wondering if you were going to continue this pr.
Thanks!
Hi @powersj, sorry for the delay. We are working on the comments of @srebhan. We have renamed the plugin. You can expect a new commit in the next days.
Hi @srebhan , we updated the PR and fixed the issues you mentioned:
- For the option 'output_json_string' we added some documentation in the README.md.
- The plugin is renamed as you suggested.
- The field name is now generated explicitly with name or implicitly with address of node configuration
- The client Subscribe method returns now an error 'context.Canceled' and is evaluated with errors.Is(err, context.Canceled) Thanks!
Hi @srebhan, sorry for the broken PR - we synchronized the branch with the latest master sources. Please look again at our PR. Thanks!
Is this PR still ongoing? Can it be marked as draft until the requested changes are done?
Hi @Hipska, we commited all change requests. Thanks for the really good review on our code!
@Hipska I'm a bit confused, is this now ready-for-final-review or do you want your comments addressed?