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.

Sign up for GitHub

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

Jump to bottom

Add acknowledgement support to fluent source #7533

Closed
jszwedko opened this issue May 20, 2021 · 7 comments
Closed

Add acknowledgement support to fluent source #7533

jszwedko opened this issue May 20, 2021 · 7 comments
Labels
source: fluent Anything `fluent` source related type: enhancement A value-adding code change that enhances its existing functionality.

Comments

@jszwedko
Copy link
Member

jszwedko commented May 20, 2021 •
edited by binarylogic
Loading

As part of #419 we are adding a fluent source, but chose not to include Fluent's acknowledgement support in the initial implementation due to the additional work it will require. This issue represents that work.

Please leave a 👍 or comment below if you would find this support useful.
@jszwedko jszwedko added the type: enhancement A value-adding code change that enhances its existing functionality. label May 20, 2021
@jszwedko jszwedko mentioned this issue May 21, 2021
Merged
@bruceg bruceg mentioned this issue May 28, 2021
Closed
jszwedko added a commit that referenced this issue Jun 7, 2021
@jszwedko
feat(new source): New fluent source (#7548)
1408491 
New source for consuming data from fluentd and fluent-bit via their native [forward protocol](https://github.com/fluent/fluentd/wiki/Forward-Protocol-Specification-v1).

This implementation supports the various formats that the specification allows for but does not implement the authentication protocol (#7532) or the acknowledgment (#7533) parts of the protocol. I left these for follow-up based on user demand given it is a significant divergence from the existing TCP source which just consumes data rather than request/response.

Aside from the source implementation, the other significant change here is extraction of some bits of the `docker_logs` source into a shared `docker` module to use it in the `fluent` source integration tests. The integration tests are more similar to the `docker_logs` integration tests than the other component integration tests in that they each run a separate docker container rather than just standing up one long running one before running the tests as these tests need to run fluentd and fluent-bit with different configurations.

Apologies, I didn't break up the early commits into logical changes, but hopefully the PR itself is reviewable.

Closes #419 

Signed-off-by: Jesse Szwedko <[email protected]>
@bruceg bruceg mentioned this issue Nov 19, 2021
Closed
@jszwedko jszwedko added the source: fluent Anything `fluent` source related label Dec 28, 2022
@zamazan4ik
Copy link
Contributor

@jszwedko could you please rename the issue? From "sink" to "source" :)

@jszwedko jszwedko changed the title Add acknowledgement support to fluent sink Add acknowledgement support to fluent source May 2, 2023
@jszwedko
Copy link
Member Author

jszwedko commented May 2, 2023

@jszwedko could you please rename the issue? From "sink" to "source" :)

Done. Thanks for the prompt!

@zamazan4ik
Copy link
Contributor

Thanks for the fix!

However, I found these lines of code in the fluent source: https://github.com/vectordotdev/vector/blob/master/src/sources/fluent/mod.rs#L529

Does it implement some kind of ack-ing for fluent? According to the PR - acknowledgment support is already implemented.

In the local Vector-related Telegram chat I found a user with a problem with the current ACK implementation.

@stanislavdip
Copy link

@zamazan4ik @jszwedko
Hello. Yes, I have some problems.
I using Vector as an aggregator and fluent-bit as a log collector, but in fluent-bit logs, I see errors like this:

[error] [engine] chunk '96-1683047707.895527211.flb' cannot be retried: task_id=0, input=tail.0 > output=forward.0
[error] [output:forward:forward.0] ACK response not MAP (type:2)

Vector deployed via Helm chart version: 0.20.1 with these options:

customconfig:   
  acknowledgements:
      enabled: true
image:
  tag: "0.29.1-distroless-libc"

Vector configuration is:

vector:
  address: 0.0.0.0:9000
  type: fluent
  receive_buffer_bytes: 1048576
  connection_limit: 2000
  keepalive:
     time_secs: 30

fluent-operator deployed via Helm chart 2.2.0, also tried 2.1.0

fluent-bit:
  image:      
    repository: "kubesphere/fluent-bit"
    tag: "v2.0.11"
  forward:    
    host: vector.fluent.svc    
    port: 9000    
    requireAckResponse: true

If you need more details, I can provide it.
Thank you

@ChezBunch ChezBunch mentioned this issue May 16, 2023
Merged
@neuronull
Copy link
Contributor

Just spent a couple mins reading up on the history here.

Does it implement some kind of ack-ing for fluent? According to the #10176 - acknowledgment support is already implemented.

I think @zamazan4ik is correct that it looks to me like #10176 should have closed this issue.

The issue

[error] [output:forward:forward.0] ACK response not MAP (type:2)

, appears to be an issue with the ack support . And hence deserves it's own separate tracking issue, that #17407 would fix (currently that issue is flagged as fixing this one).

@ChezBunch ChezBunch mentioned this issue May 17, 2023
Closed
@ChezBunch
Copy link
Contributor

ChezBunch commented May 17, 2023 •
edited
Loading

I created a specific issue for the wrong ack format returned by fluent source: #17427
This issue should now be closed since acknowledgment support was implemented in #10176

@jszwedko
Copy link
Member Author

Closed by #10176

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
source: fluent Anything `fluent` source related type: enhancement A value-adding code change that enhances its existing functionality.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@jszwedko @ChezBunch @zamazan4ik @neuronull @stanislavdip