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

output: Fix handling of metrics in output processor #8848

Merged
merged 2 commits into from
May 23, 2024

Conversation

tchrono
Copy link
Contributor

@tchrono tchrono commented May 21, 2024

This patch passes an cmt_out_context pointer so that output processors can modify metrics.

Without this change, it is not possible to modify metrics in an output processor. Example config which reproduces:

service:
    flush: 1
    log_level: info

pipeline:
    inputs:
        - name: event_type
          tag: event
          type: metrics

    outputs:
        - name: stdout
          match: event
          processors:
            metrics:
              - name: metrics_selector
                metric_name: /kubernetes/
                action: include

Without the patch, we observe unmodified metrics:

2024-05-21T17:51:30.526411118Z kubernetes_network_load_counter = 3
2024-05-21T17:51:30.526411118Z kubernetes_network_load_counter{hostname="localhost",app="cmetrics"} = 1
2024-05-21T17:51:30.526411118Z kubernetes_network_load_counter{hostname="localhost",app="test"} = 12.15
2024-05-21T17:51:30.526411118Z kubernetes_network_load_gauge = 1
2024-05-21T17:51:30.526411118Z k8s_disk_load_summary = { quantiles = { 0.1=1.1, 0.2=2.2, 0.3=3.3, 0.4=4.4, 0.5=5.5 }, sum=51.6129, count=10 }
2024-05-21T17:51:30.526411118Z k8s_disk_load_summary{my_label="my_val"} = { quantiles = { 0.1=11.11, 0.2=0, 0.3=33.33, 0.4=44.44, 0.5=55.55 }, sum=51.6129, count=10 }
2024-05-21T17:51:30.526411118Z k8s_network_load_histogram = { buckets = { 0.05=2, 5=3, 10=4, +Inf=5 }, sum=1013.02, count=5 }
2024-05-21T17:51:30.526411118Z k8s_network_load_histogram{my_label="my_val"} = { buckets = { 0.05=2, 5=3, 10=4, +Inf=5 }, sum=1013.02, count=5 }

With the patch, we see the filtered metrics:

2024-05-21T17:52:29.525856283Z kubernetes_network_load_counter = 3
2024-05-21T17:52:29.525856283Z kubernetes_network_load_counter{hostname="localhost",app="cmetrics"} = 1
2024-05-21T17:52:29.525856283Z kubernetes_network_load_counter{hostname="localhost",app="test"} = 12.15
2024-05-21T17:52:29.525856283Z kubernetes_network_load_gauge = 1

valgrind output:

==117643== Memcheck, a memory error detector
==117643== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==117643== Using Valgrind-3.16.1 and LibVEX; rerun with -h for copyright info
==117643== Command: ./build/bin/fluent-bit -c metrics-processor.yaml
==117643== Parent PID: 12681
==117643== 
==117643== 
==117643== HEAP SUMMARY:
==117643==     in use at exit: 0 bytes in 0 blocks
==117643==   total heap usage: 4,189 allocs, 4,189 frees, 2,700,236 bytes allocated
==117643== 
==117643== All heap blocks were freed -- no leaks are possible
==117643== 
==117643== For lists of detected and suppressed errors, rerun with: -s
==117643== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)

Enter [N/A] in the box, if an item is not applicable to your change.

Testing
Before we can approve your change; please submit the following in a comment:

  • Example configuration file for the change
  • Debug log output from testing the change
  • Attached Valgrind output that shows no leaks or memory corruption was found

If this is a change to packaging of containers or native binaries then please confirm it works for all targets.

  • [N/A] Run local packaging test showing all targets (including any new ones) build.
  • [N/A] Set ok-package-test label to test for all targets (requires maintainer to do).

Documentation

  • [N/A] Documentation required for this feature

Backporting

  • [N/A] Backport to latest stable release.

Fluent Bit is licensed under Apache 2.0, by submitting this pull request I understand that this code will be released under the terms of that license.

This patch passes an cmt_out_context pointer so that output processors
can modify metrics.

Signed-off-by: Thiago Padilha <thiago@chronosphere.io>
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 good. And I'd overlooked for output side of processor functionality.
Thanks for tackling this.
Is there any chance to add unit tests for output patterns like as following file?
https://github.com/fluent/fluent-bit/blob/master/tests/runtime/processor_metrics_selector.c

@tchrono tchrono force-pushed the fix-metrics-output-processing branch from f933746 to 73b276c Compare May 22, 2024 12:26
@tchrono
Copy link
Contributor Author

tchrono commented May 22, 2024

@cosmo0920 added a runtime test that attaches the processor to output.

@edsiper
Copy link
Member

edsiper commented May 22, 2024

@tchrono @tarruda

Please adjust the commit message Add runtime test for output modifying metrics, it needs to be prefixed with the interface, e.g:

  • tests: runtime: processor_metrics_selector: ....

@cosmo0920
Copy link
Contributor

The added commit is OK. But it needs to fix commit message.

@tchrono tchrono force-pushed the fix-metrics-output-processing branch from 73b276c to b275782 Compare May 22, 2024 16:01
…ut processor

Signed-off-by: Thiago Padilha <thiago@chronosphere.io>
@tchrono tchrono force-pushed the fix-metrics-output-processing branch from b275782 to a7082d7 Compare May 22, 2024 16:50
@tchrono
Copy link
Contributor Author

tchrono commented May 23, 2024

@edsiper @cosmo0920 fixed commit message

@cosmo0920
Copy link
Contributor

I approved for the workflows. Waiting for the results.

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.

Thanks for tackling this. Looks good to me.

@edsiper edsiper added this to the Fluent Bit v3.0.5 milestone May 23, 2024
@edsiper edsiper merged commit cfdd0ac into fluent:master May 23, 2024
44 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants