Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add TraceState #57

Merged
merged 9 commits into from
Jul 1, 2019
Merged
Show file tree
Hide file tree
Changes from 8 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
85 changes: 85 additions & 0 deletions packages/opentelemetry-core/src/trace/TraceState.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
/**
* Copyright 2019, OpenTelemetry Authors
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

import * as types from '@opentelemetry/types';

// TODO validate maximum number of items
// const MAX_TRACE_STATE_ITEMS = 32;

const MAX_TRACE_STATE_LEN = 512;
const LIST_MEMBERS_SEPARATOR = ',';
const LIST_MEMBER_KEY_VALUE_SPLITTER = '=';

/**
* TraceState must be a class and not a simple object type because of the spec
* requirement (https://www.w3.org/TR/trace-context/#tracestate-field).
*
* Here is the list of allowed mutations:
* - New key-value pair should be added into the beginning of the list
* - The value of any key can be updated. Modified keys MUST be moved to the
* beginning of the list.
*/
export class TraceState implements types.TraceState {
mayurkale22 marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Member

Choose a reason for hiding this comment

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

Where will this be stored? It also looks like this conflicts with DistributedContext as this is the equivalent of one of the class described in open-telemetry/opentelemetry-specification#140?

private _internalState: Map<string, string> = new Map();

constructor(rawTraceState?: string) {
if (rawTraceState) this._parse(rawTraceState);
}

set(key: string, value: string): void {
// TODO: Benchmark the different approaches(map vs list) and
// use the faster one.
if (this._internalState.has(key)) this._internalState.delete(key);
this._internalState.set(key, value);
}

unset(key: string): void {
this._internalState.delete(key);
}

get(key: string): string | undefined {
return this._internalState.get(key);
}

serialize(): string {
Copy link
Contributor

Choose a reason for hiding this comment

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

We had talked today in the SIG about doing some kind of encoding/decoding of values. E.g. using URL encoding for the unsupported characters. Is there any kind of encoding format for this specified in the spec?

Copy link
Member

Choose a reason for hiding this comment

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

There isn't in the w3c spec nor the opentelem spec. I think as an instrumentation library we should only attempt to validate the user's input and provide a stern warning (but not exception) that the input is invalid. I wouldn't want to automatically encode the input when it goes over the wire in case they aren't using an opentelem library downstream. I feel like if the user wants to include non spec conforming data and have it accessible downstream, we can recommend using URL encoding.

Copy link
Member

Choose a reason for hiding this comment

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

This is pushing unnecessary complexity on the user. I understand if W3C don't want to enforce a specific format, but I think OpenTelemetry should standardize this.

Copy link
Member

Choose a reason for hiding this comment

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

+1 on OpenTelem standardizing on it. For what it's worth I'm not super stoked on using tracestate to propagate baggage to begin with, definitely interested in seeing how the new w3c correlation context turns out.

Copy link
Contributor

Choose a reason for hiding this comment

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

The intent of tracestate is to encode tracing system specific information and it's up to the tracing system how this is encoded. Some vendors use base64 to send some json, some simply have information like a tenant id in plain text.
At some point Opentelemetry needs to come up with what should be put in there and was there is quite some overlap between the w3c group and this project it will definitely be compliant.

The intent is NOT to make it available to the user to use it for baggage. As @bg451 said, this will be up to another spec.

Copy link
Member

Choose a reason for hiding this comment

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

With your above explanation that each vendor should have a single entry, it makes sense to not require a specific format. However, if there is a entry reserved for OpenTelemetry itself, then we should define the format of that entry.

return this._keys()
.reduce((agg: string[], key) => {
agg.push(key + LIST_MEMBER_KEY_VALUE_SPLITTER + this.get(key));
return agg;
}, [])
.join(LIST_MEMBERS_SEPARATOR);
}

private _parse(rawTraceState: string) {
if (rawTraceState.length > MAX_TRACE_STATE_LEN) return;
// TODO validate maximum number of items
this._internalState = rawTraceState
.split(LIST_MEMBERS_SEPARATOR)
.reverse()
.reduce((agg: Map<string, string>, part: string) => {
const i = part.indexOf(LIST_MEMBER_KEY_VALUE_SPLITTER);
if (i !== -1) {
// TODO validate key/value constraints defined in the spec
agg.set(part.slice(0, i), part.slice(i + 1, part.length));
}
return agg;
}, new Map());
}

private _keys(): string[] {
return Array.from(this._internalState.keys()).reverse();
}
}
87 changes: 87 additions & 0 deletions packages/opentelemetry-core/test/tracestate.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
/**
* Copyright 2019, OpenTelemetry Authors
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

import * as assert from 'assert';
import { TraceState } from '../src/trace/TraceState';

describe('TraceState', () => {
describe('serialize', () => {
it('returns serialize string', () => {
const state = new TraceState('a=1,b=2');
assert.deepStrictEqual(state.serialize(), 'a=1,b=2');
});

it('must replace keys and move them to the front', () => {
const state = new TraceState('a=1,b=2');
state.set('b', '3');
assert.deepStrictEqual(state.serialize(), 'b=3,a=1');
});

it('must add new keys to the front', () => {
const state = new TraceState();
state.set('vendorname1', 'opaqueValue1');
assert.deepStrictEqual(state.serialize(), 'vendorname1=opaqueValue1');

state.set('vendorname2', 'opaqueValue2');
assert.deepStrictEqual(
state.serialize(),
'vendorname2=opaqueValue2,vendorname1=opaqueValue1'
);
});

it('must unset the entries', () => {
const state = new TraceState('c=4,b=3,a=1');
state.unset('b');
assert.deepStrictEqual(state.serialize(), 'c=4,a=1');
state.unset('c');
state.unset('A');
assert.deepStrictEqual(state.serialize(), 'a=1');
});
});

describe('parse', () => {
it('must successfully parse valid state value', () => {
const state = new TraceState(
'vendorname2=opaqueValue2,vendorname1=opaqueValue1'
);
assert.deepStrictEqual(state.get('vendorname1'), 'opaqueValue1');
assert.deepStrictEqual(state.get('vendorname2'), 'opaqueValue2');
assert.deepStrictEqual(
state.serialize(),
'vendorname2=opaqueValue2,vendorname1=opaqueValue1'
);
});

it('must drop states when the items are too long', () => {
const state = new TraceState('a=' + 'b'.repeat(512));
assert.deepStrictEqual(state.get('a'), undefined);
assert.deepStrictEqual(state.serialize(), '');
});

it('must drop states which cannot be parsed', () => {
const state = new TraceState('a=1,b,c=3');
assert.deepStrictEqual(state.get('a'), '1');
assert.deepStrictEqual(state.get('b'), undefined);
assert.deepStrictEqual(state.get('c'), '3');
assert.deepStrictEqual(state.serialize(), 'a=1,c=3');
});

it('must parse states that only have a single value with an equal sign', () => {
const state = new TraceState('a=1=');
assert.deepStrictEqual(state.get('a'), '1=');
});
});
});
46 changes: 45 additions & 1 deletion packages/opentelemetry-types/src/trace/trace_state.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,50 @@
* limitations under the License.
*/

/**
* Tracestate carries system-specific configuration data, represented as a list
* of key-value pairs. TraceState allows multiple tracing systems to
* participate in the same trace.
*/
export interface TraceState {
// TODO
/**
* Adds or updates the TraceState that has the given `key` if it is
* present. The new State will always be added in the front of the
* list of states.
*
* @param key key of the TraceState entry.
* @param value value of the TraceState entry.
*/
set(key: string, value: string): void;
mayurkale22 marked this conversation as resolved.
Show resolved Hide resolved

/**
* Removes the TraceState Entry that has the given `key` if it is present.
*
* @param key the key for the TraceState Entry to be removed.
*/
unset(key: string): void;

/**
* Returns the value to which the specified key is mapped, or `undefined` if
* this map contains no mapping for the key.
*
* @param key with which the specified value is to be associated.
* @returns the value to which the specified key is mapped, or `undefined` if
* this map contains no mapping for the key.
*/
get(key: string): string | undefined;

// TODO: Consider to add support for merging an object as well by also
// accepting a single internalTraceState argument similar to the constructor.

/**
* Serializes the TraceState to a `list` as defined below. The `list` is a
* series of `list-members` separated by commas `,`, and a list-member is a
* key/value pair separated by an equals sign `=`. Spaces and horizontal tabs
* surrounding `list-members` are ignored. There can be a maximum of 32
* `list-members` in a `list`.
*
* @returns the serialized string.
*/
serialize(): string;
}