Skip to content

Commit

Permalink
fix(otlp-grpc-exporter-base): avoid TypeError on exporter shutdown (#…
Browse files Browse the repository at this point in the history
…4612)

* fix(otlp-grpc-exporter-base): avoid TypeError on exporter shutdown

* chore: update changelog

* fix: use gRPC Client type over any

* fixup! fix: use gRPC Client type over any

* fix: use ts-lint/ts-ignore
  • Loading branch information
pichlermarc committed Apr 11, 2024
1 parent b067aed commit 3438777
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 19 deletions.
2 changes: 2 additions & 0 deletions experimental/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ All notable changes to experimental packages in this project will be documented

### :bug: (Bug Fix)

* fix(otlp-grpc-exporter-base): avoid TypeError on exporter shutdown [#4612](https://github.com/open-telemetry/opentelemetry-js/pull/4612)

### :books: (Refine Doc)

### :house: (Internal)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,12 @@

// NOTE: do not change these type imports to actual imports. Doing so WILL break `@opentelemetry/instrumentation-http`,
// as they'd be imported before the http/https modules can be wrapped.
import type { Metadata, ServiceError, ChannelCredentials } from '@grpc/grpc-js';
import type {
Metadata,
ServiceError,
ChannelCredentials,
Client,
} from '@grpc/grpc-js';
import { ExportResponse } from './export-response';
import { IExporterTransport } from './exporter-transport';

Expand Down Expand Up @@ -85,13 +90,13 @@ export interface GrpcExporterTransportParameters {
}

export class GrpcExporterTransport implements IExporterTransport {
private _client?: any;
private _client?: Client;
private _metadata?: Metadata;

constructor(private _parameters: GrpcExporterTransportParameters) {}

shutdown() {
this._client?.shutdown();
this._client?.close();
}

send(data: Uint8Array): Promise<ExportResponse> {
Expand Down Expand Up @@ -150,6 +155,8 @@ export class GrpcExporterTransport implements IExporterTransport {
});
}

// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore The gRPC client constructor is created on runtime, so we don't have any types for the resulting client.
this._client.export(
buffer,
this._metadata,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -136,37 +136,49 @@ describe('GrpcExporterTransport', function () {
});
});
describe('shutdown', function () {
let shutdownHandle: () => void | undefined;
const serverTestContext: ServerTestContext = {
requests: [],
serverResponseProvider: () => {
return { error: null, buffer: Buffer.from([]) };
},
};

beforeEach(async function () {
shutdownHandle = await startServer(serverTestContext);
});

afterEach(function () {
shutdownHandle();

// clear context
serverTestContext.requests = [];
serverTestContext.serverResponseProvider = () => {
return { error: null, buffer: Buffer.from([]) };
};

sinon.restore();
});

it('before send() does not error', function () {
const transport = new GrpcExporterTransport(simpleClientConfig);
transport.shutdown();

// no assertions, just checking that it does not throw any errors.
});

it('calls client shutdown if client is defined', function () {
// arrange
const transport = new GrpcExporterTransport({
metadata: createEmptyMetadata,
timeoutMillis: 100,
grpcPath: 'path',
grpcName: 'name',
credentials: createInsecureCredentials,
compression: 'gzip',
address: 'localhost:1234',
});
const shutdownStub = sinon.stub();
transport['_client'] = {
shutdown: shutdownStub,
};
it('calls _client.close() if client is defined', async function () {
const transport = new GrpcExporterTransport(simpleClientConfig);
// send something so that client is defined
await transport.send(Buffer.from([1, 2, 3]));
assert.ok(transport['_client'], '_client is not defined after send()');
const closeSpy = sinon.spy(transport['_client'], 'close');

// act
transport.shutdown();

// assert
sinon.assert.calledOnce(shutdownStub);
sinon.assert.calledOnce(closeSpy);
});
});
describe('send', function () {
Expand Down

0 comments on commit 3438777

Please sign in to comment.