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 Tracer API #85

Merged
merged 10 commits into from
Jul 9, 2019
15 changes: 15 additions & 0 deletions packages/opentelemetry-types/src/trace/span_data.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
/**
* 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.
*/
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 not required for now, Could you please remove this file?

83 changes: 83 additions & 0 deletions packages/opentelemetry-types/src/trace/tracer.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
/**
* 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 { Span } from './span';
import { Attributes } from './attributes';

/**
* Options needed for span creation
*
* @todo: Move into module of its own
*/
export interface SpanOptions {
Copy link
Member

Choose a reason for hiding this comment

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

Could you please add an optional kind of type SpanKind, IsRecordingEvents boolean flag and parent span?

something like:

export interface SpanOptions {
    kind?: SpanKind;
    attributes?: Attributes;
    isRecordingEvents?: boolean;
    parent?: Span | SpanContext; (or childOf?: Span | SpanContext;)
    startTime?: number;
}

WDYT?

attributes: Attributes[];
danielkhan marked this conversation as resolved.
Show resolved Hide resolved
}

export interface Tracer {
danielkhan marked this conversation as resolved.
Show resolved Hide resolved
/**
* Returns the current Span from the current context if available.
*
* If there is no Span associated with the current context, a default Span
* with invalid SpanContext is returned.
*
* @returns Span
*/
getCurrentSpan(): Span;

/**
*
danielkhan marked this conversation as resolved.
Show resolved Hide resolved
* @param name
* @param options
danielkhan marked this conversation as resolved.
Show resolved Hide resolved
*/
start(name: string, options: SpanOptions): Span;

/**
* Executes the function given by fn within the context provided by Span
*
* @param span The span that provides the context
* @param operation The name of the function
* @param fn The function to be eexcuted inside the provided context
*/
withSpan<T extends (...args: unknown[]) => unknown>(
span: Span,
operation: string,
danielkhan marked this conversation as resolved.
Show resolved Hide resolved
fn: T
): ReturnType<T>;

/**
* Send a pre-populated span object to the exporter.
* Sampling and recording decisions as well as other collection optimizations
* are the responsibility of a caller.
*
* @todo: Pending API discussion. Revisit if Span or SpanData should be passed
* in here once this is sorted out.
* @param span
*/
recordSpanData(span: Span): void;

/**
* Returns the binary format interface which can serialize/deserialize Spans.
* @todo: Change return type once BinaryFormat is available
*/
getBinaryFormat(): unknown;

/**
* Returns the HTTP text format interface which can inject/extract Spans.
*
* @todo: Change return type once HttpTextFormat is available
*/
getHttpTextFormat: unknown;
Copy link
Member

Choose a reason for hiding this comment

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

This and getBinaryFormat can be replaced with inject and extract which are compatible with both as explained in #74 (comment).

The problem with having fixed methods by propagators is that it becomes impossible to add new formats which may be needed by vendors.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is no agreement on this as far as I can see and as long as it's in the spec I will leave it in here. I think you should post such concerns in the spec repo.

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 clearly due to a limitation in Java. The spec specifies 2 formats which would be available using inject and extract.

Copy link
Contributor

Choose a reason for hiding this comment

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

I would also prefer that because I think it would be nice to have the browser implementation of this not need code for the binary encoding if it doesn't use it (browser mostly speaks HTTP and can't do gRPC directly).

Copy link
Member

Choose a reason for hiding this comment

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

}