Skip to content
This repository has been archived by the owner on Oct 3, 2023. It is now read-only.

Tracing: Add code and message to Span -> Status #328

Merged
merged 3 commits into from
Feb 6, 2019
Merged
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
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,14 @@ All notable changes to this project will be documented in this file.
- Add ignoreIncomingPaths and ignoreOutgoingUrls support to the http and https tracing instrumentations.
- Add ```opencensus-resource-util``` to auto detect AWS, GCE and Kubernetes(K8S) monitored resource, based on the environment where the application is running.
- Add optional `uncompressedSize` and `compressedSize` fields to `MessageEvent` interface.
- Add a ```setStatus``` method in the Span.

**This release has multiple breaking changes. Please test your code accordingly after upgrading.**
Copy link
Contributor

Choose a reason for hiding this comment

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

Should you mention that the status field on Span is no longer a number, since that's a breaking change?

Copy link
Member Author

Choose a reason for hiding this comment

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

Done


- Modify `Logger` interface: `level` made optional, `silly` removed.
- The ```new Stats()``` has been deprecated on Stats class. The global singleton ```globalStats``` object should be used instead. Also, ```registerView()``` is separated out from ```createView()```.
- Use ```TagKey```, ```TagValue``` and ```TagMap``` to create the tag keys, tag values.
- The `status` field on `Span` is no longer a number, use `CanonicalCode` instead.

##### Old code
```js
Expand Down
14 changes: 13 additions & 1 deletion packages/opencensus-core/src/trace/model/span-base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@ import {Clock} from '../../internal/clock';
import {randomSpanId} from '../../internal/util';
import * as types from './types';

const STATUS_OK = {
code: types.CanonicalCode.OK
};

/** Defines a base model for spans. */
export abstract class SpanBase implements types.Span {
Expand Down Expand Up @@ -52,7 +55,7 @@ export abstract class SpanBase implements types.Span {
/** Kind of span. */
kind: string = null;
/** A final status for this span */
status: number;
status: types.Status = STATUS_OK;
/** set isRootSpan */
abstract get isRootSpan(): boolean;

Expand Down Expand Up @@ -183,6 +186,15 @@ export abstract class SpanBase implements types.Span {
} as types.MessageEvent);
}

/**
* Sets a status to the span.
* @param code The canonical status code.
* @param message optional A developer-facing error message.
*/
setStatus(code: types.CanonicalCode, message?: string) {
this.status = {code, message};
}

/** Starts the span. */
start() {
if (this.started) {
Expand Down
155 changes: 154 additions & 1 deletion packages/opencensus-core/src/trace/model/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,152 @@ export interface Attributes {
[attributeKey: string]: string|number|boolean;
}

/**
* The status of a Span by providing a standard CanonicalCode in conjunction
* with an optional descriptive message.
*/
export interface Status {
/** The canonical code of this message. */
code: CanonicalCode;
/** A developer-facing error message. */
message?: string;
}

Copy link
Contributor

Choose a reason for hiding this comment

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

What would you think about adding an enum with the list of OpenCensus status codes from the table referenced above?

Copy link
Contributor

Choose a reason for hiding this comment

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

To be clear I'm not suggesting the type of the code field be the enum (that might limit clients of the library if a new status is added but they can't upgrade yet, and for opencensus-web I want to keep @opencensus/core just as a dev dependency). But just having the enum provides documentation for what the possible canonical status codes are.

Copy link
Member Author

Choose a reason for hiding this comment

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

I checked in Java implementation, we are using enum for code -> https://github.com/census-instrumentation/opencensus-java/blob/master/api/src/main/java/io/opencensus/trace/Status.java#L384. This is also matching with status spec.

Copy link
Contributor

Choose a reason for hiding this comment

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

OK, I'd be fine with using an enum for it.

/** An enumeration of canonical status codes. */
export enum CanonicalCode {
/**
* Not an error; returned on success
*/
OK = 0,
/**
* The operation was cancelled (typically by the caller).
*/
CANCELLED = 1,
/**
* Unknown error. An example of where this error may be returned is
* if a status value received from another address space belongs to
* an error-space that is not known in this address space. Also
* errors raised by APIs that do not return enough error information
* may be converted to this error.
*/
UNKNOWN = 2,
/**
* Client specified an invalid argument. Note that this differs
* from FAILED_PRECONDITION. INVALID_ARGUMENT indicates arguments
* that are problematic regardless of the state of the system
* (e.g., a malformed file name).
*/
INVALID_ARGUMENT = 3,
/**
* Deadline expired before operation could complete. For operations
* that change the state of the system, this error may be returned
* even if the operation has completed successfully. For example, a
* successful response from a server could have been delayed long
* enough for the deadline to expire.
*/
DEADLINE_EXCEEDED = 4,
/**
* Some requested entity (e.g., file or directory) was not found.
*/
NOT_FOUND = 5,
/**
* Some entity that we attempted to create (e.g., file or directory)
* already exists.
*/
ALREADY_EXISTS = 6,
/**
* The caller does not have permission to execute the specified
* operation. PERMISSION_DENIED must not be used for rejections
* caused by exhausting some resource (use RESOURCE_EXHAUSTED
* instead for those errors). PERMISSION_DENIED must not be
* used if the caller can not be identified (use UNAUTHENTICATED
* instead for those errors).
*/
PERMISSION_DENIED = 7,
/**
* Some resource has been exhausted, perhaps a per-user quota, or
* perhaps the entire file system is out of space.
*/
RESOURCE_EXHAUSTED = 8,
/**
* Operation was rejected because the system is not in a state
* required for the operation's execution. For example, directory
* to be deleted may be non-empty, an rmdir operation is applied to
* a non-directory, etc.
*
* A litmus test that may help a service implementor in deciding
* between FAILED_PRECONDITION, ABORTED, and UNAVAILABLE:
*
* - Use UNAVAILABLE if the client can retry just the failing call.
* - Use ABORTED if the client should retry at a higher-level
* (e.g., restarting a read-modify-write sequence).
* - Use FAILED_PRECONDITION if the client should not retry until
* the system state has been explicitly fixed. E.g., if an "rmdir"
* fails because the directory is non-empty, FAILED_PRECONDITION
* should be returned since the client should not retry unless
* they have first fixed up the directory by deleting files from it.
* - Use FAILED_PRECONDITION if the client performs conditional
* REST Get/Update/Delete on a resource and the resource on the
* server does not match the condition. E.g., conflicting
* read-modify-write on the same resource.
*/
FAILED_PRECONDITION = 9,
/**
* The operation was aborted, typically due to a concurrency issue
* like sequencer check failures, transaction aborts, etc.
*
* See litmus test above for deciding between FAILED_PRECONDITION,
* ABORTED, and UNAVAILABLE.
*/
ABORTED = 10,
/**
* Operation was attempted past the valid range. E.g., seeking or
* reading past end of file.
*
* Unlike INVALID_ARGUMENT, this error indicates a problem that may
* be fixed if the system state changes. For example, a 32-bit file
* system will generate INVALID_ARGUMENT if asked to read at an
* offset that is not in the range [0,2^32-1], but it will generate
* OUT_OF_RANGE if asked to read from an offset past the current
* file size.
*
* There is a fair bit of overlap between FAILED_PRECONDITION and
* OUT_OF_RANGE. We recommend using OUT_OF_RANGE (the more specific
* error) when it applies so that callers who are iterating through
* a space can easily look for an OUT_OF_RANGE error to detect when
* they are done.
*/
OUT_OF_RANGE = 11,
/**
* Operation is not implemented or not supported/enabled in this service.
*/
UNIMPLEMENTED = 12,
/**
* Internal errors. Means some invariants expected by underlying
* system has been broken. If you see one of these errors,
* something is very broken.
*/
INTERNAL = 13,
/**
* The service is currently unavailable. This is a most likely a
* transient condition and may be corrected by retrying with
* a backoff.
*
* See litmus test above for deciding between FAILED_PRECONDITION,
* ABORTED, and UNAVAILABLE.
*/
UNAVAILABLE = 14,
/**
* Unrecoverable data loss or corruption.
*/
DATA_LOSS = 15,
/**
* The request does not have valid authentication credentials for the
* operation.
*/
UNAUTHENTICATED = 16,
}

/** A text annotation with a set of attributes. */
export interface Annotation {
/** A user-supplied message describing the event. */
Expand Down Expand Up @@ -124,7 +270,7 @@ export interface Span {
logger: loggerTypes.Logger;

/** A final status for this span */
status: number;
status: Status;

/** A set of attributes, each in the format [KEY]:[VALUE] */
attributes: Attributes;
Expand Down Expand Up @@ -209,6 +355,13 @@ export interface Span {
*/
addMessageEvent(type: string, id: string, timestamp?: number): void;

/**
* Sets a status to the span.
* @param code The canonical status code.
* @param message optional A developer-facing error message.
*/
setStatus(code: CanonicalCode, message?: string): void;

/** Starts a span. */
start(): void;

Expand Down
28 changes: 28 additions & 0 deletions packages/opencensus-core/test/test-span.ts
Original file line number Diff line number Diff line change
Expand Up @@ -258,4 +258,32 @@ describe('Span', () => {
assert.ok(instanceOfLink(span.messageEvents[0]));
});
});

describe('setStatus()', () => {
it('should return default status', () => {
const rootSpan = new RootSpan(tracer);
rootSpan.start();

const span = new Span(rootSpan);
span.start();

assert.equal(rootSpan.status.code, 0);
assert.equal(rootSpan.status.message, null);
assert.equal(span.status.code, 0);
assert.equal(span.status.message, null);
});

it('should set an error status', () => {
const rootSpan = new RootSpan(tracer);
rootSpan.start();
const span = new Span(rootSpan);
span.start();
span.setStatus(types.CanonicalCode.PERMISSION_DENIED, 'This is an error');

assert.equal(rootSpan.status.code, 0);
assert.equal(rootSpan.status.message, null);
assert.equal(span.status.code, 7);
assert.equal(span.status.message, 'This is an error');
});
});
});
2 changes: 1 addition & 1 deletion packages/opencensus-exporter-instana/src/instana.ts
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ export class InstanaTraceExporter implements Exporter {
duration: span.duration,
name: span.name,
type: spanKindTranslation[span.kind] || span.kind,
error: span.status != null && span.status !== 0,
error: span.status != null && span.status.code !== 0,
data: Object.keys(span.attributes)
.reduce(
(agg: {[k: string]: string}, key) => {
Expand Down
11 changes: 1 addition & 10 deletions packages/opencensus-exporter-ocagent/src/adapters.ts
Original file line number Diff line number Diff line change
Expand Up @@ -193,15 +193,6 @@ const adaptTimeEvents =
};
};

/**
* Adapts a statusCode number to a `opencensus.proto.trace.v1.Status` type.
* @param statusCode number
* @returns opencensus.proto.trace.v1.Status
*/
const adaptStatus = (statusCode: number): opencensus.proto.trace.v1.Status => {
return {code: statusCode, message: null};
};

/**
* Adapts a traceState string to a `opencensus.proto.trace.v1.Span.Tracestate`
* type. The tracestate is a comma-delimited set of equals-delimited key-value
Expand Down Expand Up @@ -285,7 +276,7 @@ export const adaptSpan = (span: Span): opencensus.proto.trace.v1.Span => {
stackTrace: null, // Unsupported by nodejs
timeEvents: adaptTimeEvents(span.annotations, span.messageEvents),
links: adaptLinks(span.links),
status: adaptStatus(span.status),
status: span.status,
sameProcessAsParentSpan: adaptBoolean(!span.remoteParent),
childSpanCount: null,
};
Expand Down
11 changes: 5 additions & 6 deletions packages/opencensus-exporter-ocagent/test/test-ocagent.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
*/

import * as protoLoader from '@grpc/proto-loader';
import {RootSpan, TraceOptions, Tracing} from '@opencensus/core';
import {CanonicalCode, RootSpan, TraceOptions, Tracing} from '@opencensus/core';
import * as nodeTracing from '@opencensus/nodejs';
import * as assert from 'assert';
import {EventEmitter} from 'events';
Expand Down Expand Up @@ -328,7 +328,7 @@ describe('OpenCensus Agent Exporter', () => {

tracing.tracer.startRootSpan(rootSpanOptions, (rootSpan: RootSpan) => {
// Status
rootSpan.status = 200;
rootSpan.setStatus(CanonicalCode.OK);

// Attribute
rootSpan.addAttribute('my_attribute_string', 'bar2');
Expand Down Expand Up @@ -385,12 +385,11 @@ describe('OpenCensus Agent Exporter', () => {
span.tracestate.entries,
[{key: 'foo', value: 'bar'}, {key: 'baz', value: 'buzz'}]);

// Status
if (!span.status) {
assert.fail('span.status is null or undefined');
return;
} else {
assert.deepEqual(span.status, {code: 0, message: ''});
}
assert.equal(span.status.code, 200);

// Attributes
if (!span.attributes) {
Expand Down Expand Up @@ -507,4 +506,4 @@ describe('OpenCensus Agent Exporter', () => {
rootSpan.end();
});
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ export class TracezPageHandler {

// Building span list
for (const span of spans) {
if (span.status && span.status !== 0) {
if (span.status && span.status.code !== 0) {
spanCell.ERRORS += 1;
} else if (span.ended) {
const durationNs =
Expand Down Expand Up @@ -208,14 +208,14 @@ export class TracezPageHandler {
if (this.traceMap.has(params.tracename)) {
for (const span of this.traceMap.get(params.tracename)!) {
if (params.type === 'ERRORS') {
if (span.status !== 0) {
if (span.status.code !== 0) {
traceList.push(span);
}
} else if (params.type === 'RUNNING') {
if (span.started && !span.ended) {
traceList.push(span);
}
} else if (span.ended && span.status === 0) {
} else if (span.ended && span.status.code === 0) {
const durationNs =
LatencyBucketBoundaries.millisecondsToNanos(span.duration);
const latency =
Expand Down
8 changes: 0 additions & 8 deletions packages/opencensus-exporter-zpages/src/zpages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -139,17 +139,9 @@ export class ZpagesExporter implements Exporter, StatsEventListener {
* @param trace the rootSpan to be sent to the array list
*/
private sendTrace(trace: RootSpan) {
/** If there is no status, put status 0 (OK) */
if (!trace.status) {
trace.status = 0;
}
this.pushSpan(trace);

for (const span of trace.spans) {
/** If there is no status, put status 0 (OK) */
if (!span.status) {
span.status = 0;
}
this.pushSpan(span);
}
this.logger.debug('Z-PAGES: trace added');
Expand Down
Loading