Skip to content

Commit

Permalink
Reduce usage of 'as' statements (#17)
Browse files Browse the repository at this point in the history
*Issue #, if available:*

We use the `as` (e.g. `variableName as varableType`) to tell the
TypeScript compiler to trust us that we know a certain variable is a
certain type. Example:
```
span.attributes[AWS_ATTRIBUTE_KEYS.AWS_KINESIS_STREAM_NAME]
```
We know that this will be a string, but technically any span attribute
value can be of type `string | number | boolean | Array<null | undefined
| string> | Array<null | undefined | number> | Array<null | undefined |
boolean>`

So instead of assuming this is a specific variable type via `as` and
ensure it isn't undefined, we should check for the actual type in the
above list.

*Description of changes:*
- Reduce `as` statements
- Resolve TypeScript compiler blockers as a result of reducing usage of
`as` statements
- Use correct `parentSpan instanceof Span` check in
`attribute-propagating-span-processor.ts`


By submitting this pull request, I confirm that you can use, modify,
copy, and redistribute this contribution, under the terms of your
choice.
  • Loading branch information
jj22ee authored Aug 16, 2024
1 parent a83c981 commit 2110106
Show file tree
Hide file tree
Showing 5 changed files with 43 additions and 47 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -52,13 +52,14 @@ export class AttributePropagatingSpanProcessor implements SpanProcessor {
AwsSpanProcessingUtil.setIsLocalRootInformation(span, parentContext);

const parentSpan: APISpan | undefined = trace.getSpan(parentContext);
let parentReadableSpan: ReadableSpan | undefined = undefined;

// `if check` is different than Python and Java. Here we just check if parentSpan is not undefined
// Whereas in Python and Java, the check is if parentSpan is and instance of ReadableSpan, which is
// not possible in TypeScript because the check is not allowed for interfaces (such as ReadableSpan).
if (parentSpan !== undefined) {
parentReadableSpan = parentSpan as Span;
let parentReadableSpan: Span | undefined = undefined;

// In Python and Java, the check is "parentSpan is an instance of ReadableSpan" is not possible
// in TypeScript because the check is not allowed for TypeScript interfaces (such as ReadableSpan).
// This is because JavaScript doesn't support interfaces, which is what TypeScript will compile to.
// `Span` is the only class that implements ReadableSpan, so check for instance of Span.
if (parentSpan instanceof Span) {
parentReadableSpan = parentSpan;

// Add the AWS_SDK_DESCENDANT attribute to the immediate child spans of AWS SDK span.
// This attribute helps the backend differentiate between SDK spans and their immediate
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -258,17 +258,18 @@ export class AwsMetricAttributeGenerator implements MetricAttributeGenerator {
const httpUrl: AttributeValue | undefined = span.attributes[SEMATTRS_HTTP_URL];
try {
let url: URL;
if (httpUrl !== undefined) {
url = new URL(httpUrl as string);
if (typeof httpUrl === 'string') {
url = new URL(httpUrl);
remoteOperation = AwsSpanProcessingUtil.extractAPIPathValue(url.pathname);
}
} catch (e: unknown) {
diag.verbose(`invalid http.url attribute: ${httpUrl}`);
}
}
if (AwsSpanProcessingUtil.isKeyPresent(span, SEMATTRS_HTTP_METHOD)) {
const httpMethod: AttributeValue | undefined = span.attributes[SEMATTRS_HTTP_METHOD];
remoteOperation = (httpMethod as string) + ' ' + remoteOperation;

const httpMethod: AttributeValue | undefined = span.attributes[SEMATTRS_HTTP_METHOD];
if (AwsSpanProcessingUtil.isKeyPresent(span, SEMATTRS_HTTP_METHOD) && typeof httpMethod === 'string') {
remoteOperation = httpMethod + ' ' + remoteOperation;
}
if (remoteOperation === AwsSpanProcessingUtil.UNKNOWN_REMOTE_OPERATION) {
AwsMetricAttributeGenerator.logUnknownAttribute(AWS_ATTRIBUTE_KEYS.AWS_REMOTE_OPERATION, span);
Expand All @@ -283,13 +284,13 @@ export class AwsMetricAttributeGenerator implements MetricAttributeGenerator {
remoteService = AwsMetricAttributeGenerator.getRemoteService(span, SEMATTRS_NET_PEER_NAME);
if (AwsSpanProcessingUtil.isKeyPresent(span, SEMATTRS_NET_PEER_PORT)) {
const port: AttributeValue | undefined = span.attributes[SEMATTRS_NET_PEER_PORT];
remoteService += ':' + (port as string);
remoteService += ':' + port;
}
} else if (AwsSpanProcessingUtil.isKeyPresent(span, _NET_SOCK_PEER_ADDR)) {
remoteService = AwsMetricAttributeGenerator.getRemoteService(span, _NET_SOCK_PEER_ADDR);
if (AwsSpanProcessingUtil.isKeyPresent(span, _NET_SOCK_PEER_PORT)) {
const port: AttributeValue | undefined = span.attributes[_NET_SOCK_PEER_PORT];
remoteService += ':' + (port as string);
remoteService += ':' + port;
}
} else if (AwsSpanProcessingUtil.isKeyPresent(span, SEMATTRS_HTTP_URL)) {
const httpUrl: string = span.attributes[SEMATTRS_HTTP_URL] as string;
Expand Down Expand Up @@ -437,15 +438,15 @@ export class AwsMetricAttributeGenerator implements MetricAttributeGenerator {
address: AttributeValue | undefined,
port: AttributeValue | undefined
): string | undefined {
if (address === undefined) {
if (typeof address !== 'string') {
return undefined;
}

return AwsMetricAttributeGenerator.escapeDelimiters(address as string) + (port !== undefined ? '|' + port : '');
return AwsMetricAttributeGenerator.escapeDelimiters(address) + (port !== undefined ? '|' + port : '');
}

private static buildDbConnectionString(connectionString: AttributeValue | undefined): string | undefined {
if (connectionString === undefined) {
if (typeof connectionString !== 'string') {
return undefined;
}

Expand All @@ -461,11 +462,11 @@ export class AwsMetricAttributeGenerator implements MetricAttributeGenerator {
// - jdbc:mysql://localhost:3306
// - abc:def:ghi://host:3306
// Try with a dummy schema without `:`, since we do not care about the schema
const schemeEndIndex: number = (connectionString as string).indexOf('://');
const schemeEndIndex: number = connectionString.indexOf('://');
if (schemeEndIndex === -1) {
uri = new URL(connectionString as string);
uri = new URL(connectionString);
} else {
uri = new URL('dummyschema' + (connectionString as string).substring(schemeEndIndex));
uri = new URL('dummyschema' + connectionString.substring(schemeEndIndex));
}

address = uri.hostname;
Expand All @@ -492,7 +493,7 @@ export class AwsMetricAttributeGenerator implements MetricAttributeGenerator {
// `split(a).join(b)` is not equivalent for all (a,b), but works with `a = '^'` or a = '|'`.
// Implementing some regex is also possible
// e.g. let re = new RegExp(String.raw`\s${variable}\s`, "g");
return (input as string).split('^').join('^^').split('|').join('^|');
return input.split('^').join('^^').split('|').join('^|');
}

/** Span kind is needed for differentiating metrics in the EMF exporter */
Expand All @@ -517,18 +518,18 @@ export class AwsMetricAttributeGenerator implements MetricAttributeGenerator {

private static getRemoteService(span: ReadableSpan, remoteServiceKey: string): string {
let remoteService: AttributeValue | undefined = span.attributes[remoteServiceKey];
if (remoteService === undefined) {
if (typeof remoteService !== 'string') {
remoteService = AwsSpanProcessingUtil.UNKNOWN_REMOTE_SERVICE;
}
return remoteService as string;
return remoteService;
}

private static getRemoteOperation(span: ReadableSpan, remoteOperationKey: string): string {
let remoteOperation: AttributeValue | undefined = span.attributes[remoteOperationKey];
if (remoteOperation === undefined) {
if (typeof remoteOperation !== 'string') {
remoteOperation = AwsSpanProcessingUtil.UNKNOWN_REMOTE_OPERATION;
}
return remoteOperation as string;
return remoteOperation;
}

/**
Expand All @@ -539,13 +540,13 @@ export class AwsMetricAttributeGenerator implements MetricAttributeGenerator {
*/
private static getDBStatementRemoteOperation(span: ReadableSpan, remoteOperationKey: string): string {
let remoteOperation: AttributeValue | undefined = span.attributes[remoteOperationKey];
if (remoteOperation === undefined) {
if (typeof remoteOperation !== 'string') {
remoteOperation = AwsSpanProcessingUtil.UNKNOWN_REMOTE_OPERATION;
}

// Remove all whitespace and newline characters from the beginning of remote_operation
// and retrieve the first MAX_KEYWORD_LENGTH characters
remoteOperation = (remoteOperation as string).trimStart();
remoteOperation = remoteOperation.trimStart();
if (remoteOperation.length > AwsSpanProcessingUtil.MAX_KEYWORD_LENGTH) {
remoteOperation = remoteOperation.substring(0, AwsSpanProcessingUtil.MAX_KEYWORD_LENGTH);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -454,7 +454,7 @@ export class AwsSpanProcessorProvider {
}

protected _getSpanExporter(name: string): SpanExporter | undefined {
return (this.constructor as typeof AwsSpanProcessorProvider)._registeredExporters.get(name)?.();
return AwsSpanProcessorProvider._registeredExporters.get(name)?.();
}

private configureSpanProcessors(exporters: SpanExporter[]): (BatchSpanProcessor | SimpleSpanProcessor)[] {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,14 +83,14 @@ export class AwsSpanMetricsProcessor implements SpanProcessor {
let httpStatusCode: AttributeValue | undefined = spanData.attributes[SEMATTRS_HTTP_STATUS_CODE];
const statusCode: SpanStatusCode = spanData.status.code;

if (httpStatusCode === undefined) {
if (typeof httpStatusCode !== 'number') {
httpStatusCode = attributes[SEMATTRS_HTTP_STATUS_CODE];
}

if (
httpStatusCode === undefined ||
(httpStatusCode as number) < this.ERROR_CODE_LOWER_BOUND ||
(httpStatusCode as number) > this.FAULT_CODE_UPPER_BOUND
typeof httpStatusCode !== 'number' ||
httpStatusCode < this.ERROR_CODE_LOWER_BOUND ||
httpStatusCode > this.FAULT_CODE_UPPER_BOUND
) {
if (SpanStatusCode.ERROR === statusCode) {
this.errorHistogram.record(0, attributes);
Expand All @@ -99,16 +99,10 @@ export class AwsSpanMetricsProcessor implements SpanProcessor {
this.errorHistogram.record(0, attributes);
this.faultHistogram.record(0, attributes);
}
} else if (
(httpStatusCode as number) >= this.ERROR_CODE_LOWER_BOUND &&
(httpStatusCode as number) <= this.ERROR_CODE_UPPER_BOUND
) {
} else if (httpStatusCode >= this.ERROR_CODE_LOWER_BOUND && httpStatusCode <= this.ERROR_CODE_UPPER_BOUND) {
this.errorHistogram.record(1, attributes);
this.faultHistogram.record(0, attributes);
} else if (
(httpStatusCode as number) >= this.FAULT_CODE_LOWER_BOUND &&
(httpStatusCode as number) <= this.FAULT_CODE_UPPER_BOUND
) {
} else if (httpStatusCode >= this.FAULT_CODE_LOWER_BOUND && httpStatusCode <= this.FAULT_CODE_UPPER_BOUND) {
this.errorHistogram.record(0, attributes);
this.faultHistogram.record(1, attributes);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -154,12 +154,12 @@ export class AwsSpanProcessingUtil {
// is started before the other processors (e.g. AwsSpanMetricsProcessor)
// Thus this function is implemented differently than in Java/Python
const isLocalRoot: AttributeValue | undefined = spanData.attributes[AWS_ATTRIBUTE_KEYS.AWS_IS_LOCAL_ROOT];
if (isLocalRoot === undefined) {
// isLocalRoot should be precalculated, this code block should not be entered
if (typeof isLocalRoot !== 'boolean') {
// isLocalRoot should be a precalculated boolean, this code block should not be entered
diag.debug('isLocalRoot for span has not been precalculated. Assuming span is Local Root Span.');
return true;
}
return isLocalRoot as boolean;
return isLocalRoot;
}

// To identify the SQS consumer spans produced by AWS SDK instrumentation
Expand Down Expand Up @@ -223,8 +223,8 @@ export class AwsSpanProcessingUtil {
const httpUrl: AttributeValue | undefined = span.attributes[SEMATTRS_HTTP_URL];
try {
let url: URL;
if (httpUrl !== undefined) {
url = new URL(httpUrl as string);
if (typeof httpUrl === 'string') {
url = new URL(httpUrl);
httpPath = url.pathname;
}
} catch (e: unknown) {
Expand All @@ -235,8 +235,8 @@ export class AwsSpanProcessingUtil {
}
}

if (httpPath !== undefined) {
operation = this.extractAPIPathValue(httpPath as string);
if (typeof httpPath === 'string') {
operation = this.extractAPIPathValue(httpPath);
if (this.isKeyPresent(span, SEMATTRS_HTTP_METHOD)) {
const httpMethod: AttributeValue | undefined = span.attributes[SEMATTRS_HTTP_METHOD];
if (httpMethod !== undefined) {
Expand Down

0 comments on commit 2110106

Please sign in to comment.