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

Adding "encoder" modules #228

Open
KristianLyng opened this issue Apr 29, 2022 · 2 comments
Open

Adding "encoder" modules #228

KristianLyng opened this issue Apr 29, 2022 · 2 comments
Labels
core/modules Anything dealing with the general module support

Comments

@KristianLyng
Copy link
Collaborator

KristianLyng commented Apr 29, 2022

Today, receivers are agnostic to the data encoding format. E.g.: The HTTP receiver can decode json, influx, protocol buffers, m&r etc, and all the other receivers can do that as well, since it's coupled with the handler.

However, the same can not be said for senders. Most of the time this isn't a big deal, since specific senders typically require small tweaks - the influx sender might require specific HTTP headers for sending a token to authenticate for example.

However, it's becoming increasingly obvious that being able to separate encoding from transport would be beneficial. So as such, a new module type is required, the inverse of parsers (coincidentally, maybe we should rename parsers to "decoders"?).

I don't think every sender should be required to use an encoder - it makes no sense at all to implement an SQL sender using a generic encoder for example, and by and large, sender modules should do what you expect them to do by default, with no encoder set. But this change should at least reduce code re-use in the HTTP senders(plural - http, influx and hec all use similar code).

This also ties in nicely with #227 - being able to send GOB data over HTTPS, but also UDP could make sense - use UDP to duplicate data to test instances, which would isolate the sending skogul from issues with the receiving skogul.

Great care needs to be taken to avoid further overcomplicating the configuration though.

@KristianLyng KristianLyng added sender Sender-related issue good first issue Good for newcomers parser core/modules Anything dealing with the general module support and removed sender Sender-related issue good first issue Good for newcomers parser labels Apr 29, 2022
@KristianLyng
Copy link
Collaborator Author

KristianLyng commented May 11, 2022

I've added this support, but so far there's only one encoder which makes this completely worthless.

This issue should track adding of the following encoders, which will likely require unifying some logic in the HTTP sender:

  • Influx line protocol encoder
  • HEC encoder

@KristianLyng
Copy link
Collaborator Author

Ok, I ran into a conundrum today. The Kafka sender has implicit support for batching, that means the metrics[] logic is superfluous. I ended up adding a EncodeMetric method to the Encoder interface, but I'm still not convinced this is a universal thing, and it means there's a miss-match between encoder and parser.

The approach I used will "flatten" the container and transmit each metric as independent messages, which presumably makes things easier for other consumers, but it also mandated that I made a "JSONMetric"/"SkogulMetric" parser to match it, and now we have a situation where the JSON encoder can be used to send data with the HTTP sender, and then parsed by the JSON parser through a HTTP receiver, but you CAN'T use the JSON parser to parse messages sent by over Kafka, using the same encoder, because it's up to the sender which method to use.

I don't think there's an obvious solution here, and currently I'm leaning towards just seeing how this plays out as we extend the encoder-concept.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core/modules Anything dealing with the general module support
Projects
None yet
Development

No branches or pull requests

1 participant