Add compress_batches feature #1

benwh · 2020-05-17T23:06:35Z

This is best reviewed commit-by-commit, as there's quite a lot of noise in the diff due to Rubocop being introduced and auto-corrections applied.

f8d0755: Set a default ruby version

d5b9813: Remove deprecated has_rdoc option

This has been deprecated since 2011

d0e017e: Add rubocop configuration

3351fab: Add rubocop fixups and configuration

Use the rubocop generated configuration to ignore any major changes that
can't be auto-corrected.

ad03786: Adjust supported Ruby versions

Ruby 2.4 is now end-of-life. As a result of this, the google-protobuf
gem is no longer releasing artifacts compatible with this version.
(protocolbuffers/protobuf#7453)

In CI we encounter this error from bundle install:

google-protobuf-3.12.0-x86_64-linux requires ruby version >= 2.5, which is
incompatible with the current version, ruby 2.4.6p354

Therefore remove Ruby 2.4.6 as a tested version in Travis CI.

Additionally remove the constraint on patch versions in the Travis
config, so that we'll use the latest patch version available for each
release branch.

265e819: Add Prometheus metrics

Augment the existing operations with Prometheus metrics in order to
provide observability around the operations that the plugin is
performing.

Introduce a new metrics helper to prevent attempting to register the
same metric more than once in a multi-threaded or multi-instance
context.

44b4b87: Add compress_batches feature

As per the README updates, this can be used to compress a number of
input records into a single Pub/Sub message, therefore saving on costs.

benwh · 2020-05-17T23:24:41Z

Closing and reopening to kick Travis into action

This has been deprecated [since 2011][0] [0]: https://github.com/rubygems/rubygems/blob/1aebd7ddd69c9a38aa9daa3aa89f396d59c9e2a4/History.txt#L3296-L3297

Use the rubocop generated configuration to ignore any major changes that can't be auto-corrected.

Ruby 2.4 is now end-of-life. As a result of this, the `google-protobuf` gem is no longer releasing artifacts compatible with this version. (protocolbuffers/protobuf#7453) In CI we encounter this error from `bundle install`: ``` google-protobuf-3.12.0-x86_64-linux requires ruby version >= 2.5, which is incompatible with the current version, ruby 2.4.6p354 ``` Therefore remove Ruby 2.4.6 as a tested version in Travis CI. Additionally remove the constraint on patch versions in the Travis config, so that we'll use the latest patch version available for each release branch.

benwh · 2020-05-18T12:11:02Z

Note that in this PR I chose to use zlib, as it comes with the Ruby standard library and seems to do a pretty reasonable job (~85% compression on our lab logs).

Maybe we'd like to investigate alternatives, like lz4, but I'd propose doing this in a follow-up, which will be possible by specifying a different value for the compression_algorithm message attribute.

dyson · 2020-05-19T10:35:15Z

lib/fluent/plugin/gcloud_pubsub/client.rb

        @pubsub = Google::Cloud::Pubsub.new project_id: project, credentials: key
        @autocreate_topic = autocreate_topic
        @topics = {}
+
+        @compression_ratio =


I don't think this is capturing the compression ratio. See comment where this is calculated.

dyson · 2020-05-19T10:37:47Z

lib/fluent/plugin/gcloud_pubsub/client.rb

+        )
+
+        compressed_size = compressed_messages.bytesize
+        @compression_ratio.observe(


This to me looks like it is calculating the data-rate saving and not the ratio, which is what we want to do to ensue we can put this in known buckets of 0-1.

compression ratio = (original size)/(compressed size)
data-rate saving = 1 - (compressed size)/(uncompressed size)

Ah yeah you're totally right I think. I was a bit torn between producing a number which would make sense on a dashboard, Prometheus' best-practice definition of a percentage/ratio, and avoiding potential division by zero; and ended up with something slightly confusing here.

I think you're right that we probably do want to keep this formula, as if we computed the actual compression ratio then the value of that is harder to predict (e.g. 10MiB/1KiB = 1024) and therefore define buckets for.

So for what we should replace it with:

I'd argue that we're not talking about 'data rate' here, as we're operating on a single blob rather than an indefinite stream, so it should be expressed as 'space savings' right?

Current:
fluentd_output_gcloud_pubsub_messages_compression_ratio

Variable name: @compression_ratio

Proposal 1:
fluentd_output_gcloud_pubsub_messages_compression_space_savings_ratio

Variable name: @compression_space_saving

I'd like to keep compression in the name as it groups it with the compression_duration_seconds metric.

Proposal 2:
fluentd_output_gcloud_pubsub_messages_compressed_size_per_original_size_ratio

Variable name: @compression_ratio ??

This is closer to the Prometheus suggestion, but feels a little bit awkward?

That naming of Proposal 2 is really doing it for me 😆

Lets do that!

dyson · 2020-05-19T16:08:43Z

lib/fluent/plugin/gcloud_pubsub/metrics.rb

@@ -0,0 +1,14 @@
+# frozen_string_literal: true


Something super weird has happened to your commits here. This file already exists with this content in it plus more. 🤔

Ah ignore me. It's Github screwing up the ordering of the commits.

Ah I think this is probably GitHub doing its usual trick of showing commits in date order rather than actual commit order (as I moved some commits around in the history when rebasing)

dyson

Just the one comment on the naming of the compression ratio metrics but otherwise this looks super solid! Primo work dude.

If you get back to me about that I can smash approve button.

Augment the existing operations with Prometheus metrics in order to provide observability around the operations that the plugin is performing. Introduce a new metrics helper to prevent attempting to register the same metric more than once in a multi-threaded or multi-instance context.

As per the README updates, this can be used to compress a number of input records into a single Pub/Sub message, therefore saving on costs.

benwh changed the base branch from master to gocardless May 17, 2020 23:07

benwh force-pushed the compressed-batching branch 2 times, most recently from e5ddf42 to e0c92bd Compare May 17, 2020 23:21

benwh closed this May 17, 2020

benwh reopened this May 17, 2020

benwh added 4 commits May 18, 2020 00:35

Set a default ruby version

f8d0755

Remove deprecated has_rdoc option

d5b9813

This has been deprecated [since 2011][0] [0]: https://github.com/rubygems/rubygems/blob/1aebd7ddd69c9a38aa9daa3aa89f396d59c9e2a4/History.txt#L3296-L3297

Add rubocop configuration

d0e017e

Add rubocop fixups and configuration

3351fab

Use the rubocop generated configuration to ignore any major changes that can't be auto-corrected.

benwh force-pushed the compressed-batching branch from 9030e23 to ad15de0 Compare May 17, 2020 23:36

benwh force-pushed the compressed-batching branch from ad15de0 to c14ce6f Compare May 18, 2020 11:07

dyson reviewed May 19, 2020

View reviewed changes

dyson suggested changes May 19, 2020

View reviewed changes

benwh force-pushed the compressed-batching branch 2 times, most recently from 6073941 to 8d7e0bc Compare May 19, 2020 18:04

dyson approved these changes May 19, 2020

View reviewed changes

benwh added 2 commits May 19, 2020 19:40

Add compress_batches feature

44b4b87

As per the README updates, this can be used to compress a number of input records into a single Pub/Sub message, therefore saving on costs.

Include gocardless branch in Travis config

96f8528

benwh force-pushed the compressed-batching branch from 8d7e0bc to 96f8528 Compare May 19, 2020 18:40

benwh merged commit d310fbb into gocardless May 19, 2020

benwh deleted the compressed-batching branch May 19, 2020 18:46

benwh mentioned this pull request Jun 1, 2020

Support for compressed batches of messages mia-0032/fluent-plugin-gcloud-pubsub-custom#63

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add compress_batches feature #1

Add compress_batches feature #1

benwh commented May 17, 2020 •

edited

Loading

benwh commented May 17, 2020

benwh commented May 18, 2020

dyson May 19, 2020

dyson May 19, 2020

benwh May 19, 2020

dyson May 19, 2020

dyson May 19, 2020

dyson May 19, 2020

benwh May 19, 2020

dyson left a comment

Add compress_batches feature #1

Add compress_batches feature #1

Conversation

benwh commented May 17, 2020 • edited Loading

benwh commented May 17, 2020

benwh commented May 18, 2020

dyson May 19, 2020

Choose a reason for hiding this comment

dyson May 19, 2020

Choose a reason for hiding this comment

benwh May 19, 2020

Choose a reason for hiding this comment

dyson May 19, 2020

Choose a reason for hiding this comment

dyson May 19, 2020

Choose a reason for hiding this comment

dyson May 19, 2020

Choose a reason for hiding this comment

benwh May 19, 2020

Choose a reason for hiding this comment

dyson left a comment

Choose a reason for hiding this comment

benwh commented May 17, 2020 •

edited

Loading