[Issue #1107] Add gzip compression support for OTLP exporter

[wilguo]

Description

This PR adds gzip compression to OTLP exporter, closing #1107.

The changes have been made following these specifications:
https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/protocol/exporter.md#opentelemetry-protocol-exporter

Fixes #1107

Type of change

• New feature (non-breaking change which adds functionality)

How Has This Been Tested?

Ran tests for OTLP Exporter
tox -e py38-test-exporter-otlp

Checklist:

• Followed the style guidelines of this project
• Changelogs have been updated

[codeboten]

The change looks good to me, has this been tested to work with the Otel collector?

[wilguo]

No I don't think so, I only ran the tests inside the OTEL exporter. Do those tests include tests for the Otel collector?

[aabmass]

The spec also mentions OTEL_EXPORTER_OTLP_COMPRESSION environment variable which we should support here. I'm not sure what values it is supposed to take on though, do you think you could raise an issue in the spec repo to get some clarification? Or @codeboten might know?

[aabmass]

@wilguo did you look into the environment variable option?

[wilguo]

I have just opened an issue on the spec regarding the environment variable
open-telemetry/opentelemetry-specification#1001

[codeboten]

@wilguo gzip is the only value that this env config can be set to. did you want to tackle the work to support that environment variable in a separate PR? The work to support env variables in the otlp exporter has already started here: #1101

[wilguo]

Oh sorry, I didn't see this comment in time. I just added the support for environment variable for gzip compression just now and pushed my changes. Should I roll back my changes or keep it in this PR?

[codeboten]

No worries! I think having it in here is fine, it didn't look like the other PR addressed the compression variable. Would be good to remain consistent and use the global config as in the other PR though:

Configuration().EXPORTER_OTLP_INSECURE

[codeboten]

@codeboten Sorry, I've been on a crunch for another project but I will definitely make progress today!

No worries @wilguo, thanks for getting back to me!

[wilguo]

Sorry guys, I was still not able to find a solution for testing the compression. I changed the test to now check the channel variable that's created when calling insecure_channel() in the exporter constructor since I realized self._client doesn't store a insecure_channel or secure_channel object, it actually stores a <opentelemetry.proto.collector.trace.v1.trace_service_pb2_grpc.TraceServiceStub object at 0x1121db0d0> object. Right now, in the test case I am trying to do assertEquals(channelObj I'm Expecting, self.channel) but since the channel objects have different references, I'm getting an AssertionError: <grpc._channel.Channel object at 0x111911640> != <grpc._channel.Channel object at 0x11190c460>. Do you guys know if there is a way to compare the values stored inside the channel object instead of the references or perhaps get access to the compression attribute of the channel object?

[codeboten]

Sorry guys, I was still not able to find a solution for testing the compression. I changed the test to now check the channel variable that's created when calling insecure_channel() in the exporter constructor since I realized self._client doesn't store a insecure_channel or secure_channel object, it actually stores a <opentelemetry.proto.collector.trace.v1.trace_service_pb2_grpc.TraceServiceStub object at 0x1121db0d0> object. Right now, in the test case I am trying to do assertEquals(channelObj I'm Expecting, self.channel) but since the channel objects have different references, I'm getting an AssertionError: <grpc._channel.Channel object at 0x111911640> != <grpc._channel.Channel object at 0x11190c460>. Do you guys know if there is a way to compare the values stored inside the channel object instead of the references or perhaps get access to the compression attribute of the channel object?

The way I would go about it is to follow @lzchen's suggestion of checking that the method to create the channel was called with the expected parameters rather than trying to compare the channel objects themselves since all we're testing here is the input parameters

[wilguo]

The way I would go about it is to follow @lzchen's suggestion of checking that the method to create the channel was called with the expected parameters rather than trying to compare the channel objects themselves since all we're testing here is the input parameters

I did try and troubleshoot the AssertionError based on @lzchen's suggestion before trying it this way but I kept getting the assertion error still AssertionError: expected call not found. Expected: insecure_channel('localhost:55680', <Compression.Gzip: 2>) Actual: not called.. I checked that insecure_channel() does indeed get called based on print statements and self._client being assigned a correct object. I also confirmed that assert_called_with() is the correct method to use in this case, so would it be the case I wrote the mock.patch() part wrong?

[wilguo]

@codeboten @lzchen Is there another way that I could test this without using mock.path() since I'm unsure if mock.patch() is doing the right thing or not

[codeboten]

@wilguo if you want to just address @aabmass's comment, we can create a separate issue to track adding the test