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

in_tail: Add throttling metrics #4578

Merged
merged 5 commits into from
Aug 7, 2024

Conversation

Athishpranav2003
Copy link
Contributor

Which issue(s) this PR fixes:
Fixes #
fluent/fluent-plugin-prometheus#190

What this PR does / why we need it:
It adds metrics to the throttling of logs while taling logs using in_tail plugin as asked in the above issue.
Once this PR is accepted corresponding changes in prometheus plugin will be made to expose the newly added throttling metric
Docs Changes:
Need docs change in prometheus plugin for metrics description
Release Note:
NA

Signed-off-by: Athish Pranav D <[email protected]>
@Athishpranav2003
Copy link
Contributor Author

@ashie Please take a look at this. I am not sure which sort of metric is important here. Once this is merged and released i will add the changes to prometheus plugin too, I will create the PR for it too in some time

Signed-off-by: Athish Pranav D <[email protected]>
@Athishpranav2003
Copy link
Contributor Author

Athishpranav2003 commented Aug 5, 2024

@ashie can you approve the checks?

@Athishpranav2003 Athishpranav2003 changed the title Added throttling metrics Add throttling metrics Aug 5, 2024
@ashie
Copy link
Member

ashie commented Aug 6, 2024

Here is a reply for your comment you seem deleted already:

i cant run the checks locally because its too heavy

Yes, fluentd takes too long time to run whole tests.
e.g.) It takes 1035 seconds in https://github.com/fluent/fluentd/actions/runs/9925766505/job/27418533059

So even we fluentd maintainers also don't often run all tests locally.
Instead, run only partial tests related with the change.

e.g.) Run only in_tail's multiline related tests, and show test names currently running (-v option).

$ bundle exec rake test TEST=test/plugin/test_in_tail.rb TESTOPTS="-v --testcase=\"TailInputTest::multiline\""

You can see the explanation of test options by the following command:

$ bundle exe rake test TESTOPTS="--help"
...
Usage: /home/aho/Projects/Fluentd/fluentd/vendor/bundle/ruby/3.2.0/gems/rake-13.2.1/lib/rake/rake_test_loader.rb [options] [-- untouched arguments]
    -r, --runner=RUNNER              Use the given RUNNER.
                                     (c[onsole], e[macs], x[ml])
        --collector=COLLECTOR        Use the given COLLECTOR.
                                     (de[scendant], di[r], l[oad], o[bject]_space)
    -n, --name=NAME                  Runs tests matching NAME.
                                     Use '/PATTERN/' for NAME to use regular expression.
                                     Regular expression accepts options.
                                     Example: '/taRget/i' matches 'target' and 'TARGET'
        --ignore-name=NAME           Ignores tests matching NAME.
                                     Use '/PATTERN/' for NAME to use regular expression.
                                     Regular expression accepts options.
                                     Example: '/taRget/i' matches 'target' and 'TARGET'
    -t, --testcase=TESTCASE          Runs tests in TestCases matching TESTCASE.
                                     Use '/PATTERN/' for TESTCASE to use regular expression.
                                     Regular expression accepts options.
                                     Example: '/taRget/i' matches 'target' and 'TARGET'
        --ignore-testcase=TESTCASE   Ignores tests in TestCases matching TESTCASE.
                                     Use '/PATTERN/' for TESTCASE to use regular expression.
                                     Regular expression accepts options.
                                     Example: '/taRget/i' matches 'target' and 'TARGET'
        --location=LOCATION          Runs tests that defined in LOCATION.
                                     LOCATION is one of PATH:LINE, PATH or LINE.
        --attribute=EXPRESSION       Runs tests that matches EXPRESSION.
                                     EXPRESSION is evaluated as Ruby's expression.
                                     Test attribute name can be used with no receiver in EXPRESSION.
                                     EXPRESSION examples:
                                       !slow
                                       tag == 'important' and !slow
        --[no-]priority-mode         Runs some tests based on their priority.
        --default-priority=PRIORITY  Uses PRIORITY as default priority
                                     (h[igh], i[mportant], l[ow], m[ust], ne[ver], no[rmal])
    -I, --load-path=DIR[:DIR...]     Appends directory list to $LOAD_PATH.
        --color-scheme=SCHEME        Use SCHEME as color scheme.
                                     (d[efault])
        --config=FILE                Use YAML format FILE content as configuration file.
        --order=ORDER                Run tests in a test case in ORDER order.
                                     (a[lphabetic], d[efined], r[andom])
        --max-diff-target-string-size=SIZE
                                     Shows diff if both expected result string size and actual result string size are less than or equal SIZE in bytes.
                                     (1000)
        --[no-]stop-on-failure       Stops immediately on the first non success test
                                     (false)
        --[no-]debug-on-failure      Run debugger if available on failure
                                     (false)
        --[no-]gc-stress             Enable GC.stress only while each test is running
                                     (false)
    -v, --verbose=[LEVEL]            Set the output level (default is normal).
                                     (i[mportant-only], n[ormal], p[rogress], s[ilent], v[erbose])
        --[no-]use-color=[auto]      Uses color output
                                     (default is auto)
        --progress-row-max=MAX       Uses MAX as max terminal width for progress mark
                                     (default is auto)
        --progress-style=STYLE       Uses STYLE as progress style
                                     (f[ault-only], i[nplace], m[ark]
        --no-show-detail-immediately Shows not passed test details immediately.
                                     (default is yes)
        --[no-]reverse-output        Shows fault details in reverse.
                                     (default is yes for tty output, no otherwise)
        --output-file-descriptor=FD  Outputs to file descriptor FD
        --                           Stop processing options so that the
                                     remaining options will be passed to the
                                     test.
    -h, --help                       Display this help.

Deprecated options:
        --console                    Console runner (use --runner).

Copy link
Member

@ashie ashie left a comment

Choose a reason for hiding this comment

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

Thanks for your work.
It looks good.
I commented on one thing that needs to be fixed.

lib/fluent/plugin/in_tail.rb Outdated Show resolved Hide resolved
Signed-off-by: Athish Pranav D <[email protected]>
@Athishpranav2003
Copy link
Contributor Author

@ashie addressed the comments

It's introduced at fluent#3504 but it's not needed even in the original pull
request because `create_driver` runs `configure` and in_tail's one
initializes metrics.

Signed-off-by: Takuro Ashie <[email protected]>
`throttling_metrics` has been newly added.

Signed-off-by: Takuro Ashie <[email protected]>
Copy link
Member

@ashie ashie left a comment

Choose a reason for hiding this comment

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

LGTM

Although It would be better to add tests for it, we don't have tests for metrics originally. So I don't always require adding them in this pull request. Let's tackle on it in other pull request(s).

I'll wait merging this until tomorrow to see if other maintainers have comments.

test/plugin/test_in_tail.rb Show resolved Hide resolved
test/plugin/test_in_tail.rb Show resolved Hide resolved
@ashie
Copy link
Member

ashie commented Aug 6, 2024

I'll wait merging this until tomorrow to see if other maintainers have comments.

(And waiting CI)

Copy link
Contributor

@cosmo0920 cosmo0920 left a comment

Choose a reason for hiding this comment

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

Looks attractive. Thanks!

@ashie ashie merged commit 5e75248 into fluent:master Aug 7, 2024
16 checks passed
@ashie
Copy link
Member

ashie commented Aug 7, 2024

Thanks!

@daipom daipom added this to the v1.17.1 milestone Aug 15, 2024
@daipom daipom changed the title Add throttling metrics in_tail: Add throttling metrics Aug 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants