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

refactor(api): fix eslint warnings #5366

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
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
17 changes: 8 additions & 9 deletions api/src/diag/ComponentLogger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
*/

import { getGlobal } from '../internal/global-utils';
import { ComponentLoggerOptions, DiagLogger, DiagLogFunction } from './types';
import { ComponentLoggerOptions, DiagLogger } from './types';

/**
* Component Logger which is meant to be used as part of any component which
Expand All @@ -33,38 +33,37 @@ export class DiagComponentLogger implements DiagLogger {
this._namespace = props.namespace || 'DiagComponentLogger';
}

public debug(...args: any[]): void {
public debug(...args: unknown[]): void {
return logProxy('debug', this._namespace, args);
}

public error(...args: any[]): void {
public error(...args: unknown[]): void {
return logProxy('error', this._namespace, args);
}

public info(...args: any[]): void {
public info(...args: unknown[]): void {
return logProxy('info', this._namespace, args);
}

public warn(...args: any[]): void {
public warn(...args: unknown[]): void {
return logProxy('warn', this._namespace, args);
}

public verbose(...args: any[]): void {
public verbose(...args: unknown[]): void {
return logProxy('verbose', this._namespace, args);
}
}

function logProxy(
funcName: keyof DiagLogger,
namespace: string,
args: any
args: unknown[]
): void {
const logger = getGlobal('diag');
// shortcut if logger not set
if (!logger) {
return;
}

args.unshift(namespace);
return logger[funcName](...(args as Parameters<DiagLogFunction>));
return logger[funcName](namespace, ...args);
}
10 changes: 8 additions & 2 deletions api/src/experimental/trace/SugaredTracer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,13 @@
* limitations under the License.
*/
import { SugaredSpanOptions } from './SugaredOptions';
import { context, Context, Span, SpanStatusCode, Tracer } from '../../';
import {
context as contextApi,
Context,
Span,
SpanStatusCode,
Tracer,
} from '../../';

const defaultOnException = (e: Error, span: Span) => {
span.recordException(e);
Expand Down Expand Up @@ -174,7 +180,7 @@ function massageParams<F extends (span: Span) => ReturnType<F>>(
fn = arg3 as F;
}
opts = opts ?? {};
ctx = ctx ?? context.active();
ctx = ctx ?? contextApi.active();

return { opts, ctx, fn };
}
Expand Down
6 changes: 5 additions & 1 deletion api/src/trace/NoopTracer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -99,11 +99,15 @@ export class NoopTracer implements Tracer {
}
}

function isSpanContext(spanContext: any): spanContext is SpanContext {
function isSpanContext(spanContext: unknown): spanContext is SpanContext {
return (
spanContext !== null &&
typeof spanContext === 'object' &&
'spanId' in spanContext &&
typeof spanContext['spanId'] === 'string' &&
'traceId' in spanContext &&
typeof spanContext['traceId'] === 'string' &&
'traceFlags' in spanContext &&
typeof spanContext['traceFlags'] === 'number'
);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This imposes a very slight runtime cost from the additional check. If that is undesirable here, there are plenty of alternative factoring that would accomplish the same without runtime code, this is just the most readable IMO.

Alternatively, if we trust the types, this function can be deleted altogether. It's only ever called from L47; the input came from getSpanContext(), which is typed to return SpanContext | undefined. If that is accurate (looks like it to me), then all we really needed to check is !== undefined and it can be done inline.

}
Loading