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.

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

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

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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