x-pack/filebeat/input/websocket: refactor input for generalisation #40308
Conversation
efd6
added
enhancement
Filebeat
botelastic
bot
added
needs_team
efd6
force-pushed
the
40264-websocket_refactor
branch
from
5d44c53
to
ed71a8b
Compare
|
Pinging @elastic/security-service-integrations (Team:Security-Service Integrations) |
This extracts the common logic between streaming inputs into a new interface type to allow addition of new streaming inputs.
efd6
force-pushed
the
40264-websocket_refactor
branch
from
ed71a8b
to
7deec48
Compare
| return s.time() | ||
| } | ||
|
|
||
| func (s *websocketStream) Close() error { |
There was a problem hiding this comment.
Should the websocket stream contain a handler to the ws connection and (s *websocketStream) Close() function should also take care of gracefully closing the ws connection if present ? ISTM that this would make make the design more consistent.
There was a problem hiding this comment.
Unless I'm missing something, that would be a change in functionality. This is a pure refactor with no change in behaviour. If this is something that should be done (rather than something I've missed) then it should happen in another PR. I'm not convinced that doing this would make things simpler; in order to do this we would need the websocketStream to hold on to the ws conn so that it can close it while at the moment it just naturally happens due to the c.Close() call in the follow method.
There was a problem hiding this comment.
Yea, I was just speaking from a design perspective here, not that this would make it simpler, just more consistent in this new structure. I agree that if we consider to do this it should be a separate PR and not this one so approving this.
Proposed commit message
This extracts the common logic between streaming inputs into a new interface type to allow addition of new streaming inputs.
No behaviour change.
Checklist
CHANGELOG.next.asciidocorCHANGELOG-developer.next.asciidoc.Disruptive User Impact
Author's Checklist
How to test this PR locally
Related issues
Use cases
Screenshots
Logs