#34054 Add an optional delay for shutting down the fluentforwarder plugin #43978
Conversation
…ntforwarder plugin
|
Welcome, contributor! Thank you for your contribution to opentelemetry-collector-contrib. Important reminders:
A maintainer will review your pull request soon. Thank you for helping make OpenTelemetry better! |
| package fluentforwardreceiver // import "github.com/open-telemetry/opentelemetry-collector-contrib/receiver/fluentforwardreceiver" | ||
|
|
||
| // Config defines configuration for the fluentforward receiver. | ||
| import "time" // Config defines configuration for the fluentforward receiver. |
There was a problem hiding this comment.
this will not pass CI, please move up in the import block
| if r.listener == nil { | ||
| return nil | ||
| } | ||
| time.Sleep(r.waitBeforeShutdownDuration) |
There was a problem hiding this comment.
this doesn't seem like a good solution to me. It is possible more requests be made to the port while we wait.
There was a problem hiding this comment.
That's intended, this is to catch the case where (in my infra) AWS Firelens hasn't sent everything yet, so this adds a delay to ensure those logs are received
There was a problem hiding this comment.
Something is way off. I would build a test that continuously sends requests and counts them, and compares to successful logs received. On the first request refused, we stop the test and make sure we didn't drop any request.
The issue seems to come from the server.go file ; it's doing a few too many things with goroutines that can be problematic.
There was a problem hiding this comment.
Just to confirm, is that intended test to confirm that logs are being dropped from Firelens or a unit test that could be added to check behaviour here?
|
Please add a changelog and tests. |
I'm not sure exactly how to test this, verify that an amount of time has passed after issuing a shutdown call to the receiver and it's shut down? Or is there a way that doesn't involve making the CI sleep? |
|
To add a changelog, type |
4 participants
Description
#34054 - When fluentforward is used as a firelens target by a sidecar logs can be dropped due to delays in sending/receiving.
Link to tracking issue
#34054
Testing
Verified there is a substantial difference between 1s, 10s, and unset configuration options locally, unsure what the best method of testing that with a unit test would be
Documentation
I believe this is auto-generated?