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

fluent-plugin-grafana-loki: Add fluentd_thread label when `flush_thread_count' > 1 #1514

Merged

Conversation

srstsavage
Copy link
Contributor

@srstsavage srstsavage commented Jan 13, 2020

What this PR does / why we need it:

Currently fluent-plugin-grafana-loki can't be used with multiple buffer flush threads (num_threads or flush_thread_count > 1) because logs with the same label sets are flushed in parallel, resulting in entry out of order for stream errors in Loki.

This PR changes fluent-plugin-grafana-loki to automatically add a 'fluentd_threadlog label when the fluentd loki output is configured withflush_thread_count` > 1, which allows logs flushed in parallel to have unique label sets and avoid out-of-order errors.

In our deployment (~300 fluentd agents forwarding to a single loki), using this change and setting flush_thread_count to 4 has allowed us to achieve sub-500ms log latecy times (measured with loki-canary). Previously we were seeing 32s latency, even when using multi-worker fluentd configs.

Which issue(s) this PR fixes:
No issue currently, should I create one?

Special notes for your reviewer:
Thanks for reviewing!

Checklist

  • Documentation added
  • Tests updated

@claassistantio
Copy link

claassistantio commented Jan 13, 2020

CLA assistant check
All committers have signed the CLA.

@cyriltovena
Copy link
Contributor

Thank you for this !! Really love that you're contributing back.

Can you bump the gem version ? And we should be good to go.

@srstsavage
Copy link
Contributor Author

@cyriltovena Done. Set it to 1.2.7, unless you think 1.3.0 is better since there is a small behavior change.

Copy link
Contributor

@cyriltovena cyriltovena left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

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

Successfully merging this pull request may close these issues.

3 participants