Unite docs for fluentd plugin #1634

takanabe · 2020-02-05T03:02:48Z

What this PR does / why we need it:

To improve documents around fluent-plugin-grafana-loki.

doc/client/fluentd.md includes obsolete explanations and it confuses new users. For example, label_keys option was removed at #1186 but fluentd.md has the explanation. It's simple to maitain fluentd/fluent-plugin-grafana-loki/README.md for the explantion of the output plugin and use the link when we share how to use it.

Which issue(s) this PR fixes:

N/A

Special notes for your reviewer:

~~I found a tiny bug in docker-compose.yml for this plugin and fix it in 8ed58ac. If I should split this fix to another PR, please let me know.~~

I excluded this commit. See #1634 (comment)

Checklist

Documentation added

rfratto · 2020-02-05T03:53:11Z

Hi, I can't comment on the rest of this PR but I personally would prefer to keep the docs folder the definitive source for documentation rather than linking to something that's outside of it. One reason for doing this is to make sure that our docs folder and all of its link play nice with our future plans to launch a documentation website.

Could we instead move the fluentd README to docs/ and replace fluentd/fluent-plugin-grafana-loki/README.md with a link to the docs and a small blurb with any minimal info users would need to know?

takanabe · 2020-02-05T04:34:29Z

thank you for the quick review.

One reason for doing this is to make sure that our docs folder and all of its link play nice with our future plans to launch a documentation website.

Sounds great! I don't have strong objections for the static site generations and how Loki team maintains the docs. Let me update based on @rfratto 's suggestion.

takanabe · 2020-02-05T10:02:00Z

Special notes for your reviewer:

I found a tiny bug in docker-compose.yml for this plugin and fix it in 8ed58ac. If I should split this fix to another PR, please let me know.

I think I should split the PR for this purpose. So, I reverted my commit with 316a93d

`doc/client/fluentd.m` includes obsolete explanations and it confuses new users. For example, label_keys option was removed at grafana#1186 but `fluentd.md` has the explanation. It's simple to maitain `fluentd/fluent-plugin-grafana-loki/README.md` for the explantion of the output plugin and use the link when we share how to use it.

`docker-compose up` fails due to the wrong environment variable specifying the fluentd config. As a result this causes the following error ``` docker-compose up --build WARNING: The LOKI_USERNAME variable is not set. Defaulting to a blank string. WARNING: The LOKI_PASSWORD variable is not set. Defaulting to a blank string. Building fluentd Step 1/17 : FROM fluent/fluentd:v1.3.2-debian ---> 4790aaf4d1e5 Step 2/17 : USER root ---> Using cache ---> 7b54da6cec9e Step 3/17 : WORKDIR /home/fluent ---> Using cache ---> a161d2fcf64b Step 4/17 : ENV PATH /fluentd/vendor/bundle/ruby/2.3.0/bin:$PATH ---> Using cache ---> b67529aad0b5 Step 5/17 : ENV GEM_PATH /fluentd/vendor/bundle/ruby/2.3.0 ---> Using cache ---> 3a8b05beb185 Step 6/17 : ENV GEM_HOME /fluentd/vendor/bundle/ruby/2.3.0 ---> Using cache ---> d68a94345cb4 Step 7/17 : ENV FLUENTD_DISABLE_BUNDLER_INJECTION 1 ---> Using cache ---> 70f9bbf646e8 Step 8/17 : COPY docker/Gemfile* /fluentd/ ---> Using cache ---> 5f8b2f6ffe1a Step 9/17 : RUN buildDeps="sudo make gcc g++ libc-dev ruby-dev" && apt-get update && apt-get install -y --no-install-recommends $buildDeps libsystemd0 net-tools libjemalloc1 && gem install bundler --version 1.16.2 && bundle config silence_root_warning true && bundle install --gemfile=/fluentd/Gemfile --path=/fluentd/vendor/bundle && sudo gem sources --clear-all && SUDO_FORCE_REMOVE=yes apt-get purge -y --auto-remove -o APT::AutoRemove::RecommendsImportant=false $buildDeps && rm -rf /var/lib/apt/lists/* /home/fluent/.gem/ruby/2.3.0/cache/*.gem /tmp/* /var/tmp/* /usr/lib/ruby/gems/*/cache/*.gem ---> Using cache ---> 36c960a53c2c Step 10/17 : COPY docker/entrypoint.sh /fluentd/entrypoint.sh ---> Using cache ---> 7f8d72ae63ca Step 11/17 : COPY lib/fluent/plugin/out_loki.rb /fluentd/plugins/out_loki.rb ---> Using cache ---> d09473ee4f25 Step 12/17 : COPY docker/conf/ /fluentd/etc/loki/ ---> Using cache ---> 00d0439ef3c8 Step 13/17 : ENV FLUENTD_CONF="/fluentd/etc/loki/fluentd.conf" ---> Using cache ---> 2d33a002baaf Step 14/17 : ENV FLUENTD_OPT="" ---> Using cache ---> c314df064756 Step 15/17 : ENV LOKI_URL "https://logs-us-west1.grafana.net" ---> Using cache ---> df78c829d416 Step 16/17 : ENV LD_PRELOAD="/usr/lib/x86_64-linux-gnu/libjemalloc.so.1" ---> Using cache ---> 3d7d14cf37f5 Step 17/17 : ENTRYPOINT ["/fluentd/entrypoint.sh"] ---> Using cache ---> 82c70c2354ac Successfully built 82c70c2354ac Successfully tagged fluentd:loki Starting docker_fluentd_1 ... done Attaching to docker_fluentd_1 fluentd_1 | /fluentd/vendor/bundle/ruby/2.3.0/gems/fluentd-1.6.2/lib/fluent/supervisor.rb:769:in `initialize': No such file or directory @ rb_sysopen - /fluentd/etc/loki/fluentd.conf (Errno::ENOENT) fluentd_1 | from /fluentd/vendor/bundle/ruby/2.3.0/gems/fluentd-1.6.2/lib/fluent/supervisor.rb:769:in `open' fluentd_1 | from /fluentd/vendor/bundle/ruby/2.3.0/gems/fluentd-1.6.2/lib/fluent/supervisor.rb:769:in `read_config' fluentd_1 | from /fluentd/vendor/bundle/ruby/2.3.0/gems/fluentd-1.6.2/lib/fluent/supervisor.rb:479:in `run_supervisor' fluentd_1 | from /fluentd/vendor/bundle/ruby/2.3.0/gems/fluentd-1.6.2/lib/fluent/command/fluentd.rb:314:in `<top (required)>' fluentd_1 | from /usr/lib/ruby/2.3.0/rubygems/core_ext/kernel_require.rb:55:in `require' fluentd_1 | from /usr/lib/ruby/2.3.0/rubygems/core_ext/kernel_require.rb:55:in `require' fluentd_1 | from /fluentd/vendor/bundle/ruby/2.3.0/gems/fluentd-1.6.2/bin/fluentd:8:in `<top (required)>' fluentd_1 | from /fluentd/vendor/bundle/ruby/2.3.0/bin/fluentd:22:in `load' fluentd_1 | from /fluentd/vendor/bundle/ruby/2.3.0/bin/fluentd:22:in `<main>' docker_fluentd_1 exited with code 1 ```

…lanation

This reverts commit 8ed58ac97a82d5536321fb9a76120fed766ae506.

codecov-io · 2020-02-05T10:14:49Z

Codecov Report

Merging #1634 into master will decrease coverage by 0.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #1634      +/-   ##
==========================================
- Coverage   61.84%   61.83%   -0.02%     
==========================================
  Files         109      109              
  Lines        8304     8304              
==========================================
- Hits         5136     5135       -1     
- Misses       2774     2775       +1     
  Partials      394      394

Impacted Files	Coverage Δ
pkg/promtail/targets/filetarget.go	`68.71% <0%> (-1.85%)`	⬇️
pkg/promtail/targets/tailer.go	`78.16% <0%> (+2.29%)`	⬆️

rfratto

LGTM from my end, I'd like @cyriltovena to take a look though.

owen-d · 2020-02-06T14:39:07Z

docs/clients/fluentd/README.md

+
+### Multi-worker usage
+
+Loki doesn't currently support out-of-order inserts - if you try to insert a log entry an earlier timestamp after a log entry with with identical labels but a later timestamp, the insert will fail with `HTTP status code: 500, message: rpc error: code = Unknown desc = Entry out of order`. Therefore, in order to use this plugin in a multi worker Fluentd setup, you'll need to include the worker ID in the labels.


Suggested change

Loki doesn't currently support out-of-order inserts - if you try to insert a log entry an earlier timestamp after a log entry with with identical labels but a later timestamp, the insert will fail with `HTTP status code: 500, message: rpc error: code = Unknown desc = Entry out of order`. Therefore, in order to use this plugin in a multi worker Fluentd setup, you'll need to include the worker ID in the labels.

Loki doesn't currently support out-of-order inserts - if you try to insert a log entry an earlier timestamp after a log entry with with identical labels but a later timestamp, the insert will fail with `HTTP status code: 500, message: rpc error: code = Unknown desc = Entry out of order`. Therefore, in order to use this plugin in a multi worker Fluentd setup, you'll need to include the worker ID in the labels or otherwise ensure log streams are always sent to the same worker.

Not sure if this is possible (I haven't used fluentd), but wanted to put in this small disclaimer

What about this 3f20282 ?

owen-d · 2020-02-06T14:49:32Z

docs/clients/fluentd/README.md

+
+## Docker Image
+
+There is a Docker image `grafana/fluent-plugin-grafana-loki:master` which contains default configuration files to git log information


Having difficulty understanding this line. Can you elaborate?

I'm asking about this line because it's not clear for me https://grafana.slack.com/archives/CEPJRLQNL/p1580902179287300

let me try to elaborate with my understanding anyway

♻️ 704c303

owen-d

Looks great, thanks @takanabe!

* 'extraPorts' of github.com:billimek/loki: (25 commits) Ensure status codes are set correctly in the frontend. (grafana#1684) --dry-run Promtail. (grafana#1652) Fix logcli --quiet parameter parsing issue (grafana#1682) This improves the log ouput for statistics in the logcli. (grafana#1644) Loki stack helm chart can deploy datasources without Grafana (grafana#1688) Automatically prune metrics from the /metrics output of the promtail metrics pipeline stage after an idle period. Allow a metric defined as a counter to match all lines, useful for creating line count metrics which include all your labels. Found a bug in the the test runner where it didn't fail if the actual error was nil but the test expected an error Added some tests to the counters to test valid configs maintainer links & usernames (grafana#1675) Binary operators in LogQL (grafana#1662) Pipe data to Promtail (grafana#1649) Add Owen to the maintainer team. (grafana#1673) Update tanka.md so that promtail.yml is in the correct format (grafana#1671) Correcte syntax of rate example (grafana#1641) Frontend & Querier query statistics instrumentation. (grafana#1661) loki-canary: fix indent of DaemonSet manifest written in .md file (grafana#1648) Query frontend service should be headless. (grafana#1665) Support custom prefix name in metrics stage (grafana#1664) pkg/promtail/positions: handle empty positions file (grafana#1660) Convert second(Integer class) to nanosecond precision (grafana#1656) Unite docs for fluentd plugin (grafana#1634) ...

pull-request-size bot added the size/L label Feb 5, 2020

takanabe added 6 commits February 5, 2020 19:06

Remove label_keys option and simplify usage for drop_single_key

d4bac13

Use docs/clients/fluentd/README.md for fluent-plugin-grafana-loki exp…

a780e96

…lanation

Arrange list order based on number

56a926e

Revert "Fix environment variable: FLUENT_CONFIG -> FLUENTD_CONFIG"

16bf9e4

This reverts commit 8ed58ac97a82d5536321fb9a76120fed766ae506.

takanabe force-pushed the unite_docs_for_fluentd_plugin branch from 316a93d to 16bf9e4 Compare February 5, 2020 10:06

takanabe requested a review from woodsaj February 5, 2020 10:19

rfratto approved these changes Feb 5, 2020

View reviewed changes

pull-request-size bot added size/XL and removed size/L labels Feb 6, 2020

owen-d reviewed Feb 6, 2020

View reviewed changes

takanabe added 3 commits February 7, 2020 09:47

Elaborate the way to use docker image

704c303

Fix grammar: remove unnecessary with

7d0b463

Add disclaimer for multi-worker usage

3f20282

takanabe requested a review from owen-d February 7, 2020 03:15

owen-d approved these changes Feb 7, 2020

View reviewed changes

cyriltovena merged commit 973b96d into grafana:master Feb 7, 2020

takanabe deleted the unite_docs_for_fluentd_plugin branch February 7, 2020 15:11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Unite docs for fluentd plugin #1634

Unite docs for fluentd plugin #1634

takanabe commented Feb 5, 2020 •

edited

Loading

rfratto commented Feb 5, 2020

takanabe commented Feb 5, 2020

takanabe commented Feb 5, 2020

codecov-io commented Feb 5, 2020 •

edited

Loading

rfratto left a comment

owen-d Feb 6, 2020

owen-d Feb 6, 2020

takanabe Feb 7, 2020

owen-d Feb 6, 2020

takanabe Feb 6, 2020

takanabe Feb 7, 2020

owen-d left a comment


		### Multi-worker usage

		Loki doesn't currently support out-of-order inserts - if you try to insert a log entry an earlier timestamp after a log entry with with identical labels but a later timestamp, the insert will fail with `HTTP status code: 500, message: rpc error: code = Unknown desc = Entry out of order`. Therefore, in order to use this plugin in a multi worker Fluentd setup, you'll need to include the worker ID in the labels.


		## Docker Image

		There is a Docker image `grafana/fluent-plugin-grafana-loki:master` which contains default configuration files to git log information

Unite docs for fluentd plugin #1634

Unite docs for fluentd plugin #1634

Conversation

takanabe commented Feb 5, 2020 • edited Loading

rfratto commented Feb 5, 2020

takanabe commented Feb 5, 2020

takanabe commented Feb 5, 2020

codecov-io commented Feb 5, 2020 • edited Loading

Codecov Report

rfratto left a comment

Choose a reason for hiding this comment

owen-d Feb 6, 2020

Choose a reason for hiding this comment

owen-d Feb 6, 2020

Choose a reason for hiding this comment

takanabe Feb 7, 2020

Choose a reason for hiding this comment

owen-d Feb 6, 2020

Choose a reason for hiding this comment

takanabe Feb 6, 2020

Choose a reason for hiding this comment

takanabe Feb 7, 2020

Choose a reason for hiding this comment

owen-d left a comment

Choose a reason for hiding this comment

takanabe commented Feb 5, 2020 •

edited

Loading

codecov-io commented Feb 5, 2020 •

edited

Loading