search

DataDog / fluent-plugin-datadog

Fluentd output plugin for Datadog: https://www.datadog.com
Apache License 2.0
39 stars 26 forks source link

Migrate to use v1 API #21

Closed cosmo0920 closed 4 years ago

cosmo0920 commented 5 years ago

Fixes #19

What does this PR do?

Use the brand new Fluentd v1 API.

Motivation

I want to use Fluentd v1 API in this plugin.

Additional Notes

Fluentd v1 API does not have backward compatibility. If you merge this patch, users must upgrade their Fluentd. (From v0.12 to 0.14/v1)

Fluentd v1 has plugin helper layer which provides some encapsulated utility methods.

In Fluentd v2 (future release), v0.12 API, which is traditional Fluentd API, not be supported.

okkez commented 4 years ago

@FlorianVeaux Is there any chance to merge this PR?

cosmo0920 commented 4 years ago

@remeh ping?

moserke commented 4 years ago

Would love to see this merged so that https://docs.fluentd.org/plugin-development/api-plugin-output#extract_placeholders-str-chunk can be used for config values.

For example:

if @dd_source
  record["ddsource"] ||= @dd_source
end

becomes

if @dd_source
  record["ddsource"] ||= extract_placeholders(@dd_source, chunk)
end
remeh commented 4 years ago

Hey 👋 Thanks for the contribution @cosmo0920! I'll look into that in the coming days. As this is a breaking change for people still using the 0.x API, we will probably do a major bump of the plugin. I'll keep you posted when I have the time to move forward 👍

cosmo0920 commented 4 years ago

Wow. I'm looking for your reviewing. Note that Fluentd v0.12 series is already EOL. see:. https://www.fluentd.org/blog/drop-schedule-announcement-in-2019 We can just consider plugin compatibility for Fluentd v1 with major version bumping of this plugin.

gaetan-deputier commented 4 years ago

Hey, thanks for the contribution @cosmo0920 . I've included some of your changes in our next forwarder version here: https://github.com/DataDog/fluent-plugin-datadog/pull/34/files if you'd like to review

cosmo0920 commented 4 years ago

@gaetan-deputier I've added suggested comments. PTAL!

cosmo0920 commented 4 years ago

Migrating v1 process should be in #34. Closing this.