Exporter should use gzip compression by default

[johnnyshields]

Gzip should be the standard for all tracing. 99.9% of practical application use cases would want it; it should only be disabled in very specific scenarios (I'm not even sure what they would be).

[arielvalentin]

Thanks for this contribution.

Based on my understanding of the specification there should not be a default value for compression, which is why we have left it unset by default.

Am I misinterpreting this?

Supported values for OTEL_EXPORTER_OTLP_*COMPRESSION options:

If the value is missing, then compression is disabled.
gzip is the only specified compression method for now. Other options MAY be supported by language SDKs and should be documented for each particular language.

https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/protocol/exporter.md#configuration-options

[johnnyshields]

OK, I will raise a PR to change the spec. It is ridiculous that gzip is not the default.

[johnnyshields]

PR raised: open-telemetry/opentelemetry-specification#1895

[fbogsany]

It looks like we're going with open-telemetry/opentelemetry-specification#1923 which allows SDKs to decide the default (valid choices are none and gzip). We can go ahead with this PR when the spec PR is approved & merged.

[johnnyshields]

@fbogsany open-telemetry/opentelemetry-specification#1923 has been merged. Let's now merge this PR.

[fbogsany]

We have had reports of Zlib::DataError: data error when using gzip with the OTLP exporter. This is likely related to a race condition in MRI with zlib, similar to fluent/fluentd#1903. Before we make this the default, I think we should "fix" this issue by handling the error and retrying.

[johnnyshields]

@fbogsany thanks. I'm also seeing the same error on about 1 out of every 1,000,000 requests. Let's fix it.

[johnnyshields]

@fbogsany it looks like the gzip error was fixed in #983. Are we good to merge now?

[fbogsany]

There's a CI failure - this expects a default of no compression:

opentelemetry-ruby/exporter/otlp/test/opentelemetry/exporter/otlp/exporter_test.rb

Line 20 in 61b84ae

_(exp.instance_variable_get(:@compression)).must_be_nil

Spec change makes it reasonable to proceed