Skip to content
This repository was archived by the owner on Oct 3, 2023. It is now read-only.
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
8 changes: 8 additions & 0 deletions packages/opencensus-core/src/trace/model/span-base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,9 @@ export abstract class SpanBase implements types.Span {
/** The number of dropped attributes. */
droppedAttributesCount = 0;

/** The number of dropped links. */
droppedLinksCount = 0;

/** Constructs a new SpanBaseModel instance. */
constructor() {
this.className = this.constructor.name;
Expand Down Expand Up @@ -180,6 +183,11 @@ export abstract class SpanBase implements types.Span {
addLink(
traceId: string, spanId: string, type: string,
attributes?: types.Attributes) {
if (this.links.length >= this.activeTraceParams.numberOfLinksPerSpan) {
this.links.shift();
this.droppedLinksCount++;
}

this.links.push({
'traceId': traceId,
'spanId': spanId,
Expand Down
7 changes: 5 additions & 2 deletions packages/opencensus-core/src/trace/model/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -320,11 +320,14 @@ export interface Span {
/** Gives the TraceContext of the span. */
readonly spanContext: SpanContext;

/** Trace Parameters */
activeTraceParams: configTypes.TraceParams;

/** The number of dropped attributes. */
droppedAttributesCount: number;

/** Trace Parameters */
activeTraceParams: configTypes.TraceParams;
/** The number of dropped links. */
droppedLinksCount: number;

/**
* Adds an atribute to the span.
Expand Down
1 change: 1 addition & 0 deletions packages/opencensus-core/test/test-root-span.ts
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,7 @@ describe('RootSpan', () => {
rootSpan.addLink(rootSpan.traceId, span.id, LINK_TYPE);

assert.ok(rootSpan.links.length > 0);
assert.equal(rootSpan.droppedLinksCount, 0);
assert.ok(instanceOfLink(rootSpan.links[0]));
});
});
Expand Down
34 changes: 34 additions & 0 deletions packages/opencensus-core/test/test-span.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,10 @@ import {Annotation, Attributes, Link} from '../src/trace/model/types';
// rootspan

const tracer = new CoreTracer();
tracer.activeTraceParams = {
numberOfAttributesPerSpan: 32,
numberOfLinksPerSpan: 32
};

describe('Span', () => {
/**
Expand Down Expand Up @@ -186,6 +190,20 @@ describe('Span', () => {
span.attributes['testKey' + attType], 'testValue' + attType);
});
});

it('should drop extra attributes', () => {
const rootSpan = new RootSpan(tracer);
rootSpan.start();

const span = new Span(rootSpan);
span.start();
for (let i = 0; i < 40; i++) {
span.addAttribute('attr' + i, 100);
}

assert.equal(Object.keys(span.attributes).length, 32);
assert.equal(span.droppedAttributesCount, 8);
});
});

/**
Expand Down Expand Up @@ -232,8 +250,24 @@ describe('Span', () => {
span.addLink(span.traceId, rootSpan.id, LINK_TYPE);

assert.ok(span.links.length > 0);
assert.equal(span.droppedLinksCount, 0);
assert.ok(instanceOfLink(span.links[0]));
});

it('should drop extra links', () => {
const rootSpan = new RootSpan(tracer);
rootSpan.start();
const span = new Span(rootSpan);
span.start();

const LINK_TYPE = 'PARENT_LINKED_SPAN';
for (let i = 0; i < 35; i++) {
span.addLink(span.traceId, rootSpan.id, LINK_TYPE);
}

assert.equal(span.links.length, 32);
assert.equal(span.droppedLinksCount, 3);
});
});

/**
Expand Down
11 changes: 5 additions & 6 deletions packages/opencensus-exporter-ocagent/src/adapters.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
*/

import {Annotation, Attributes, Link, MessageEvent, RootSpan, Span} from '@opencensus/core';

import {google, opencensus} from './types';

/**
Expand Down Expand Up @@ -246,10 +245,10 @@ const adaptLink = (link: Link): opencensus.proto.trace.v1.Span.Link => {
* @param links Link[]
* @returns opencensus.proto.trace.v1.Span.Links
*/
const adaptLinks =
(links: Link[] = []): opencensus.proto.trace.v1.Span.Links => {
return {link: links.map(adaptLink), droppedLinksCount: null};
};
const adaptLinks = (links: Link[] = [], droppedLinksCount: number):
opencensus.proto.trace.v1.Span.Links => {
return {link: links.map(adaptLink), droppedLinksCount};
};

/**
* Adapts a boolean to a `google.protobuf.BoolValue` type.
Expand All @@ -276,7 +275,7 @@ export const adaptSpan = (span: Span): opencensus.proto.trace.v1.Span => {
attributes: adaptAttributes(span.attributes, span.droppedAttributesCount),
stackTrace: null, // Unsupported by nodejs
timeEvents: adaptTimeEvents(span.annotations, span.messageEvents),
links: adaptLinks(span.links),
links: adaptLinks(span.links, span.droppedLinksCount),
status: span.status,
sameProcessAsParentSpan: adaptBoolean(!span.remoteParent),
childSpanCount: null,
Expand Down
179 changes: 176 additions & 3 deletions packages/opencensus-exporter-ocagent/test/test-ocagent.ts
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ describe('OpenCensus Agent Exporter', () => {
tracing = nodeTracing.start({
exporter: ocAgentExporter,
samplingRate: INITIAL_SAMPLER_PROBABILITY,
traceParams: {numberOfAttributesPerSpan: 4}
traceParams: {numberOfAttributesPerSpan: 4, numberOfLinksPerSpan: 3}
});
});

Expand Down Expand Up @@ -345,7 +345,7 @@ describe('OpenCensus Agent Exporter', () => {
rootSpan.addAnnotation(
'my_annotation', {myString: 'bar', myNumber: 123, myBoolean: true});

// Metric Event
// Message Event
const timeStamp = 123456789;
rootSpan.addMessageEvent('MessageEventTypeSent', 'ffff', timeStamp);
rootSpan.addMessageEvent('MessageEventTypeRecv', 'ffff', timeStamp);
Expand All @@ -354,6 +354,8 @@ describe('OpenCensus Agent Exporter', () => {
rootSpan.addMessageEvent(null as any, 'ffff', timeStamp);

// Links
rootSpan.addLink('aaaaa', 'aaa', 'CHILD_LINKED_SPAN');
rootSpan.addLink('bbbbb', 'bbbbb', 'CHILD_LINKED_SPAN');
rootSpan.addLink('ffff', 'ffff', 'CHILD_LINKED_SPAN', {
'child_link_attribute_string': 'foo1',
'child_link_attribute_number': 123,
Expand Down Expand Up @@ -476,7 +478,7 @@ describe('OpenCensus Agent Exporter', () => {
// Links
const buff = Buffer.from([255, 255]);
assert.deepEqual(span.links, {
droppedLinksCount: 0,
droppedLinksCount: 2,
link: [
{
type: 'CHILD_LINKED_SPAN',
Expand Down Expand Up @@ -517,4 +519,175 @@ describe('OpenCensus Agent Exporter', () => {
rootSpan.end();
});
});

it('should adapt a span correctly without overflowing trace param limits',
(done) => {
const rootSpanOptions: TraceOptions = {
name: 'root',
kind: 'SERVER',
spanContext: {
traceId: hexId(),
spanId: hexId(),
traceState: 'foo=bar,baz=buzz',
options: 0x1
}
};

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

// Attribute
rootSpan.addAttribute('my_first_attribute', 'foo');
rootSpan.addAttribute('my_second_attribute', 'foo2');

// Annotation
rootSpan.addAnnotation(
'my_annotation',
{myString: 'bar', myNumber: 123, myBoolean: true});

// Message Event
const timeStamp = 123456789;
rootSpan.addMessageEvent('MessageEventTypeSent', 'ffff', timeStamp);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Curious, why is this 'MessageEventTypeSent' and not just 'SENT'?

This would be for a follow up PR, but what would you think about adding enums for span kind, message event type and link type?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This is legacy code, I am not sure rational behind this. Created an issue to track this -> #333. Thanks 👍

rootSpan.addMessageEvent('MessageEventTypeRecv', 'ffff', timeStamp);

// Links
rootSpan.addLink('ffff', 'ffff', 'CHILD_LINKED_SPAN', {
'child_link_attribute_string': 'foo1',
'child_link_attribute_number': 123,
'child_link_attribute_boolean': true,
});
rootSpan.addLink('ffff', 'ffff', 'PARENT_LINKED_SPAN');

server.on(
MockAgentEvent.ExportStreamMessageReceived,
(message: opencensus.proto.agent.trace.v1
.ExportTraceServiceRequest) => {
assert.equal(message.spans.length, 1);
const span = message.spans[0];
// Name / Context
if (!span.name) {
assert.fail('span.name is null or undefined');
return;
}
assert.equal(span.name.value, 'root');
assert.equal(span.kind, 'SERVER');

if (!span.tracestate) {
assert.fail('span.tracestate is null or undefined');
return;
}
assert.deepEqual(
span.tracestate.entries,
[{key: 'foo', value: 'bar'}, {key: 'baz', value: 'buzz'}]);

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

// Attributes
if (!span.attributes) {
assert.fail('span.attributes is null or undefined');
return;
}
assert.deepEqual(span.attributes.attributeMap, {
my_first_attribute: {
value: 'stringValue',
stringValue: {value: 'foo', truncatedByteCount: 0}
},
my_second_attribute: {
value: 'stringValue',
stringValue: {value: 'foo2', truncatedByteCount: 0}
}
});
assert.equal(span.attributes.droppedAttributesCount, 0);

// Time Events
assert.deepEqual(span.timeEvents, {
droppedAnnotationsCount: 0,
droppedMessageEventsCount: 0,
timeEvent: [
{
value: 'annotation',
time: null,
annotation: {
description:
{value: 'my_annotation', truncatedByteCount: 0},
attributes: {
attributeMap: {
myString: {
value: 'stringValue',
stringValue:
{value: 'bar', truncatedByteCount: 0}
},
myNumber: {value: 'intValue', intValue: '123'},
myBoolean: {value: 'boolValue', boolValue: true}
},
droppedAttributesCount: 0
}
}
},
{
messageEvent: {
compressedSize: '0',
id: '65535',
type: 'SENT',
uncompressedSize: '0'
},
time: {seconds: '123456', nanos: 789000000},
value: 'messageEvent'
},
{
value: 'messageEvent',
messageEvent: {
compressedSize: '0',
id: '65535',
type: 'RECEIVED',
uncompressedSize: '0'
},
time: {seconds: '123456', nanos: 789000000},
}
]
});

// Links
const buff = Buffer.from([255, 255]);
assert.deepEqual(span.links, {
droppedLinksCount: 0,
link: [
{
type: 'CHILD_LINKED_SPAN',
traceId: buff,
spanId: buff,
attributes: {
droppedAttributesCount: 0,
attributeMap: {
child_link_attribute_string: {
value: 'stringValue',
stringValue: {value: 'foo1', truncatedByteCount: 0}
},
child_link_attribute_number:
{value: 'intValue', intValue: '123'},
child_link_attribute_boolean:
{value: 'boolValue', boolValue: true}
}
}
},
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

you have checked the drop counts for links earlier but it would nice to check that oldest links is removed. you can either add this is as part of this or where you test the dropLinkCount.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Good Point! We already have asset check here https://github.com/census-instrumentation/opencensus-node/pull/331/files#diff-0ab7878c00c0a6393dd66fd59e796156R482 for the links. Basically, it doesn't contain a dropped link object.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

as per our conversation, this is already taken care of in previous test 'should adapt a span correctly' on line 320.
so ignore the comment.

{
type: 'PARENT_LINKED_SPAN',
traceId: buff,
spanId: buff,
attributes: null
}
]
});

done();
});

rootSpan.end();
});
});
});