Skip to content

Commit

Permalink
address comments
Browse files Browse the repository at this point in the history
  • Loading branch information
jj22ee committed Aug 8, 2024
1 parent 8c64f96 commit ba88fca
Show file tree
Hide file tree
Showing 8 changed files with 11 additions and 24 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@ export class AlwaysRecordSampler implements Sampler {
}

private constructor(rootSampler: Sampler) {
if (rootSampler === null) {
throw new Error('rootSampler is null. It must be provided');
if (rootSampler == null) {
throw new Error('rootSampler is null/undefined. It must be provided');
}
this.rootSampler = rootSampler;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -116,17 +116,9 @@ export class AttributePropagatingSpanProcessor implements SpanProcessor {
return SpanKind.SERVER === span.kind;
}

public isStartRequired(): boolean {
return true;
}

// eslint-disable-next-line @typescript-eslint/no-unused-vars
public onEnd(span: ReadableSpan): void {}

public isEndRequired(): boolean {
return false;
}

public shutdown(): Promise<void> {
return this.forceFlush();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,9 @@ export class AwsMetricAttributesSpanExporterBuilder {
* defaults to {@link DEFAULT_GENERATOR}. Must not be null.
*/
public setGenerator(generator: MetricAttributeGenerator): AwsMetricAttributesSpanExporterBuilder {
if (generator == null) {
throw new Error('generator must not be null/undefined');
}
this.generator = generator;
return this;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,8 @@ export class AwsMetricAttributesSpanExporter implements SpanExporter {
}

// Bypass `readonly` restriction of ReadableSpan's attributes.
// Workaround provided from official TypeScript docs:
// https://www.typescriptlang.org/docs/handbook/release-notes/typescript-2-8.html#improved-control-over-mapped-type-modifiers
type Mutable<T> = { -readonly [P in keyof T]: T[P] };
const mutableSpan: Mutable<ReadableSpan> = span;
mutableSpan.attributes = updateAttributes;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,9 @@ export class AwsSpanMetricsProcessorBuilder {
* processor. If unset, defaults to {@link DEFAULT_GENERATOR}. Must not be null.
*/
public setGenerator(generator: MetricAttributeGenerator): AwsSpanMetricsProcessorBuilder {
if (generator == null) {
throw new Error('generator must not be null/undefined');
}
this.generator = generator;
return this;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,10 +68,6 @@ export class AwsSpanMetricsProcessor implements SpanProcessor {
// eslint-disable-next-line @typescript-eslint/no-unused-vars
public onStart(span: Span, parentContext: Context): void {}

public isStartRequired(): boolean {
return false;
}

public onEnd(span: ReadableSpan): void {
const attributeMap: AttributeMap = this.generator.generateMetricAttributeMapFromSpan(span, this.resource);

Expand All @@ -80,10 +76,6 @@ export class AwsSpanMetricsProcessor implements SpanProcessor {
}
}

public isEndRequired(): boolean {
return true;
}

// The logic to record error and fault should be kept in sync with the aws-xray exporter whenever
// possible except for the throttle
// https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/main/exporter/awsxrayexporter/internal/translator/cause.go#L121-L160
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ export class SqsUrlParser {
}

private static isValidQueueName(input: string): boolean {
if (input === null || input.length === 0 || input.length > 80) {
if (input == null || input.length === 0 || input.length > 80) {
return false;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,11 +97,6 @@ describe('AwsSpanMetricsProcessorTest', () => {
);
});

it('testIsRequired', () => {
expect(awsSpanMetricsProcessor.isStartRequired()).toBeFalsy();
expect(awsSpanMetricsProcessor.isEndRequired()).toBeTruthy();
});

it('testStartDoesNothingToSpan', () => {
const parentContextMock: Context = {
getValue: (key: symbol) => 'unknown',
Expand Down

0 comments on commit ba88fca

Please sign in to comment.