Update fluentd to 1.12.3, update json to 2.4.1, bump version to 0.11.0 #447

Cryptophobia · 2021-02-03T23:35:16Z

increment gem version for plugin
update to allow fluentd v1.12.3 gem
update json to 2.4.1 for CVE-2020-10663

Signed-off-by: Anton Ouzounov aouzounov@vmware.com

Hello maintainers! We are using fluentd-plugin-google-cloud as a plugin in the kube-fluentd-operator . We want to be able to migrate to using fluentd v1.12.0 for the benefit of our users, this is the only plugin currently forcing us to use fluentd v1.11.2 . Can we help upgrade? I don't see any breaking changes in the fluentd changelogs that will present problems.

google-cla · 2021-02-03T23:35:19Z

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.

What to do if you already signed the CLA

Individual signers

It's possible we don't have your GitHub username or you're using a different email address on your commit. Check your existing CLA data and verify that your email is set on your git commits.

Corporate signers

Your company has a Point of Contact who decides which employees are authorized to participate. Ask your POC to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the Google project maintainer to go/cla#troubleshoot (Public version).
The email used to register you as an authorized contributor must be the email used for the Git commit. Check your existing CLA data and verify that your email is set on your git commits.
The email used to register you as an authorized contributor must also be attached to your GitHub account.

ℹ️ Googlers: Go here for more info.

Cryptophobia · 2021-02-04T23:17:17Z

@googlebot I signed it!

google-cla · 2021-02-04T23:17:20Z

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.

What to do if you already signed the CLA

Individual signers

It's possible we don't have your GitHub username or you're using a different email address on your commit. Check your existing CLA data and verify that your email is set on your git commits.

Corporate signers

Your company has a Point of Contact who decides which employees are authorized to participate. Ask your POC to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the Google project maintainer to go/cla#troubleshoot (Public version).
The email used to register you as an authorized contributor must be the email used for the Git commit. Check your existing CLA data and verify that your email is set on your git commits.
The email used to register you as an authorized contributor must also be attached to your GitHub account.

ℹ️ Googlers: Go here for more info.

Cryptophobia · 2021-02-04T23:19:19Z

@googlebot I signed it!

Cryptophobia · 2021-02-04T23:22:21Z

@davidbtucker, do you think this is a safe update or does it need more testing?

Cryptophobia · 2021-03-26T14:08:36Z

@qingling128 @ekund , can we relax the runtime dependency fluentd fixed at a particular fluentd version?

qingling128 · 2021-03-29T22:39:42Z

Hi @Cryptophobia - The reason we pin dependency versions is to achieve reproducible builds. Any specific features / bugs you are looking for in Fluentd 1.12? What's the impact scope?

Cryptophobia · 2021-03-30T02:58:43Z

@qingling128 , that is fine and you can version fluentd separately in your build system or when/where you build your containers. There is no reason why your gem, which is a plugin dependency of fluentd, needs to specify a required version for fluentd.

For example, your gem is the only one that does not allow us to upgrade past fluentd v1.11.2 here: https://github.com/vmware/kube-fluentd-operator/blob/master/base-image/Gemfile . We have so many plugins, and this is the only one that constrains us from upgrading to v0.12.0 for absolutely no reason.

All of the other plugin gems have very relaxed constraints such as this one for example fluent-plugin-systemd: spec.add_runtime_dependency 'fluentd', ['>= 0.14.11', '< 2']

Same for fluent-plugin-mongo.gemspec: gem.add_dependency "fluentd", [">= 0.14.22", "< 2"]...and so on...

qingling128 · 2021-03-31T00:04:30Z

@Cryptophobia - Hi, thanks for the suggestion. That is another possibility (aka refactor the build infra to move the version pinning away from gem spec). We'll need to evaluate its impact and get it prioritized. Given the current workload on the team's plate, it might not happen in the short term.

We could consider upgrading Fluentd version though. Note that we have internal integration and load tests that need to pass for each release. Fluentd version turned out to be one of main causes when CPU / memory regression happened in the past. So we need to go through a set of validation process to upgrade the Fluentd version.

Cryptophobia · 2021-03-31T14:44:15Z

@qingling128 , okay, we would appreciate it very much! Hopefully, there is not many breaking changes from fluentd v1.11.x to fluentd v1.12.0. When I looked at the changelogs, I did not notice anything immediately obvious that may be a problem.

https://github.com/fluent/fluentd/blob/master/CHANGELOG.md

cosmo0920 · 2021-04-06T03:17:16Z

We're investigating some breakages on introducing follow_inodes feature on in_tail:
fluent/fluentd#3239

Any feedback is appreciated.

Cryptophobia · 2021-04-06T19:03:58Z

@cosmo0920 , are you hitting that particular issue on 1.12.0 with this particular plugin, have you tried downgrading to 1.11.x?

Alternative to see if 1.12.2 will fix the issue.

cosmo0920 · 2021-04-07T04:09:28Z

@cosmo0920 , are you hitting that particular issue on 1.12.0 with this particular plugin, have you tried downgrading to 1.11.x?

Yeah, downgrading to v1.11.x vanishes this issue.

Alternative to see if 1.12.2 will fix the issue.

We'd like to go Fluentd v1.12 world. Fluentd v1.12.2 will fix almost in_tail issues.
But we're still investigating whether unknown issues still exists or not on users environments.

qingling128 · 2021-04-08T04:28:21Z

Sounds like v1.12.2 is the version we want? Can someone confirm it so that we can start the internal validation process on it?

Cryptophobia · 2021-04-08T14:37:13Z

https://www.fluentd.org/blog/fluentd-v1.12.2-has-been-released

1.12.2 has all of the in_tail fixes but there is still an issue open that @cosmo0920 linked to above:

I'm going to include this gem built from my fork using fluentd v1.12.2 for our KFO project.

Cryptophobia · 2021-04-08T21:40:49Z

I had to change the dependency version pinning to this:

  gem.add_runtime_dependency 'fluentd', '~> 1.12', '>= 1.12.2', '< 1.13'

Cryptophobia · 2021-04-09T21:17:32Z

Also added upgrade of json gem for CVE-2020-10663 to this PR:

Cryptophobia · 2021-04-17T16:47:30Z

Looks like fluentd maintainers will be releasing v1.12.3 with bug fixes for some remaining in_tail issue. fluent/fluentd#3323 (comment)

I will update this PR once that version is released.

Cryptophobia · 2021-04-27T14:35:08Z

fluentd v1.12.3 has been released! can we get this merged in now?

qingling128 · 2021-04-27T16:51:51Z

fluent-plugin-google-cloud.gemspec

@@ -19,14 +19,14 @@ eos
  gem.test_files    = gem.files.grep(/^(test)/)
  gem.require_paths = ['lib']

-  gem.add_runtime_dependency 'fluentd', '1.11.2'
+  gem.add_runtime_dependency 'fluentd', '~> 1.12', '>= 1.12.2', '< 1.13'


Let me kick off an internal validation process for Fluentd 1.12.3. As mentioned in #447 (comment), the version has to pinned to an exact version for now, e.g.

gem.add_runtime_dependency 'fluentd', '1.12.3'

Please update.

Okay, updated the PR to force exact version 1.12.3. Ideally, can we move away from forcing fluentd to particular version. Otherwise, anytime we have to upgrade KFO we will always have to fork this gem.

qingling128 · 2021-04-27T16:52:14Z

fluent-plugin-google-cloud.gemspec

  gem.add_runtime_dependency 'googleapis-common-protos', '1.3.10'
  gem.add_runtime_dependency 'googleauth', '0.9.0'
  gem.add_runtime_dependency 'google-api-client', '0.30.8'
  gem.add_runtime_dependency 'google-cloud-logging', '1.6.6'
  gem.add_runtime_dependency 'google-protobuf', '3.15.8'
  gem.add_runtime_dependency 'grpc', '1.31.1'
-  gem.add_runtime_dependency 'json', '2.2.0'
+  gem.add_runtime_dependency 'json', '~> 2.3.0'


Which exact version of json are we looking for here?

We are looking for any version above 2.3.0 which is not vulnerable. We can try 2.4.1 as that is recommended in the fluentd provided docker images?

Information here: https://github.com/fluent/fluentd-docker-image/blob/master/v1.12/debian/Dockerfile

- increment gem version for plugin - update to allow fluentd v1.12.3 gem - update json to 2.4.1 for [CVE-2020-10663](https://www.ruby-lang.org/en/news/2020/03/19/json-dos-cve-2020-10663/) Signed-off-by: Anton Ouzounov <aouzounov@vmware.com>

Cryptophobia · 2021-04-27T17:22:52Z

Updated @qingling128 .

qingling128 · 2021-04-27T20:54:32Z

Updated @qingling128 .

Thanks! Our internal validation process takes a few days to soak to catch any potential memory leaks etc. We should have some result on Monday.

qingling128 · 2021-04-28T01:41:37Z

Initial numbers look good.

CPU

RSS

Log entry ingestion rate

Merging this PR so that the internal 4-day validation process can pick it up tonight.

qingling128

LGTM

Cryptophobia · 2021-04-28T14:46:39Z

Thank you @qingling128 ! Numbers indeed look stable. Will make other PRs in the future when we need to update fluentd version!

qingling128 · 2021-05-14T03:06:28Z

The release was delayed for a bit because we saw some resource usage regression (turned out to be false alarm). The May 12 release candidate is going through the 4-day validation. If everything passes, the release will happen early next week.

qingling128 · 2021-05-21T13:31:40Z

Actually, it turned out there is still a memory regression, and related to this PR. Currently investigating the issue and try to figure out whether it's due to fluentd or json gem version update.

qingling128 · 2021-05-24T16:47:54Z

The issue was caused by the Fluentd version bump:

The memory and CPU usage of Fluentd 1.12.3 is higher than 1.11.2, resulting in the logging agent not being able to catch up with 3000 log entries / second throughput.

Fixes #447 (comment)

Fixes GoogleCloudPlatform/fluent-plugin-google-cloud#447 (comment)

Cryptophobia · 2021-05-24T17:17:54Z

@qingling128 , are we going to report this to https://github.com/fluent/fluentd/ repo?

Are the other basegem dependencies versions equivalent to these: https://github.com/vmware/kube-fluentd-operator/blob/master/base-image/basegems/Gemfile ? It is possible that it could be a ruby version issue or another base gem that needs to be upgraded for better memory optimization?

qingling128 · 2021-05-24T19:07:12Z

Filed an upstream bug at fluent/fluentd#3389. The ruby version, dep gems are the same. See details in the upstream bug.

Cryptophobia · 2021-05-25T14:20:35Z

Looks like fluentd v1.13.0 with a few in_tail fixes will release soon. fluent/fluentd#3333 (comment) . Maybe they will get to looking at the memory/cpu issues. We should try to update to v1.13.0 when it goes out and test again.

cosmo0920 · 2021-05-25T23:30:54Z

Looks like fluentd v1.13.0 with a few in_tail fixes will release soon. fluent/fluentd#3333 (comment) . Maybe they will get to looking at the memory/cpu issues.

We're also planning to fix in_tail fixes on Fluentd v1.12.4.

google-cla bot added the cla: no label Feb 3, 2021

google-cla bot added cla: yes and removed cla: no labels Feb 4, 2021

cosmo0920 mentioned this pull request Apr 6, 2021

Remove fluent-plugin-google-cloud fluent/fluentd-kubernetes-daemonset#553

Closed

Cryptophobia changed the title ~~chore: update to fluentd v1.12.0~~ chore: update to at least fluentd v1.12.2 Apr 8, 2021

Cryptophobia force-pushed the master branch from fd7dadb to fd13d61 Compare April 8, 2021 14:34

Cryptophobia force-pushed the master branch 2 times, most recently from 9953de0 to e2c1136 Compare April 8, 2021 21:29

Cryptophobia force-pushed the master branch from e2c1136 to 6cbee80 Compare April 9, 2021 21:15

Cryptophobia force-pushed the master branch from 6cbee80 to f76e8d7 Compare April 9, 2021 21:18

Cryptophobia force-pushed the master branch 2 times, most recently from 3acc8f4 to c25d4ab Compare April 27, 2021 14:34

qingling128 requested changes Apr 27, 2021

View reviewed changes

Cryptophobia force-pushed the master branch from c25d4ab to f2df2d3 Compare April 27, 2021 17:14

chore: update to fluentd v1.12.3

9521330

- increment gem version for plugin - update to allow fluentd v1.12.3 gem - update json to 2.4.1 for [CVE-2020-10663](https://www.ruby-lang.org/en/news/2020/03/19/json-dos-cve-2020-10663/) Signed-off-by: Anton Ouzounov <aouzounov@vmware.com>

Cryptophobia force-pushed the master branch from f2df2d3 to 9521330 Compare April 27, 2021 17:17

Cryptophobia changed the title ~~chore: update to at least fluentd v1.12.2~~ chore: update to at least fluentd v1.12.3 Apr 27, 2021

Cryptophobia changed the title ~~chore: update to at least fluentd v1.12.3~~ chore: update to fluentd v1.12.3 Apr 27, 2021

qingling128 self-assigned this Apr 27, 2021

qingling128 reviewed Apr 28, 2021

View reviewed changes

qingling128 approved these changes Apr 28, 2021

View reviewed changes

qingling128 merged commit 595f555 into GoogleCloudPlatform:master Apr 28, 2021

qingling128 changed the title ~~chore: update to fluentd v1.12.3~~ Update fluentd to 1.12.3, update json to 2.4.1, bump version to 0.11.0 Apr 28, 2021

qingling128 added a commit that referenced this pull request May 24, 2021

Revert Fluentd from 1.12.3 to 1.11.2 for memory / cpu regression.

c1ef896

Fixes #447 (comment)

qingling128 mentioned this pull request May 24, 2021

Revert Fluentd from 1.12.3 to 1.11.2 for memory / cpu regression. #461

Merged

qingling128 added a commit to GoogleCloudPlatform/google-fluentd that referenced this pull request May 24, 2021

Revert Fluentd from 1.12.3 to 1.11.2 for memory / cpu regression.

93232bd

Fixes GoogleCloudPlatform/fluent-plugin-google-cloud#447 (comment)

qingling128 mentioned this pull request May 24, 2021

Revert Fluentd from 1.12.3 to 1.11.2 for memory / cpu regression. GoogleCloudPlatform/google-fluentd#340

Merged

ashie mentioned this pull request May 24, 2021

Logging from single k8s node stops and Fluentd cpu -> 100%. Log events lost. fluent/fluentd#3382

Closed

qingling128 mentioned this pull request Sep 14, 2021

chore(gemspec): update fluentd >= 1.14.x #475

Open

Update fluentd to 1.12.3, update json to 2.4.1, bump version to 0.11.0 #447

Update fluentd to 1.12.3, update json to 2.4.1, bump version to 0.11.0 #447

Conversation

Cryptophobia commented Feb 3, 2021 • edited

google-cla bot commented Feb 3, 2021

What to do if you already signed the CLA

Individual signers

Corporate signers

Cryptophobia commented Feb 4, 2021

google-cla bot commented Feb 4, 2021

What to do if you already signed the CLA

Individual signers

Corporate signers

Cryptophobia commented Feb 4, 2021

Cryptophobia commented Feb 4, 2021

Cryptophobia commented Mar 26, 2021

qingling128 commented Mar 29, 2021

Cryptophobia commented Mar 30, 2021 • edited

qingling128 commented Mar 31, 2021

Cryptophobia commented Mar 31, 2021

cosmo0920 commented Apr 6, 2021

Cryptophobia commented Apr 6, 2021

cosmo0920 commented Apr 7, 2021

qingling128 commented Apr 8, 2021

Cryptophobia commented Apr 8, 2021

Cryptophobia commented Apr 8, 2021

Cryptophobia commented Apr 9, 2021

Cryptophobia commented Apr 17, 2021

Cryptophobia commented Apr 27, 2021

qingling128 Apr 27, 2021

Choose a reason for hiding this comment

Cryptophobia Apr 27, 2021

Choose a reason for hiding this comment

qingling128 Apr 27, 2021

Choose a reason for hiding this comment

Cryptophobia Apr 27, 2021

Choose a reason for hiding this comment

Cryptophobia Apr 27, 2021

Choose a reason for hiding this comment

Cryptophobia commented Apr 27, 2021

qingling128 commented Apr 27, 2021

qingling128 commented Apr 28, 2021

qingling128 left a comment

Choose a reason for hiding this comment

Cryptophobia commented Apr 28, 2021

qingling128 commented May 14, 2021

qingling128 commented May 21, 2021

qingling128 commented May 24, 2021

Cryptophobia commented May 24, 2021

qingling128 commented May 24, 2021

Cryptophobia commented May 25, 2021

cosmo0920 commented May 25, 2021

Cryptophobia commented Feb 3, 2021 •

edited

Cryptophobia commented Mar 30, 2021 •

edited