Skip to content

Commit

Permalink
refactor(otlp-exporter-base): fix eslint warnigns
Browse files Browse the repository at this point in the history
Fix the following eslint warnings:

```
/home/runner/work/opentelemetry-js/opentelemetry-js/experimental/packages/otlp-exporter-base/src/transport/http-exporter-transport.ts
  55:9  warning  Forbidden non-null assertion  @typescript-eslint/no-non-null-assertion

/home/runner/work/opentelemetry-js/opentelemetry-js/experimental/packages/otlp-exporter-base/src/transport/http-transport-utils.ts
  95:35  warning  Unexpected any. Specify a different type  @typescript-eslint/no-explicit-any
```

The first was that we know we already checked and initialized a
related variable, but TypeScript has no idea they are related. We
can better communicate that, both to TypeScript and to humans, by
groupping the related variables into a single one, that is either
set or not.

The second was a little strange, and I don't really see what the
`any` is doing there. `res.on('end')` is already typed to have an
`Error` type on the callback argument (maybe that wasn't always the
case?). If we trust the type, then there is nothing more we need to
do. If we don't trust the type, widening it to `any` without other
runtime checks isn't going to do anything for us either. So I just
removed it and everything was still fine.

Ref open-telemetry#5365
  • Loading branch information
chancancode committed Jan 29, 2025
1 parent e42fbb9 commit 14051e4
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 21 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -26,33 +26,23 @@ import type * as http from 'http';
import { ExportResponse } from '../export-response';
import { IExporterTransport } from '../exporter-transport';

interface Utils {
agent: http.Agent | https.Agent;
send: sendWithHttp;
}

class HttpExporterTransport implements IExporterTransport {
private _send: sendWithHttp | null = null;
private _agent: http.Agent | https.Agent | null = null;
private _utils: Utils | null = null;

constructor(private _parameters: HttpRequestParameters) {}

async send(data: Uint8Array, timeoutMillis: number): Promise<ExportResponse> {
if (this._send == null) {
// Lazy require to ensure that http/https is not required before instrumentations can wrap it.
const {
sendWithHttp,
createHttpAgent,
// eslint-disable-next-line @typescript-eslint/no-var-requires
} = require('./http-transport-utils');
this._agent = createHttpAgent(
this._parameters.url,
this._parameters.agentOptions
);
this._send = sendWithHttp;
}
const { agent, send } = this._loadUtils();

return new Promise<ExportResponse>(resolve => {
// this will always be defined
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
this._send?.(
send(
this._parameters,
this._agent!,
agent,
data,
result => {
resolve(result);
Expand All @@ -61,9 +51,33 @@ class HttpExporterTransport implements IExporterTransport {
);
});
}

shutdown() {
// intentionally left empty, nothing to do.
}

private _loadUtils(): Utils {
let utils = this._utils;

if (utils === null) {
// Lazy require to ensure that http/https is not required before instrumentations can wrap it.
const {
sendWithHttp,
createHttpAgent,
// eslint-disable-next-line @typescript-eslint/no-var-requires
} = require('./http-transport-utils');

utils = this._utils = {
agent: createHttpAgent(
this._parameters.url,
this._parameters.agentOptions
),
send: sendWithHttp,
};
}

return utils;
}
}

export function createHttpExporterTransport(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,10 +92,10 @@ export function sendWithHttp(
error: new Error('Request Timeout'),
});
});
req.on('error', (error: Error | any) => {
req.on('error', (error: Error) => {
onDone({
status: 'failure',
error: error,
error,
});
});

Expand Down

0 comments on commit 14051e4

Please sign in to comment.