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 ability to stop Input #27

Open
cretz opened this issue May 28, 2019 · 4 comments
Open

Add ability to stop Input #27

cretz opened this issue May 28, 2019 · 4 comments

Comments

@cretz
Copy link

cretz commented May 28, 2019

Currently this is not very usable as a library since FrameStreamSockInputs ReadInto runs forever, leaking the goroutine. Consider a Close on the input interface (I may submit a PR if I end up using this as a library).

Also, in general this code logs errors silently among other practices that are poor for library use. If adapting the code for use as a library is not a goal, please at least add caveats via comments to warn other potential users.

@cmikk
Copy link
Member

cmikk commented May 29, 2019

Agreed. The golang-dnstap API needs that and other refactoring to be more library-like. Discussion in #23 also noted the need for a zero-copy (or at least less-copying) approach for higher-volume dnstap processors.

As a start, I'd propose:

  • expose lower-level Reader/Writer APIs in addition to the channel-based Input/Output, for callers which need more control
  • add "logger" callbacks to the FrameStreamFOOInput/Output structures
  • add Decoder/Encoder APIs which speak Dnstap structures instead of byte buffers

Other proposals and/or contributions welcome.

@cmikk
Copy link
Member

cmikk commented Jun 5, 2019

Feedback welcome on the API changes in: https://github.com/dnstap/golang-dnstap/tree/issue-27

Note that this requires the "reader-writer" branch of https://github.com/farsightsec/golang-framestream

@cretz
Copy link
Author

cretz commented Jun 5, 2019

I ended up using the framestream lib directly. Didn't dig too deep, but might want to golint (first word of Godoc invalid), also seeing misspellings like "SocktWriter", etc. But I'll defer to others on substantive review.

@mvanholsteijn
Copy link

Just added a new output destination which allows dnstap to sent messages to AWS S3 buckets; This issue prevents dnstap itself gracefully shutdown all of the output streams.

This issue just celebrated its 3rd anniversary: Is there any chance that somebody is going to pick this up?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants