x-pack/filebeat/input/streaming: add falcon hose stream follower #40838
Conversation
efd6
added
enhancement
Filebeat
botelastic
bot
added
needs_team
|
This pull request does not have a backport label.
To fixup this pull request, you need to add the backport labels for the needed
|
|
|
mergify
bot
added
the
backport-8.x
efd6
force-pushed
the
40264-crowdstrike
branch
3 times, most recently
from
3fd22c6
to
473744b
Compare
|
Pinging @elastic/security-service-integrations (Team:Security-Service Integrations) |
pierrehilbert
added
the
Team:Elastic-Agent-Data-Plane
|
Pinging @elastic/elastic-agent-data-plane (Team:Elastic-Agent-Data-Plane) |
pierrehilbert
requested review from
belimawr and
rdner
and removed request for
mauri870 and
VihasMakwana
| if offset > 0 { | ||
| feedURL, err := url.Parse(r.FeedURL) | ||
| if err != nil { | ||
| log.Fatalf("failed to parse feed url: %v", err) | ||
| } | ||
| feedQuery, err := url.ParseQuery(feedURL.RawQuery) | ||
| if err != nil { | ||
| log.Fatalf("failed to parse feed query: %v", err) | ||
| } | ||
| feedQuery.Set("offset", strconv.Itoa(offset)) | ||
| feedURL.RawQuery = feedQuery.Encode() | ||
| r.FeedURL = feedURL.String() | ||
| } |
There was a problem hiding this comment.
The calls to log.Fatalf on this block will cause Filebeat to exit instantly. An input failure should not crash the whole Filebeat instance.
If there is no way for the input to recover, returning an error, logging and stopping the input would be the best way to handle it.
There was a problem hiding this comment.
This is accidental leftovers from the original toy.
| if err != nil { | ||
| s.metrics.errorsTotal.Inc() | ||
| if err == io.EOF { | ||
| s.log.Info("stream ended, restarting") | ||
| return state, nil | ||
| } | ||
| return state, Warning{fmt.Errorf("error decoding event: %w", err)} | ||
| } |
There was a problem hiding this comment.
An EOF just signals the end of the stream, looking at how it's handled, I believe it should not be counted as an error for metrics.
| if err != nil { | |
| s.metrics.errorsTotal.Inc() | |
| if err == io.EOF { | |
| s.log.Info("stream ended, restarting") | |
| return state, nil | |
| } | |
| return state, Warning{fmt.Errorf("error decoding event: %w", err)} | |
| } | |
| if err != nil { | |
| if err == io.EOF { | |
| s.log.Info("stream ended, restarting") | |
| return state, nil | |
| } | |
| s.metrics.errorsTotal.Inc() | |
| return state, Warning{fmt.Errorf("error decoding event: %w", err)} | |
| } |
There was a problem hiding this comment.
No, in this case an EOF would only happen if something went wrong in keeping up with session refresh requests. So I would like to keep this as an error metric addition.
|
wrt to the lint complaints, I fixed the errors that it noted that are meaningful. The two remaining are related to untouched code (the unused field) and what is an antipattern of using |
| } | ||
| feedQuery, err := url.ParseQuery(feedURL.RawQuery) | ||
| if err != nil { | ||
| log.Fatalf("failed to parse feed query: %v", err) | ||
| return state, Warning{fmt.Errorf("failed to parse feed query: %v", err)} |
There was a problem hiding this comment.
That's a valid warning from the lint, the error should be wrapped
| return state, Warning{fmt.Errorf("failed to parse feed query: %v", err)} | |
| return state, Warning{fmt.Errorf("failed to parse feed query: %w", err)} |
|
This pull request is now in conflicts. Could you fix it? 🙏 |
efd6
force-pushed
the
40264-crowdstrike
branch
from
5e54e80
to
b7079ec
Compare
efd6
force-pushed
the
40264-crowdstrike
branch
from
b7079ec
to
5d34c42
Compare
|
/cc @elastic/ingest-eng-prod for codeowners change. |
|
/test |
) (cherry picked from commit e561736)
Proposed commit message
See title.
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