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

chore: Fix lint warnings in core package #2405

Merged
merged 3 commits into from
Aug 11, 2021
Merged

chore: Fix lint warnings in core package #2405

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8fe9f66
chore(core): fix lint warnings
alisabzevari Aug 8, 2021
4b3363d
chore(core): rename time helper function parameter names
alisabzevari Aug 10, 2021
0f5f47c
Merge branch 'main' into lint-warnings-core
dyladan Aug 10, 2021
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: 1 addition & 1 deletion packages/opentelemetry-core/src/baggage/propagation/HttpBaggagePropagator.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ import {
* https://w3c.github.io/baggage/
*/
export class HttpBaggagePropagator implements TextMapPropagator {
inject(context: Context, carrier: unknown, setter: TextMapSetter) {
inject(context: Context, carrier: unknown, setter: TextMapSetter): void {
const baggage = propagation.getBaggage(context);
if (!baggage || isTracingSuppressed(context)) return;
const keyPairs = getKeyPairs(baggage)
Expand Down
26 changes: 14 additions & 12 deletions packages/opentelemetry-core/src/baggage/utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,33 +13,34 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
import { Baggage, baggageEntryMetadataFromString } from '@opentelemetry/api';
import { Baggage, BaggageEntryMetadata, baggageEntryMetadataFromString } from '@opentelemetry/api';
import {
BAGGAGE_ITEMS_SEPARATOR,
BAGGAGE_PROPERTIES_SEPARATOR,
BAGGAGE_KEY_PAIR_SEPARATOR,
BAGGAGE_MAX_TOTAL_LENGTH,
} from './constants';

export const serializeKeyPairs = (keyPairs: string[]) => {
type ParsedBaggageKeyValue = { key: string, value: string, metadata: BaggageEntryMetadata | undefined };
dyladan marked this conversation as resolved.
Show resolved Hide resolved

export function serializeKeyPairs(keyPairs: string[]): string {
return keyPairs.reduce((hValue: string, current: string) => {
const value = `${hValue}${
hValue !== '' ? BAGGAGE_ITEMS_SEPARATOR : ''
}${current}`;
const value = `${hValue}${hValue !== '' ? BAGGAGE_ITEMS_SEPARATOR : ''
}${current}`;
return value.length > BAGGAGE_MAX_TOTAL_LENGTH ? hValue : value;
}, '');
};
}

export const getKeyPairs = (baggage: Baggage): string[] => {
export function getKeyPairs(baggage: Baggage): string[] {
return baggage
.getAllEntries()
.map(
([key, value]) =>
`${encodeURIComponent(key)}=${encodeURIComponent(value.value)}`
);
};
}

export const parsePairKeyValue = (entry: string) => {
export function parsePairKeyValue(entry: string): ParsedBaggageKeyValue | undefined {
const valueProps = entry.split(BAGGAGE_PROPERTIES_SEPARATOR);
if (valueProps.length <= 0) return;
const keyPairPart = valueProps.shift();
Expand All @@ -55,13 +56,13 @@ export const parsePairKeyValue = (entry: string) => {
);
}
return { key, value, metadata };
};
}

/**
* Parse a string serialized in the baggage HTTP Format (without metadata):
* https://github.com/w3c/baggage/blob/master/baggage/HTTP_HEADER_FORMAT.md
*/
export const parseKeyPairsIntoRecord = (value?: string) => {
export function parseKeyPairsIntoRecord(value?: string): Record<string, string> {
if (typeof value !== 'string' || value.length === 0) return {};
return value
.split(BAGGAGE_ITEMS_SEPARATOR)
Expand All @@ -70,7 +71,8 @@ export const parseKeyPairsIntoRecord = (value?: string) => {
})
.filter(keyPair => keyPair !== undefined && keyPair.value.length > 0)
.reduce<Record<string, string>>((headers, keyPair) => {
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
headers[keyPair!.key] = keyPair!.value;
return headers;
}, {});
};
}
1 change: 1 addition & 0 deletions packages/opentelemetry-core/src/common/attributes.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ export function sanitizeAttributes(attributes: unknown): SpanAttributes {
return out;
}

// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
for (const [k, v] of Object.entries(attributes!)) {
if (isAttributeValue(v)) {
if (Array.isArray(v)) {
Expand Down
6 changes: 3 additions & 3 deletions packages/opentelemetry-core/src/common/global-error-handler.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,16 +25,16 @@ let delegateHandler = loggingErrorHandler();
* Set the global error handler
* @param {ErrorHandler} handler
*/
export function setGlobalErrorHandler(handler: ErrorHandler) {
export function setGlobalErrorHandler(handler: ErrorHandler): void {
delegateHandler = handler;
}

/**
* Return the global error handler
* @param {Exception} ex
*/
export const globalErrorHandler = (ex: Exception) => {
export function globalErrorHandler(ex: Exception): void {
try {
delegateHandler(ex);
} catch {} // eslint-disable-line no-empty
};
}
8 changes: 6 additions & 2 deletions packages/opentelemetry-core/src/common/time.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,7 @@ export function hrTimeDuration(
* Convert hrTime to timestamp, for example "2019-05-14T17:00:00.000123456Z"
* @param hrTime
*/
// eslint-disable-next-line @typescript-eslint/no-shadow
dyladan marked this conversation as resolved.
Show resolved Hide resolved
export function hrTimeToTimeStamp(hrTime: api.HrTime): string {
const precision = NANOSECOND_DIGITS;
const tmp = `${'0'.repeat(precision)}${hrTime[1]}Z`;
Expand All @@ -129,6 +130,7 @@ export function hrTimeToTimeStamp(hrTime: api.HrTime): string {
* Convert hrTime to nanoseconds.
* @param hrTime
*/
// eslint-disable-next-line @typescript-eslint/no-shadow
Copy link
Member

Choose a reason for hiding this comment

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

I think the no-shadow error should be rather fixed then ignored, in most cases this should be an easy fix

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have answered this in another comment as well. I couldn't find a better name for the parameter and thought the name of the parameter should be meaningful for users who don't use types.

Copy link
Member

Choose a reason for hiding this comment

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

I would be fine with just using time

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

Copy link
Member

Choose a reason for hiding this comment

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

The parameter name is only ever likely to show up in intellisense and generated docs. In both cases it woudl be alongside its type which is clearly named.

export function hrTimeToNanoseconds(hrTime: api.HrTime): number {
return hrTime[0] * SECOND_TO_NANOSECONDS + hrTime[1];
}
Expand All @@ -137,6 +139,7 @@ export function hrTimeToNanoseconds(hrTime: api.HrTime): number {
* Convert hrTime to milliseconds.
* @param hrTime
*/
// eslint-disable-next-line @typescript-eslint/no-shadow
export function hrTimeToMilliseconds(hrTime: api.HrTime): number {
return Math.round(hrTime[0] * 1e3 + hrTime[1] / 1e6);
}
Expand All @@ -145,6 +148,7 @@ export function hrTimeToMilliseconds(hrTime: api.HrTime): number {
* Convert hrTime to microseconds.
* @param hrTime
*/
// eslint-disable-next-line @typescript-eslint/no-shadow
export function hrTimeToMicroseconds(hrTime: api.HrTime): number {
return Math.round(hrTime[0] * 1e6 + hrTime[1] / 1e3);
}
Expand All @@ -153,7 +157,7 @@ export function hrTimeToMicroseconds(hrTime: api.HrTime): number {
* check if time is HrTime
* @param value
*/
export function isTimeInputHrTime(value: unknown) {
export function isTimeInputHrTime(value: unknown): value is api.HrTime {
return (
Array.isArray(value) &&
value.length === 2 &&
Expand All @@ -166,7 +170,7 @@ export function isTimeInputHrTime(value: unknown) {
* check if input value is a correct types.TimeInput
* @param value
*/
export function isTimeInput(value: unknown) {
export function isTimeInput(value: unknown): value is api.HrTime | number | Date {
return (
isTimeInputHrTime(value) ||
typeof value === 'number' ||
Expand Down
4 changes: 3 additions & 1 deletion packages/opentelemetry-core/src/common/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,11 @@ export interface TimeOriginLegacy {
* This interface defines the params that are be added to the wrapped function
* using the "shimmer.wrap"
*/
export interface ShimWrapped {
export interface ShimWrapped extends Function {
__wrapped: boolean;
// eslint-disable-next-line @typescript-eslint/ban-types
__unwrap: Function;
// eslint-disable-next-line @typescript-eslint/ban-types
__original: Function;
}

Expand Down
2 changes: 1 addition & 1 deletion packages/opentelemetry-core/src/propagation/composite.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ export class CompositePropagator implements TextMapPropagator {
* @param context Context to inject
* @param carrier Carrier into which context will be injected
*/
inject(context: Context, carrier: unknown, setter: TextMapSetter) {
inject(context: Context, carrier: unknown, setter: TextMapSetter): void {
for (const propagator of this._propagators) {
try {
propagator.inject(context, carrier, setter);
Expand Down
2 changes: 1 addition & 1 deletion packages/opentelemetry-core/src/trace/HttpTraceContextPropagator.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ export function parseTraceParent(traceParent: string): SpanContext | null {
* https://www.w3.org/TR/trace-context/
*/
export class HttpTraceContextPropagator implements TextMapPropagator {
inject(context: Context, carrier: unknown, setter: TextMapSetter) {
inject(context: Context, carrier: unknown, setter: TextMapSetter): void {
const spanContext = trace.getSpanContext(context);
if (
!spanContext ||
Expand Down
2 changes: 1 addition & 1 deletion packages/opentelemetry-core/src/utils/wrap.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import { ShimWrapped } from '../common/types';
* Checks if certain function has been already wrapped
* @param func
*/
export function isWrapped(func: any) {
export function isWrapped(func: unknown): func is ShimWrapped {
return (
typeof func === 'function' &&
typeof (func as ShimWrapped).__original === 'function' &&
Expand Down