Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Split connection management away from exporter #1369

Merged
merged 13 commits into from
Dec 21, 2020
Merged

Split connection management away from exporter #1369

merged 13 commits into from
Dec 21, 2020

Commits on Dec 21, 2020

  1. Split protocol handling away from exporter 

    This commits adds a ProtocolDriver interface, which the exporter
will use to connect to the collector and send both metrics and traces
to it. That way, the Exporter type is free from dealing with any
connection/protocol details, as this business is taken over by the
implementations of the ProtocolDriver interface.

The gRPC code from the exporter is moved into the implementation of
ProtocolDriver. Currently it only maintains a single connection,
just as the Exporter used to do.

With the split, most of the Exporter options became actually gRPC
connection manager's options. Currently the only option that remained
to be Exporter's is about setting the export kind selector.
    @krnowak
    krnowak committed Dec 21, 2020
    Configuration menu
    Copy the full SHA
    1f77f7a View commit details
    Browse the repository at this point in the history

  2. Update changelog

    @krnowak
    krnowak committed Dec 21, 2020
    Configuration menu
    Copy the full SHA
    833fcad View commit details
    Browse the repository at this point in the history

  3. Increase the test coverage of GRPC driver

    @krnowak
    krnowak committed Dec 21, 2020
    Configuration menu
    Copy the full SHA
    d3d3ac7 View commit details
    Browse the repository at this point in the history

  4. Do not close a channel with multiple senders 

    The disconnected channel can be used for sending by multiple
goroutines (for example, by metric controller and span processor), so
this channel should not be closed at all. Dropping this line closes a
race between closing a channel and sending to it.
    @krnowak
    krnowak committed Dec 21, 2020
    Configuration menu
    Copy the full SHA
    8be7ab5 View commit details
    Browse the repository at this point in the history

  5. Simplify new connection handler 

    The callbacks never return an error, so drop the return type from it.
    @krnowak
    krnowak committed Dec 21, 2020
    Configuration menu
    Copy the full SHA
    30baa83 View commit details
    Browse the repository at this point in the history

  6. Access clients under a lock 

    The client may change as a result on reconnection in background, so
guard against a racy access.
    @krnowak
    krnowak committed Dec 21, 2020
    Configuration menu
    Copy the full SHA
    f5ebb7c View commit details
    Browse the repository at this point in the history

  7. Simplify the GRPC driver a bit 

    The config type was exported earlier to have a consistent way of
configuring the driver, when also the multiple connection driver would
appear. Since we are not going to add a multiple connection driver,
pass the options directly to the driver constructor. Also shorten the
name of the constructor to `NewGRPCDriver`.
    @krnowak
    krnowak committed Dec 21, 2020
    Configuration menu
    Copy the full SHA
    b24e438 View commit details
    Browse the repository at this point in the history

  8. Merge common gRPC code back into the driver 

    The common code was supposed to be shared between single connection
driver and multiple connection driver, but since the latter won't be
happening, it makes no sense to keep the not-so-common code in a
separate file. Also drop some abstraction too.
    @krnowak
    krnowak committed Dec 21, 2020
    Configuration menu
    Copy the full SHA
    c7eb424 View commit details
    Browse the repository at this point in the history

  9. Rename the file with gRPC driver implementation

    @krnowak
    krnowak committed Dec 21, 2020
    Configuration menu
    Copy the full SHA
    f096993 View commit details
    Browse the repository at this point in the history

  10. Update changelog

    @krnowak
    krnowak committed Dec 21, 2020
    Configuration menu
    Copy the full SHA
    6984cbf View commit details
    Browse the repository at this point in the history

  11. Sleep for a second to trigger the timeout 

    Sometimes CI has it's better moments, so it's blazing fast and manages
to finish shutting the exporter down within the 1 microsecond timeout.
    @krnowak
    krnowak committed Dec 21, 2020
    Configuration menu
    Copy the full SHA
    23ca98b View commit details
    Browse the repository at this point in the history

  12. Increase the timeout for shutting down the exporter 

    One millisecond is quite short, and I was getting failures locally or
in CI:

go test ./... + race in ./exporters/otlp
2020/12/14 18:27:54 rpc error: code = Canceled desc = context canceled
2020/12/14 18:27:54 context deadline exceeded
--- FAIL: TestNewExporter_withMultipleAttributeTypes (0.37s)
    otlp_integration_test.go:541: resource span count: got 0, want 1
FAIL
FAIL	go.opentelemetry.io/otel/exporters/otlp	5.278s

or

go test ./... + coverage in ./exporters/otlp
2020/12/14 17:41:16 rpc error: code = Canceled desc = context canceled
2020/12/14 17:41:16 exporter disconnected
--- FAIL: TestNewExporter_endToEnd (1.53s)
    --- FAIL: TestNewExporter_endToEnd/WithCompressor (0.41s)
        otlp_integration_test.go:246: span counts: got 3, want 4
2020/12/14 17:41:18 context canceled
FAIL
coverage: 35.3% of statements in ./...
FAIL	go.opentelemetry.io/otel/exporters/otlp	4.753s
    @krnowak
    krnowak committed Dec 21, 2020
    Configuration menu
    Copy the full SHA
    6843a0d View commit details
    Browse the repository at this point in the history

  13. Shut down the providers in end to end test 

    This is to make sure that all batched spans are actually flushed
before closing the exporter.
    @krnowak
    krnowak committed Dec 21, 2020
    Configuration menu
    Copy the full SHA
    1123f58 View commit details
    Browse the repository at this point in the history