Skip to content

Commit

Permalink
Review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
dyladan committed Aug 2, 2022
1 parent 39b27d0 commit f23b868
Show file tree
Hide file tree
Showing 7 changed files with 84 additions and 73 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,7 @@ describe('Utility', () => {
'test',
{ spanId: '', traceId: '', traceFlags: TraceFlags.SAMPLED },
SpanKind.INTERNAL,
AnchoredClock.create(Date, otperformance),
new AnchoredClock(Date, otperformance),
);
/* tslint:disable-next-line:no-any */
utils.setSpanWithError(span, new Error(errorMessage));
Expand Down Expand Up @@ -488,7 +488,7 @@ describe('Utility', () => {
'test',
{ spanId: '', traceId: '', traceFlags: TraceFlags.SAMPLED },
SpanKind.INTERNAL,
AnchoredClock.create(Date, otperformance),
new AnchoredClock(Date, otperformance),
);
});

Expand Down
27 changes: 17 additions & 10 deletions packages/opentelemetry-core/src/common/anchored-clock.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@

/**
* A utility for returning wall times anchored to a given point in time. Wall time measurements will
* not be taken from the system, but instead are computed by adding performance.now() monotonic time
* not be taken from the system, but instead are computed by adding a monotonic clock time
* to the anchor point.
*
* This is needed because the system time can change and result in unexpected situations like
Expand All @@ -31,23 +31,30 @@
* https://github.com/open-telemetry/opentelemetry-java/blob/main/sdk/trace/src/main/java/io/opentelemetry/sdk/trace/AnchoredClock.java
*/
export class AnchoredClock {
private constructor(
private _performance: Clock,
private _epochMillis: number,
private _performanceMillis: number,
) { }
private _monotonicClock: Clock;
private _epochMillis: number;
private _performanceMillis: number;

/** Create a new AnchoredClock anchored to the current time returned by clock */
public static create(clock: Clock, performance: Clock) {
return new AnchoredClock(performance, clock.now(), performance.now());
/**
* Create a new AnchoredClock anchored to the current time returned by systemClock.
*
* @param systemClock should be a clock that returns the number of milliseconds since January 1 1970 such as Date
* @param monotonicClock should be a clock that counts milliseconds monotonically such as window.performance or perf_hooks.performance
*/
public constructor(systemClock: Clock, monotonicClock: Clock) {
this._monotonicClock = monotonicClock;
this._epochMillis = systemClock.now();
this._performanceMillis = monotonicClock.now();
}

/** */

/**
* Returns the current time by adding the number of milliseconds since the
* AnchoredClock was created to the creation epoch time
*/
public now(): number {
const delta = this._performance.now() - this._performanceMillis;
const delta = this._monotonicClock.now() - this._performanceMillis;
return this._epochMillis + delta;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ describe('transform', () => {
'my-span',
spanContext,
api.SpanKind.SERVER,
AnchoredClock.create(Date, otperformance),
new AnchoredClock(Date, otperformance),
parentId
);
span.setAttributes({
Expand Down Expand Up @@ -120,7 +120,7 @@ describe('transform', () => {
'my-span',
spanContext,
api.SpanKind.SERVER,
AnchoredClock.create(Date, otperformance)
new AnchoredClock(Date, otperformance)
);
span.end();

Expand Down Expand Up @@ -169,7 +169,7 @@ describe('transform', () => {
'my-span',
spanContext,
item.ot,
AnchoredClock.create(Date, otperformance)
new AnchoredClock(Date, otperformance)
);
span.end();

Expand Down Expand Up @@ -212,7 +212,7 @@ describe('transform', () => {
'my-span',
spanContext,
api.SpanKind.SERVER,
AnchoredClock.create(Date, otperformance),
new AnchoredClock(Date, otperformance),
parentId
);
span.setAttributes({
Expand Down Expand Up @@ -243,7 +243,7 @@ describe('transform', () => {
'my-span',
spanContext,
api.SpanKind.SERVER,
AnchoredClock.create(Date, otperformance),
new AnchoredClock(Date, otperformance),
parentId
);
const status: api.SpanStatus = {
Expand Down Expand Up @@ -280,7 +280,7 @@ describe('transform', () => {
'my-span',
spanContext,
api.SpanKind.SERVER,
AnchoredClock.create(Date, otperformance),
new AnchoredClock(Date, otperformance),
parentId
);
const status: api.SpanStatus = {
Expand Down Expand Up @@ -322,7 +322,7 @@ describe('transform', () => {
'my-span',
spanContext,
api.SpanKind.SERVER,
AnchoredClock.create(Date, otperformance),
new AnchoredClock(Date, otperformance),
parentId
);
span.addEvent('my-event1');
Expand Down
10 changes: 5 additions & 5 deletions packages/opentelemetry-sdk-trace-base/src/Span.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ export class Span implements api.Span, ReadableSpan {
private readonly _spanProcessor: SpanProcessor;
private readonly _spanLimits: SpanLimits;
private readonly _attributeValueLengthLimit: number;
private readonly clock: AnchoredClock;
private readonly _clock: AnchoredClock;

/** Constructs a new Span instance. */
constructor(
Expand All @@ -73,7 +73,7 @@ export class Span implements api.Span, ReadableSpan {
links: api.Link[] = [],
startTime?: api.TimeInput,
) {
this.clock = clock;
this._clock = clock;
this.name = spanName;
this._spanContext = spanContext;
this.parentSpanId = parentSpanId;
Expand Down Expand Up @@ -150,7 +150,7 @@ export class Span implements api.Span, ReadableSpan {
attributesOrStartTime = undefined;
}
if (typeof startTime === 'undefined') {
startTime = this.clock.now();
startTime = this._clock.now();
}

const attributes = sanitizeAttributes(attributesOrStartTime);
Expand Down Expand Up @@ -181,7 +181,7 @@ export class Span implements api.Span, ReadableSpan {
}
this._ended = true;

this.endTime = timeInputToHrTime(endTime ?? this.clock.now());
this.endTime = timeInputToHrTime(endTime ?? this._clock.now());
this._duration = hrTimeDuration(this.startTime, this.endTime);

if (this._duration[0] < 0) {
Expand All @@ -199,7 +199,7 @@ export class Span implements api.Span, ReadableSpan {
return this._ended === false;
}

recordException(exception: api.Exception, time: api.TimeInput = this.clock.now()): void {
recordException(exception: api.Exception, time: api.TimeInput = this._clock.now()): void {
const attributes: api.SpanAttributes = {};
if (typeof exception === 'string') {
attributes[SemanticAttributes.EXCEPTION_MESSAGE] = exception;
Expand Down
10 changes: 7 additions & 3 deletions packages/opentelemetry-sdk-trace-base/src/Tracer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ export class Tracer implements api.Tracer {
private readonly _idGenerator: IdGenerator;
readonly resource: Resource;
readonly instrumentationLibrary: InstrumentationLibrary;
private clocks: WeakMap<api.Span, AnchoredClock> = new WeakMap();

/**
* Constructs a new Tracer instance.
Expand Down Expand Up @@ -126,10 +125,15 @@ export class Tracer implements api.Tracer {

let clock: AnchoredClock | undefined;
if (parentSpan) {
clock = this.clocks.get(parentSpan);
clock = (parentSpan as any)["_clock"];
}

clock = clock ?? AnchoredClock.create(Date, otperformance);
if (!clock) {
clock = new AnchoredClock(Date, otperformance);
if (parentSpan) {
(parentSpan as any)["_clock"] = clock;
}
}

const span = new Span(
this,
Expand Down

0 comments on commit f23b868

Please sign in to comment.