Skip to content

Conversation

Watson1978
Copy link
Contributor

@Watson1978 Watson1978 commented May 27, 2025

Which issue(s) this PR fixes:
Fixes #

What this PR does / why we need it:
In multi_output, two metrics are created with emit_records.

@emit_count_metrics = metrics_create(namespace: "fluentd", subsystem: "multi_output", name: "emit_records", help_text: "Number of count emits")
@emit_records_metrics = metrics_create(namespace: "fluentd", subsystem: "multi_output", name: "emit_records", help_text: "Number of emit records")

Since it is not possible to distinguish between them, this PR will fix the name.

Docs Changes:
Not needed.

Release Note:
The same as the title.

Signed-off-by: Shizuo Fujita <fujita@clear-code.com>
@Watson1978 Watson1978 marked this pull request as ready for review May 27, 2025 06:55
@Watson1978 Watson1978 requested a review from daipom May 27, 2025 06:56
Copy link
Contributor

@daipom daipom left a comment

Choose a reason for hiding this comment

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

Thanks!

@cosmo0920
If you know anything, could you please let me know?
Could this name change potentially affect users?
I don't know much about metrics...
As for Fluentd's internal logic, it does not seem to be affected.

@cosmo0920
Copy link
Contributor

It is no relevant issues for now. Thanks for the good catch. 👍

@daipom
Copy link
Contributor

daipom commented May 27, 2025

Thanks!

@daipom daipom added this to the v1.19.0 milestone May 27, 2025
@daipom daipom merged commit cc02dec into fluent:master May 27, 2025
13 checks passed
@cosmo0920
Copy link
Contributor

cosmo0920 commented May 27, 2025

The local metrics plugin should not handle the name of metrics. This is only for affected for metrics_cmetrics plugin which was intended to handle the internal metrics in Fluentd with cmetrics mechanism.
https://github.com/chronosphereio/calyptia-fluent-plugin-metrics-cmetrics

Are y'all interested in handling metrics with the current cmetrics?
This is because we had changed the API especially for msgpack encoding/decoding. So, if the using cmetrics library upgraded, we need to publish another gem like as cmetrics2 in rubygems due to incompatibilities for older version of Fluent Bit.
https://rubygems.org/gems/cmetrics

This should be using the old version of cmetrics which was in older fluent-bit.

For now, I have no idea to fix the incompatibilities and how to handle newer version of cmetrics msgpack payloads and the older version of payloads in the same C extension context.

Currently, cmetrics had been reached v1.0.0 milestone. So, it's suitable time to upgrade the cmetrics ruby binding here:
https://github.com/chronosphereio/calyptia-cmetrics-ruby

@Watson1978 Watson1978 deleted the multi_output branch May 27, 2025 07:45
@daipom
Copy link
Contributor

daipom commented May 28, 2025

Thanks!
For now, I just wanted to know the impact of changing a metric name.
I would be happy to improve Fluentd metrics and observability, but I don't have much time to focus on it for a while...
(By the way, @Watson1978 made https://github.com/fluent-plugins-nursery/fluent-plugin-opentelemetry, so Fluentd supports OTel at last.)

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.

3 participants