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
Prev Previous commit
Next Next commit
storage-queue: same story, just swapping to core-tracing's versions.
  • Loading branch information
richardpark-msft committed Feb 20, 2021
commit 7edc5dd4450cea746e7ed535a8c525ddf1de4341
71 changes: 31 additions & 40 deletions sdk/storage/storage-queue/src/QueueClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ import {
} from "./utils/utils.common";
import { StorageSharedKeyCredential } from "./credentials/StorageSharedKeyCredential";
import { AnonymousCredential } from "./credentials/AnonymousCredential";
import { createSpan } from "./utils/tracing";
import { convertTracingToRequestOptionsBase, createSpan } from "./utils/tracing";
import { Metadata } from "./models";
import { generateQueueSASQueryParameters } from "./QueueSASSignatureValues";
import { SasIPRange } from "./SasIPRange";
Expand Down Expand Up @@ -584,12 +584,12 @@ export class QueueClient extends StorageClient {
* ```
*/
public async create(options: QueueCreateOptions = {}): Promise<QueueCreateResponse> {
const { span, spanOptions } = createSpan("QueueClient-create", options.tracingOptions);
const { span, updatedOptions } = createSpan("QueueClient-create", options);
try {
return await this.queueContext.create({
...options,
abortSignal: options.abortSignal,
spanOptions
...convertTracingToRequestOptionsBase(updatedOptions)
});
} catch (e) {
span.setStatus({
Expand All @@ -612,15 +612,9 @@ export class QueueClient extends StorageClient {
public async createIfNotExists(
options: QueueCreateOptions = {}
): Promise<QueueCreateIfNotExistsResponse> {
const { span, spanOptions } = createSpan(
"QueueClient-createIfNotExists",
options.tracingOptions
);
const { span, updatedOptions } = createSpan("QueueClient-createIfNotExists", options);
try {
const response = await this.create({
...options,
tracingOptions: { ...options!.tracingOptions, spanOptions }
});
const response = await this.create(updatedOptions);

// When a queue with the specified name already exists, the Queue service checks the metadata associated with the existing queue.
// If the existing metadata is identical to the metadata specified on the Create Queue request, status code 204 (No Content) is returned.
Expand Down Expand Up @@ -667,12 +661,9 @@ export class QueueClient extends StorageClient {
public async deleteIfExists(
options: QueueDeleteOptions = {}
): Promise<QueueDeleteIfExistsResponse> {
const { span, spanOptions } = createSpan("QueueClient-deleteIfExists", options.tracingOptions);
const { span, updatedOptions } = createSpan("QueueClient-deleteIfExists", options);
try {
const res = await this.delete({
...options,
tracingOptions: { ...options!.tracingOptions, spanOptions }
});
const res = await this.delete(updatedOptions);
return {
succeeded: true,
...res
Expand Down Expand Up @@ -716,11 +707,11 @@ export class QueueClient extends StorageClient {
* ```
*/
public async delete(options: QueueDeleteOptions = {}): Promise<QueueDeleteResponse> {
const { span, spanOptions } = createSpan("QueueClient-delete", options.tracingOptions);
const { span, updatedOptions } = createSpan("QueueClient-delete", options);
try {
return await this.queueContext.deleteMethod({
abortSignal: options.abortSignal,
spanOptions
...convertTracingToRequestOptionsBase(updatedOptions)
});
} catch (e) {
span.setStatus({
Expand All @@ -743,11 +734,11 @@ export class QueueClient extends StorageClient {
* @param options - options to Exists operation.
*/
public async exists(options: QueueExistsOptions = {}): Promise<boolean> {
const { span, spanOptions } = createSpan("QueueClient-exists", options.tracingOptions);
const { span, updatedOptions } = createSpan("QueueClient-exists", options);
try {
await this.getProperties({
abortSignal: options.abortSignal,
tracingOptions: { ...options.tracingOptions, spanOptions }
tracingOptions: updatedOptions.tracingOptions
});
return true;
} catch (e) {
Expand Down Expand Up @@ -784,11 +775,11 @@ export class QueueClient extends StorageClient {
public async getProperties(
options: QueueGetPropertiesOptions = {}
): Promise<QueueGetPropertiesResponse> {
const { span, spanOptions } = createSpan("QueueClient-getProperties", options.tracingOptions);
const { span, updatedOptions } = createSpan("QueueClient-getProperties", options);
try {
return await this.queueContext.getProperties({
abortSignal: options.abortSignal,
spanOptions
...convertTracingToRequestOptionsBase(updatedOptions)
});
} catch (e) {
span.setStatus({
Expand Down Expand Up @@ -816,12 +807,12 @@ export class QueueClient extends StorageClient {
metadata?: Metadata,
options: QueueSetMetadataOptions = {}
): Promise<QueueSetMetadataResponse> {
const { span, spanOptions } = createSpan("QueueClient-setMetadata", options.tracingOptions);
const { span, updatedOptions } = createSpan("QueueClient-setMetadata", options);
try {
return await this.queueContext.setMetadata({
abortSignal: options.abortSignal,
metadata,
spanOptions
...convertTracingToRequestOptionsBase(updatedOptions)
});
} catch (e) {
span.setStatus({
Expand All @@ -848,11 +839,11 @@ export class QueueClient extends StorageClient {
public async getAccessPolicy(
options: QueueGetAccessPolicyOptions = {}
): Promise<QueueGetAccessPolicyResponse> {
const { span, spanOptions } = createSpan("QueueClient-getAccessPolicy", options.tracingOptions);
const { span, updatedOptions } = createSpan("QueueClient-getAccessPolicy", options);
try {
const response = await this.queueContext.getAccessPolicy({
abortSignal: options.abortSignal,
spanOptions
...convertTracingToRequestOptionsBase(updatedOptions)
});

const res: QueueGetAccessPolicyResponse = {
Expand Down Expand Up @@ -911,7 +902,7 @@ export class QueueClient extends StorageClient {
queueAcl?: SignedIdentifier[],
options: QueueSetAccessPolicyOptions = {}
): Promise<QueueSetAccessPolicyResponse> {
const { span, spanOptions } = createSpan("QueueClient-setAccessPolicy", options.tracingOptions);
const { span, updatedOptions } = createSpan("QueueClient-setAccessPolicy", options);
try {
const acl: SignedIdentifierModel[] = [];
for (const identifier of queueAcl || []) {
Expand All @@ -932,7 +923,7 @@ export class QueueClient extends StorageClient {
return await this.queueContext.setAccessPolicy({
abortSignal: options.abortSignal,
queueAcl: acl,
spanOptions
...convertTracingToRequestOptionsBase(updatedOptions)
});
} catch (e) {
span.setStatus({
Expand All @@ -955,11 +946,11 @@ export class QueueClient extends StorageClient {
public async clearMessages(
options: QueueClearMessagesOptions = {}
): Promise<QueueClearMessagesResponse> {
const { span, spanOptions } = createSpan("QueueClient-clearMessages", options.tracingOptions);
const { span, updatedOptions } = createSpan("QueueClient-clearMessages", options);
try {
return await this.messagesContext.clear({
abortSignal: options.abortSignal,
spanOptions
...convertTracingToRequestOptionsBase(updatedOptions)
});
} catch (e) {
span.setStatus({
Expand Down Expand Up @@ -997,7 +988,7 @@ export class QueueClient extends StorageClient {
messageText: string,
options: QueueSendMessageOptions = {}
): Promise<QueueSendMessageResponse> {
const { span, spanOptions } = createSpan("QueueClient-sendMessage", options.tracingOptions);
const { span, updatedOptions } = createSpan("QueueClient-sendMessage", options);
try {
const response = await this.messagesContext.enqueue(
{
Expand All @@ -1006,7 +997,7 @@ export class QueueClient extends StorageClient {
{
abortSignal: options.abortSignal,
...options,
spanOptions
...convertTracingToRequestOptionsBase(updatedOptions)
}
);
const item = response[0];
Expand Down Expand Up @@ -1062,12 +1053,12 @@ export class QueueClient extends StorageClient {
public async receiveMessages(
options: QueueReceiveMessageOptions = {}
): Promise<QueueReceiveMessageResponse> {
const { span, spanOptions } = createSpan("QueueClient-receiveMessages", options.tracingOptions);
const { span, updatedOptions } = createSpan("QueueClient-receiveMessages", options);
try {
const response = await this.messagesContext.dequeue({
abortSignal: options.abortSignal,
...options,
spanOptions
...convertTracingToRequestOptionsBase(updatedOptions)
});

const res: QueueReceiveMessageResponse = {
Expand Down Expand Up @@ -1113,12 +1104,12 @@ export class QueueClient extends StorageClient {
public async peekMessages(
options: QueuePeekMessagesOptions = {}
): Promise<QueuePeekMessagesResponse> {
const { span, spanOptions } = createSpan("QueueClient-peekMessages", options.tracingOptions);
const { span, updatedOptions } = createSpan("QueueClient-peekMessages", options);
try {
const response = await this.messagesContext.peek({
abortSignal: options.abortSignal,
...options,
spanOptions
...convertTracingToRequestOptionsBase(updatedOptions)
});

const res: QueuePeekMessagesResponse = {
Expand Down Expand Up @@ -1161,11 +1152,11 @@ export class QueueClient extends StorageClient {
popReceipt: string,
options: QueueDeleteMessageOptions = {}
): Promise<QueueDeleteMessageResponse> {
const { span, spanOptions } = createSpan("QueueClient-deleteMessage", options.tracingOptions);
const { span, updatedOptions } = createSpan("QueueClient-deleteMessage", options);
try {
return await this.getMessageIdContext(messageId).deleteMethod(popReceipt, {
abortSignal: options.abortSignal,
spanOptions
...convertTracingToRequestOptionsBase(updatedOptions)
});
} catch (e) {
span.setStatus({
Expand Down Expand Up @@ -1202,7 +1193,7 @@ export class QueueClient extends StorageClient {
visibilityTimeout?: number,
options: QueueUpdateMessageOptions = {}
): Promise<QueueUpdateMessageResponse> {
const { span, spanOptions } = createSpan("QueueClient-updateMessage", options.tracingOptions);
const { span, updatedOptions } = createSpan("QueueClient-updateMessage", options);
let queueMessage = undefined;
if (message !== undefined) {
queueMessage = { messageText: message };
Expand All @@ -1211,7 +1202,7 @@ export class QueueClient extends StorageClient {
try {
return await this.getMessageIdContext(messageId).update(popReceipt, visibilityTimeout || 0, {
abortSignal: options.abortSignal,
spanOptions,
...convertTracingToRequestOptionsBase(updatedOptions),
queueMessage
});
} catch (e) {
Expand Down
50 changes: 13 additions & 37 deletions sdk/storage/storage-queue/src/QueueServiceClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ import {
} from "./utils/utils.common";
import { StorageSharedKeyCredential } from "./credentials/StorageSharedKeyCredential";
import { AnonymousCredential } from "./credentials/AnonymousCredential";
import { createSpan } from "./utils/tracing";
import { convertTracingToRequestOptionsBase, createSpan } from "./utils/tracing";
import { QueueClient, QueueCreateOptions, QueueDeleteOptions } from "./QueueClient";
import { AccountSASPermissions } from "./AccountSASPermissions";
import { generateAccountSASQueryParameters } from "./AccountSASSignatureValues";
Expand Down Expand Up @@ -321,10 +321,7 @@ export class QueueServiceClient extends StorageClient {
marker?: string,
options: ServiceListQueuesSegmentOptions = {}
): Promise<ServiceListQueuesSegmentResponse> {
const { span, spanOptions } = createSpan(
"QueueServiceClient-listQueuesSegment",
options.tracingOptions
);
const { span, updatedOptions } = createSpan("QueueServiceClient-listQueuesSegment", options);

if (options.prefix === "") {
options.prefix = undefined;
Expand All @@ -337,7 +334,7 @@ export class QueueServiceClient extends StorageClient {
maxPageSize: options.maxPageSize,
prefix: options.prefix,
include: options.include === undefined ? undefined : [options.include],
spanOptions
...convertTracingToRequestOptionsBase(updatedOptions)
});
} catch (e) {
span.setStatus({
Expand Down Expand Up @@ -525,14 +522,11 @@ export class QueueServiceClient extends StorageClient {
public async getProperties(
options: ServiceGetPropertiesOptions = {}
): Promise<ServiceGetPropertiesResponse> {
const { span, spanOptions } = createSpan(
"QueueServiceClient-getProperties",
options.tracingOptions
);
const { span, updatedOptions } = createSpan("QueueServiceClient-getProperties", options);
try {
return await this.serviceContext.getProperties({
abortSignal: options.abortSignal,
spanOptions
...convertTracingToRequestOptionsBase(updatedOptions)
});
} catch (e) {
span.setStatus({
Expand All @@ -558,14 +552,11 @@ export class QueueServiceClient extends StorageClient {
properties: QueueServiceProperties,
options: ServiceGetPropertiesOptions = {}
): Promise<ServiceSetPropertiesResponse> {
const { span, spanOptions } = createSpan(
"QueueServiceClient-setProperties",
options.tracingOptions
);
const { span, updatedOptions } = createSpan("QueueServiceClient-setProperties", options);
try {
return await this.serviceContext.setProperties(properties, {
abortSignal: options.abortSignal,
spanOptions
...convertTracingToRequestOptionsBase(updatedOptions)
});
} catch (e) {
span.setStatus({
Expand All @@ -590,14 +581,11 @@ export class QueueServiceClient extends StorageClient {
public async getStatistics(
options: ServiceGetStatisticsOptions = {}
): Promise<ServiceGetStatisticsResponse> {
const { span, spanOptions } = createSpan(
"QueueServiceClient-getStatistics",
options.tracingOptions
);
const { span, updatedOptions } = createSpan("QueueServiceClient-getStatistics", options);
try {
return await this.serviceContext.getStatistics({
abortSignal: options.abortSignal,
spanOptions
...convertTracingToRequestOptionsBase(updatedOptions)
});
} catch (e) {
span.setStatus({
Expand All @@ -622,15 +610,9 @@ export class QueueServiceClient extends StorageClient {
queueName: string,
options: QueueCreateOptions = {}
): Promise<QueueCreateResponse> {
const { span, spanOptions } = createSpan(
"QueueServiceClient-createQueue",
options.tracingOptions
);
const { span, updatedOptions } = createSpan("QueueServiceClient-createQueue", options);
try {
return await this.getQueueClient(queueName).create({
...options,
tracingOptions: { ...options!.tracingOptions, spanOptions }
});
return await this.getQueueClient(queueName).create(updatedOptions);
} catch (e) {
span.setStatus({
code: CanonicalCode.UNKNOWN,
Expand All @@ -654,15 +636,9 @@ export class QueueServiceClient extends StorageClient {
queueName: string,
options: QueueDeleteOptions = {}
): Promise<QueueDeleteResponse> {
const { span, spanOptions } = createSpan(
"QueueServiceClient-deleteQueue",
options.tracingOptions
);
const { span, updatedOptions } = createSpan("QueueServiceClient-deleteQueue", options);
try {
return await this.getQueueClient(queueName).delete({
...options,
tracingOptions: { ...options!.tracingOptions, spanOptions }
});
return await this.getQueueClient(queueName).delete(updatedOptions);
} catch (e) {
span.setStatus({
code: CanonicalCode.UNKNOWN,
Expand Down
16 changes: 16 additions & 0 deletions sdk/storage/storage-queue/src/utils/tracing.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT license.

import { OperationOptions, RequestOptionsBase } from "@azure/core-http";
import { createSpanFunction } from "@azure/core-tracing";

/**
Expand All @@ -11,3 +12,18 @@ export const createSpan = createSpanFunction({
packagePrefix: "Azure.Storage.Queue",
namespace: "Microsoft.Storage"
});

/**
* @internal
*
* Adapt the tracing options from OperationOptions to what they need to be for
* RequestOptionsBase (when we update to later OpenTelemetry versions this is now
* two separate fields, not just one).
*/
export function convertTracingToRequestOptionsBase(
options?: OperationOptions
): Pick<RequestOptionsBase, "spanOptions"> {
return {
spanOptions: options?.tracingOptions?.spanOptions
};
}