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

[telemetry] Centralize (as much as is practical) the creation of spans to ease upgrades #13887

Merged
merged 55 commits into from
Feb 26, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
55 commits
Select commit Hold shift + click to select a range
2ccc04f
Centralize createSpan code into core-tracing, update the minimum need…
richardpark-msft Feb 19, 2021
d00744f
Formatting
richardpark-msft Feb 19, 2021
96f7c97
Updating the package reference to be the new core-tracing version (wh…
richardpark-msft Feb 19, 2021
13d5b35
Formatting
richardpark-msft Feb 19, 2021
450e1ba
core-http compatible!
richardpark-msft Feb 19, 2021
88b5783
Formatting
richardpark-msft Feb 19, 2021
724092a
Centralize most of the createSpan logic (except for keyvault, which i…
richardpark-msft Feb 19, 2021
7092a57
Formatting
richardpark-msft Feb 19, 2021
df7f0e1
Service Bus was using the properties from createSpan() with a differe…
richardpark-msft Feb 19, 2021
cca3325
Formatting
richardpark-msft Feb 19, 2021
89da87e
Remove unneeded OperationTracingOptionsLike (the same package already…
richardpark-msft Feb 20, 2021
fd10725
Formatting
richardpark-msft Feb 20, 2021
b00a413
identity: the deconstructed names are different (and null checks need…
richardpark-msft Feb 20, 2021
ee2b382
Formatting
richardpark-msft Feb 20, 2021
2aea35c
Update storage-blob for the pending update to the latest OpenTelemetr…
richardpark-msft Feb 20, 2021
4dc72d8
Formatting
richardpark-msft Feb 20, 2021
42ff058
Update to use the core-tracing version of createSpan
richardpark-msft Feb 20, 2021
6f99488
Formatting
richardpark-msft Feb 20, 2021
ee8c6f0
Formatting
richardpark-msft Feb 20, 2021
7edc5dd
storage-queue: same story, just swapping to core-tracing's versions.
richardpark-msft Feb 20, 2021
63b013d
storage-file-datalike now using the core-tracing functions
richardpark-msft Feb 20, 2021
68c5ce0
Formatting
richardpark-msft Feb 20, 2021
7031b79
storage-file-share: ported to using core-tracing
richardpark-msft Feb 20, 2021
4bd5363
Formatting
richardpark-msft Feb 20, 2021
b9c51e6
Removing export of core-tracing functions from core-client and core-http
richardpark-msft Feb 22, 2021
6365b72
Formatting
richardpark-msft Feb 22, 2021
f645aec
Merge remote-tracking branch 'upstream/master' into ot-upgrading-step1
richardpark-msft Feb 22, 2021
4b0e99e
- createSpa now guarantees it's return value is not null (eliminates …
richardpark-msft Feb 22, 2021
3404d31
Formatting
richardpark-msft Feb 22, 2021
7887702
synapse-access-control: use core-tracing createSpan
richardpark-msft Feb 22, 2021
5a71f92
Formatting
richardpark-msft Feb 22, 2021
e86b11d
synapse-managed-privateendpoints: update to use core-tracing
richardpark-msft Feb 22, 2021
36e30c1
Formatting
richardpark-msft Feb 22, 2021
5e3fada
synapse-monitoring: change to use core-tracing
richardpark-msft Feb 22, 2021
fc4e05f
synapse-spark: changed to use core-tracing
richardpark-msft Feb 22, 2021
3785e5e
Formatting
richardpark-msft Feb 22, 2021
00cbaba
Updated to use core-tracing directly
richardpark-msft Feb 22, 2021
81d17b7
our semver rule was tripping over using preview.10 as the version. Ju…
richardpark-msft Feb 22, 2021
b48f98c
Bring back a compatible createSpan function so we don't cause runtime…
richardpark-msft Feb 23, 2021
5cf9288
After talking with @joheredi it's obvious we can't remove this functi…
richardpark-msft Feb 23, 2021
0b5e2d6
Formatting
richardpark-msft Feb 23, 2021
bd1a4e8
Change name from SpanConfig (which is too generic!) to CreateSpanFunc…
richardpark-msft Feb 23, 2021
b168a32
Formatting again.
richardpark-msft Feb 23, 2021
bed4e50
Updating changelog for core-client to note that createSpanFunction ha…
richardpark-msft Feb 23, 2021
af57992
Renamed SpanOptions to CreateSpanFunctionArgs
richardpark-msft Feb 23, 2021
bf06056
Simplified the signature for createSpanFunction
richardpark-msft Feb 23, 2021
0e3b80f
Formatting
richardpark-msft Feb 23, 2021
4900eb1
Formalizing the deprecated'ness of the core-http exports by adding in…
richardpark-msft Feb 23, 2021
caf7f63
Formatting
richardpark-msft Feb 23, 2021
7f7ecc5
Formatting of the breaking changes was incorrect.
richardpark-msft Feb 23, 2021
fe61219
Merge remote-tracking branch 'upstream/master' into ot-upgrading-step1
richardpark-msft Feb 24, 2021
d8dd23b
Merge remote-tracking branch 'upstream/master' into ot-upgrading-step1
richardpark-msft Feb 24, 2021
8f62518
Removed unused import
richardpark-msft Feb 24, 2021
fce3238
Remove unused functions.
richardpark-msft Feb 24, 2021
72d141f
Remove two unused imports
richardpark-msft Feb 24, 2021
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
Next Next commit
Centralize createSpan code into core-tracing, update the minimum need…
…ed for core-tracing to compile and test
  • Loading branch information
richardpark-msft committed Feb 19, 2021
commit 2ccc04f15ea44ed35a11e7205af5b071402d0ac0
4 changes: 3 additions & 1 deletion sdk/core/core-tracing/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,8 @@
"rollup-plugin-visualizer": "^4.0.4",
"typescript": "4.1.2",
"util": "^0.12.1",
"typedoc": "0.15.2"
"typedoc": "0.15.2",
"sinon": "^9.0.2",
"@types/sinon": "^9.0.4"
}
}
15 changes: 15 additions & 0 deletions sdk/core/core-tracing/review/core-tracing.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,16 @@ import { TimeInput } from '@opentelemetry/api';
import { Tracer } from '@opentelemetry/api';
import { TracerBase } from '@opencensus/web-types';

// Warning: (ae-forgotten-export) The symbol "SpanConfig" needs to be exported by the entry point index.d.ts
//
// @public
export function createSpanFunction({ packagePrefix, namespace }: SpanConfig): <T extends {
tracingOptions?: OperationTracingOptionsLike | undefined;
}>(operationName: string, operationOptions: T) => {
span: Span;
updatedOptions: T;
};

// @public
export function extractSpanContextFromTraceParentHeader(traceParentHeader: string): SpanContext | undefined;

Expand Down Expand Up @@ -79,6 +89,11 @@ export interface OperationTracingOptions {
spanOptions?: SpanOptions;
}

// @public
export interface OperationTracingOptionsLike {
spanOptions?: OTSpanOptions;
}

export { OTSpanContext }

export { OTSpanOptions }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,22 @@
// Licensed under the MIT license.

import { Span, SpanOptions, SpanKind } from "@opentelemetry/api";
import { getTracer } from "@azure/core-tracing";
import { OperationOptions } from "./coreHttp";
import { getTracer } from "../src/tracerProxy";

type OperationTracingOptions = OperationOptions["tracingOptions"];
/**
* Tracing options to set on an operation.
* @hidden
*/
export interface OperationTracingOptionsLike {
/**
* OpenTelemetry SpanOptions used to create a span when tracing is enabled.
*/
spanOptions?: SpanOptions;
}

/**
* Configuration for creating a new Tracing Span
* @hidden
*/
export interface SpanConfig {
richardpark-msft marked this conversation as resolved.
Show resolved Hide resolved
/**
Expand All @@ -28,7 +37,7 @@ export interface SpanConfig {
* @param tracingOptions - The options for the underlying http request.
*/
export function createSpanFunction({ packagePrefix, namespace }: SpanConfig) {
return function<T extends OperationOptions>(
return function<T extends { tracingOptions?: OperationTracingOptionsLike }>(
operationName: string,
operationOptions: T
): { span: Span; updatedOptions: T } {
Expand All @@ -55,9 +64,10 @@ export function createSpanFunction({ packagePrefix, namespace }: SpanConfig) {
};
}

const newTracingOptions: OperationTracingOptions = {
const newTracingOptions: OperationTracingOptionsLike = {
...tracingOptions,
spanOptions: newSpanOptions
// TODO: .context soon.
richardpark-msft marked this conversation as resolved.
Show resolved Hide resolved
richardpark-msft marked this conversation as resolved.
Show resolved Hide resolved
};

const newOperationOptions: T = {
Expand Down
1 change: 1 addition & 0 deletions sdk/core/core-tracing/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ export { OpenCensusSpanWrapper } from "./tracers/opencensus/openCensusSpanWrappe
export { OpenCensusTracerWrapper } from "./tracers/opencensus/openCensusTracerWrapper";
export { TestTracer, SpanGraph, SpanGraphNode } from "./tracers/test/testTracer";
export { TestSpan } from "./tracers/test/testSpan";
export { OperationTracingOptionsLike, createSpanFunction } from "./createSpan";

richardpark-msft marked this conversation as resolved.
Show resolved Hide resolved
// Shared interfaces
export { SpanContext, SpanOptions, TraceFlags } from "./interfaces";
Expand Down
Original file line number Diff line number Diff line change
@@ -1,14 +1,15 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT license.

import { assert } from "chai";
import { SpanKind, TraceFlags } from "@opentelemetry/api";
import { setTracer, TestSpan, TestTracer } from "@azure/core-tracing";
import * as assert from "assert";
import sinon from "sinon";
import { SpanKind, TraceFlags } from "@opentelemetry/api";

import { OperationOptions } from "../src/coreHttp";
import { setTracer } from "../src/tracerProxy";
import { TestTracer } from "../src/tracers/test/testTracer";
import { TestSpan } from "../src/tracers/test/testSpan";
import { createSpanFunction, OperationTracingOptionsLike } from "../src/createSpan";

import { createSpanFunction } from "../src/createSpan";
const createSpan = createSpanFunction({ namespace: "Microsoft.Test", packagePrefix: "Azure.Test" });

describe("createSpan", () => {
Expand All @@ -24,21 +25,44 @@ describe("createSpan", () => {
const startSpanStub = sinon.stub(tracer, "startSpan");
startSpanStub.returns(testSpan);
setTracer(tracer);
const { span } = createSpan("testMethod", {});
const { span, updatedOptions } = createSpan("testMethod", {
tracingOptions: ({
// validate that we dumbly just copy any fields (this makes future upgrades easier)
someOtherField: "someOtherFieldValue",
context: { someContext: "some Context" }
} as OperationTracingOptionsLike) as any
});
assert.strictEqual(span, testSpan, "Should return mocked span");
assert.isTrue(startSpanStub.calledOnce);
assert.ok(startSpanStub.calledOnce);
const [name, options] = startSpanStub.firstCall.args;
assert.strictEqual(name, "Azure.Test.testMethod");
assert.deepEqual(options, { kind: SpanKind.INTERNAL });
assert.isTrue(setAttributeSpy.calledOnceWithExactly("az.namespace", "Microsoft.Test"));
assert.ok(setAttributeSpy.calledOnceWithExactly("az.namespace", "Microsoft.Test"));

assert.deepEqual(updatedOptions.tracingOptions, {
someOtherField: "someOtherFieldValue",
// TODO: note, this will be incorrect (and break) when we get to the next opentelemetry
// upgrade (and that'll be a good reminder to fix it)
context: { someContext: "some Context" },
spanOptions: {
attributes: {
'az.namespace': 'Microsoft.Test'
},
parent: {
spanId: '',
traceFlags: 0,
traceId: ''
}
}
});
});

it("returns updated SpanOptions", () => {
const options: OperationOptions = {};
const options: { tracingOptions?: OperationTracingOptionsLike } = {};
const { span, updatedOptions } = createSpan("testMethod", options);
assert.isEmpty(options, "original options should not be modified");
assert.deepStrictEqual(options, {}, "original options should not be modified");
assert.notStrictEqual(updatedOptions, options, "should return new object");
const expected: OperationOptions = {
const expected: { tracingOptions?: OperationTracingOptionsLike } = {
tracingOptions: {
spanOptions: {
parent: span.context(),
Expand All @@ -52,7 +76,7 @@ describe("createSpan", () => {
});

it("preserves existing attributes", () => {
const options: OperationOptions = {
const options: { tracingOptions?: OperationTracingOptionsLike } = {
tracingOptions: {
spanOptions: {
attributes: {
Expand All @@ -63,7 +87,7 @@ describe("createSpan", () => {
};
const { span, updatedOptions } = createSpan("testMethod", options);
assert.notStrictEqual(updatedOptions, options, "should return new object");
const expected: OperationOptions = {
const expected: { tracingOptions?: OperationTracingOptionsLike } = {
tracingOptions: {
spanOptions: {
parent: span.context(),
Expand Down