fluent / fluent-plugin-prometheus

A fluent plugin that collects metrics and exposes for Prometheus.
Apache License 2.0
257 stars 79 forks source link

Build own record transformer #129

Closed ganmacs closed 4 years ago

ganmacs commented 4 years ago

fix https://github.com/fluent/fluent-plugin-prometheus/issues/125, https://github.com/fluent/fluent-plugin-prometheus/issues/84

ganmacs commented 4 years ago

@repeatedly ?

repeatedly commented 4 years ago

record_modifier uses pre-eval based implementation and it is faster: https://github.com/repeatedly/fluent-plugin-record-modifier/blob/dc86ba87d1484b948d25dd792077244eb35ba8bf/lib/fluent/plugin/filter_record_modifier.rb#L164 Can we use this for prometheus?

ganmacs commented 4 years ago

I don't think so. This class which makes users be able to use ruby in ${} seems to be too much for this feature. also, if we use this, ${hostname} should be changed to ${@hostname}, right? (or another way?). It's breaking change we want to avoid.

repeatedly commented 4 years ago

${hostname} should be changed to ${@hostname}, right?

Add hostname method to expander resolves it. But if support existing features is hard with pre-eval approach, this implementation is good.

ganmacs commented 4 years ago

Can I merge this? @repeatedly @kazegusuri

repeatedly commented 4 years ago

@kazegusuri how about this change?

kazegusuri commented 4 years ago

ah sorry. I think it's okay to keep the compatibility and make it easy. LGTM to merge this.

btw I'm wondering long time that fluentd officially provides the way to expand placeholder by messages to 3rd party plugins or not. Otherwise every plugins coping this implementation or implement their own exapander with a different UI.

ganmacs commented 4 years ago

btw I'm wondering long time that fluentd officially provides the way to expand placeholder by messages to 3rd party plugins or not. Otherwise every plugins coping this implementation or implement their own exapander with a different UI.

Right. Currently, fluentd doesn't provide a consistent way to expand placeholders for 3rd party plugins. It'd be good to do it.

I raised the issue. https://github.com/fluent/fluentd/issues/2800