Skip to content

Commit af70569

Browse files
authored
[eventhubs] - Upgrade Event Hubs to core-tracing preview.14 (Azure#20240)
### Packages impacted by this PR @azure/event-hubs @azure/test-utils ### Issues associated with this PR Azure#20213 ### Describe the problem that is addressed by this PR Now that @azure/[email protected] is out, and hopefully the last version before GA, we need to upgrade a few packages in order to dogfood both the upgrade experience and the usage of these packages with the new [instrumentation package](https://www.npmjs.com/package/@azure/opentelemetry-instrumentation-azure-sdk). My goal was to upgrade one AMQP package and a few HTTP packages in addition to core-rest-pipeline to collect feedback. Upgrading EventHubs now allows us to start using the new APIs in a client package. ### What are the possible designs available to address the problem? If there are more than one possible design, why was the one in this PR chosen? Haven't decided if this should be merged _or_ if we should just publish to NPM under a custom tag in order to allow customers to dogfood. I know there's an internal IoT Hub engineer that is interested in helping out so we just need something for them to try. Right now though I am leaning towards merging this in. I also decided to dump all the `parentSpan` legacy stuff - there really isn't a need to maintain tracing backwards compat with an option that has not been used in over 6 months.
1 parent ccc7c3f commit af70569

File tree

20 files changed

+846
-1527
lines changed

20 files changed

+846
-1527
lines changed

common/config/rush/pnpm-lock.yaml

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

sdk/eventhub/event-hubs/CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,10 @@
1010

1111
### Other Changes
1212

13+
- Updated our `@azure/core-tracing` dependency to the latest version (1.0.0-preview.14)
14+
- Notable changes include Removal of `@opentelemetry/api` as a transitive dependency and ensuring that the active context is properly propagated.
15+
- Customers who would like to continue using OpenTelemetry driven tracing should visit our [OpenTelemetry Instrumentation](https://www.npmjs.com/package/@azure/opentelemetry-instrumentation-azure-sdk) package for instructions.
16+
1317
## 5.8.0-beta.1 (2022-02-08)
1418

1519
### Features Added

sdk/eventhub/event-hubs/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,7 @@
110110
"@azure/core-amqp": "^3.1.0",
111111
"@azure/core-asynciterator-polyfill": "^1.0.0",
112112
"@azure/core-auth": "^1.3.0",
113-
"@azure/core-tracing": "1.0.0-preview.13",
113+
"@azure/core-tracing": "1.0.0-preview.14",
114114
"@azure/logger": "^1.0.0",
115115
"buffer": "^6.0.0",
116116
"is-buffer": "^2.0.3",

sdk/eventhub/event-hubs/review/event-hubs.api.md

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,6 @@ import { OperationTracingOptions } from '@azure/core-tracing';
1515
import { RetryMode } from '@azure/core-amqp';
1616
import { RetryOptions } from '@azure/core-amqp';
1717
import { SASCredential } from '@azure/core-auth';
18-
import { Span } from '@azure/core-tracing';
19-
import { SpanContext } from '@azure/core-tracing';
2018
import { TokenCredential } from '@azure/core-auth';
2119
import { WebSocketImpl } from 'rhea-promise';
2220
import { WebSocketOptions } from '@azure/core-amqp';
@@ -374,8 +372,6 @@ export { TokenCredential }
374372

375373
// @public
376374
export interface TryAddOptions {
377-
// @deprecated (undocumented)
378-
parentSpan?: Span | SpanContext;
379375
tracingOptions?: OperationTracingOptions;
380376
}
381377

sdk/eventhub/event-hubs/src/diagnostics/instrumentEventData.ts

Lines changed: 15 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -2,15 +2,10 @@
22
// Licensed under the MIT license.
33

44
import { EventData, isAmqpAnnotatedMessage } from "../eventData";
5-
import {
6-
extractSpanContextFromTraceParentHeader,
7-
getTraceParentHeader,
8-
isSpanContextValid,
9-
} from "@azure/core-tracing";
5+
import { TracingContext } from "@azure/core-tracing";
106
import { AmqpAnnotatedMessage } from "@azure/core-amqp";
117
import { OperationOptions } from "../util/operationOptions";
12-
import { SpanContext } from "@azure/core-tracing";
13-
import { createMessageSpan } from "./tracing";
8+
import { toSpanOptions, tracingClient } from "./tracing";
149

1510
/**
1611
* @internal
@@ -29,7 +24,7 @@ export function instrumentEventData(
2924
options: OperationOptions,
3025
entityPath: string,
3126
host: string
32-
): { event: EventData; spanContext: SpanContext | undefined } {
27+
): { event: EventData; spanContext: TracingContext | undefined } {
3328
const props = isAmqpAnnotatedMessage(eventData)
3429
? eventData.applicationProperties
3530
: eventData.properties;
@@ -41,7 +36,11 @@ export function instrumentEventData(
4136
return { event: eventData, spanContext: undefined };
4237
}
4338

44-
const { span: messageSpan } = createMessageSpan(options, { entityPath, host });
39+
const { span: messageSpan, updatedOptions } = tracingClient.startSpan(
40+
"message",
41+
options,
42+
toSpanOptions({ entityPath, host }, "producer")
43+
);
4544
try {
4645
if (!messageSpan.isRecording()) {
4746
return {
@@ -50,8 +49,10 @@ export function instrumentEventData(
5049
};
5150
}
5251

53-
const traceParent = getTraceParentHeader(messageSpan.spanContext());
54-
if (traceParent && isSpanContextValid(messageSpan.spanContext())) {
52+
const traceParent = tracingClient.createRequestHeaders(
53+
updatedOptions.tracingOptions?.tracingContext
54+
)["traceparent"];
55+
if (traceParent) {
5556
const copiedProps = { ...props };
5657

5758
// create a copy so the original isn't modified
@@ -65,7 +66,7 @@ export function instrumentEventData(
6566

6667
return {
6768
event: eventData,
68-
spanContext: messageSpan.spanContext(),
69+
spanContext: updatedOptions.tracingOptions?.tracingContext,
6970
};
7071
} finally {
7172
messageSpan.end();
@@ -77,11 +78,11 @@ export function instrumentEventData(
7778
* @param eventData - An individual `EventData` object.
7879
* @internal
7980
*/
80-
export function extractSpanContextFromEventData(eventData: EventData): SpanContext | undefined {
81+
export function extractSpanContextFromEventData(eventData: EventData): TracingContext | undefined {
8182
if (!eventData.properties || !eventData.properties[TRACEPARENT_PROPERTY]) {
8283
return;
8384
}
8485

8586
const diagnosticId = eventData.properties[TRACEPARENT_PROPERTY];
86-
return extractSpanContextFromTraceParentHeader(diagnosticId);
87+
return tracingClient.parseTraceparentHeader(diagnosticId);
8788
}
Lines changed: 23 additions & 122 deletions
Original file line numberDiff line numberDiff line change
@@ -1,136 +1,37 @@
11
// Copyright (c) Microsoft Corporation.
22
// Licensed under the MIT license.
33

4-
import {
5-
Span,
6-
SpanContext,
7-
SpanKind,
8-
SpanOptions,
9-
context,
10-
createSpanFunction,
11-
setSpan,
12-
setSpanContext,
13-
} from "@azure/core-tracing";
4+
import { createTracingClient, TracingSpanOptions, TracingSpanKind } from "@azure/core-tracing";
145
import { EventHubConnectionConfig } from "../eventhubConnectionConfig";
15-
import { OperationOptions } from "../util/operationOptions";
16-
import { TryAddOptions } from "../eventDataBatch";
6+
import { packageJsonInfo } from "../util/constants";
177

18-
const _createSpan = createSpanFunction({
8+
/**
9+
* The {@link TracingClient} that is used to add tracing spans.
10+
*/
11+
export const tracingClient = createTracingClient({
1912
namespace: "Microsoft.EventHub",
20-
packagePrefix: "Azure.EventHubs",
13+
packageName: packageJsonInfo.name,
14+
packageVersion: packageJsonInfo.version,
2115
});
2216

2317
/**
24-
* Creates an EventHubs specific span, with peer.address and message_bus.destination filled out.
25-
* @internal
18+
* Creates {@link TracingSpanOptions} from the provided data.
19+
* @param eventHubConfig - The configuration object containing initial attributes to set on the span.
20+
* @param spanKind - The {@link TracingSpanKind} for the newly created span.
21+
* @returns a {@link TracingSpanOptions} that can be passed to a {@link TracingClient}
2622
*/
27-
export function createEventHubSpan(
28-
operationName: string,
29-
operationOptions: OperationOptions | undefined,
30-
connectionConfig: Pick<EventHubConnectionConfig, "entityPath" | "host">,
31-
additionalSpanOptions?: SpanOptions
32-
): { span: Span; updatedOptions: OperationOptions } {
33-
const { span, updatedOptions } = _createSpan(operationName, {
34-
...operationOptions,
35-
tracingOptions: {
36-
...operationOptions?.tracingOptions,
37-
spanOptions: {
38-
// By passing spanOptions if they exist at runtime, we're backwards compatible with @azure/[email protected] and earlier.
39-
...(operationOptions?.tracingOptions as any)?.spanOptions,
40-
...additionalSpanOptions,
41-
},
23+
export function toSpanOptions(
24+
eventHubConfig: Pick<EventHubConnectionConfig, "entityPath" | "host">,
25+
spanKind?: TracingSpanKind
26+
): TracingSpanOptions {
27+
const spanOptions: TracingSpanOptions = {
28+
spanAttributes: {
29+
"message_bus.destination": eventHubConfig.entityPath,
30+
"peer.address": eventHubConfig.host,
4231
},
43-
});
44-
45-
span.setAttribute("message_bus.destination", connectionConfig.entityPath);
46-
span.setAttribute("peer.address", connectionConfig.host);
47-
48-
return {
49-
span,
50-
updatedOptions,
5132
};
52-
}
53-
54-
/**
55-
* @internal
56-
*/
57-
export function createMessageSpan(
58-
operationOptions: OperationOptions,
59-
eventHubConfig: Pick<EventHubConnectionConfig, "entityPath" | "host">
60-
): ReturnType<typeof createEventHubSpan> {
61-
return createEventHubSpan("message", operationOptions, eventHubConfig, {
62-
kind: SpanKind.PRODUCER,
63-
});
64-
}
65-
66-
/**
67-
* Converts TryAddOptions into the modern shape (OperationOptions) when needed.
68-
* (this is something we can eliminate at the next major release of EH _or_ when
69-
* we release with the GA version of opentelemetry).
70-
*
71-
* @internal
72-
*/
73-
export function convertTryAddOptionsForCompatibility(tryAddOptions: TryAddOptions): TryAddOptions {
74-
/* eslint-disable-next-line @typescript-eslint/ban-ts-comment */
75-
// @ts-ignore: parentSpan is deprecated and this is compat code to translate it until we can get rid of it.
76-
const legacyParentSpanOrSpanContext = tryAddOptions.parentSpan;
77-
78-
/*
79-
Our goal here is to offer compatibility but there is a case where a user might accidentally pass
80-
_both_ sets of options. We'll assume they want the OperationTracingOptions code path in that case.
81-
82-
Example of accidental span passing:
83-
84-
const someOptionsPassedIntoTheirFunction = {
85-
parentSpan: span; // set somewhere else in their code
86-
}
87-
88-
function takeSomeOptionsFromSomewhere(someOptionsPassedIntoTheirFunction) {
89-
90-
batch.tryAddMessage(message, {
91-
// "runtime" blend of options from some other part of their app
92-
...someOptionsPassedIntoTheirFunction, // parentSpan comes along for the ride...
93-
94-
tracingOptions: {
95-
// thank goodness, I'm doing this right! (thinks the developer)
96-
spanOptions: {
97-
context: context
98-
}
99-
}
100-
});
101-
}
102-
103-
And now they've accidentally been opted into the legacy code path even though they think
104-
they're using the modern code path.
105-
106-
This does kick the can down the road a bit - at some point we will be putting them in this
107-
situation where things looked okay but their spans are becoming unparented but we can
108-
try to announce this (and other changes related to tracing) in our next big rev.
109-
*/
110-
111-
if (!legacyParentSpanOrSpanContext || tryAddOptions.tracingOptions) {
112-
// assume that the options are already in the modern shape even if (possibly)
113-
// they were still specifying `parentSpan`
114-
return tryAddOptions;
115-
}
116-
117-
const convertedOptions: TryAddOptions = {
118-
...tryAddOptions,
119-
tracingOptions: {
120-
tracingContext: isSpan(legacyParentSpanOrSpanContext)
121-
? setSpan(context.active(), legacyParentSpanOrSpanContext)
122-
: setSpanContext(context.active(), legacyParentSpanOrSpanContext),
123-
},
124-
};
125-
126-
return convertedOptions;
127-
}
128-
129-
function isSpan(possibleSpan: Span | SpanContext | undefined): possibleSpan is Span {
130-
if (possibleSpan == null) {
131-
return false;
33+
if (spanKind) {
34+
spanOptions.spanKind = spanKind;
13235
}
133-
134-
const x = possibleSpan as Span;
135-
return typeof x.spanContext === "function";
36+
return spanOptions;
13637
}

sdk/eventhub/event-hubs/src/eventDataBatch.ts

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,8 @@ import { AmqpAnnotatedMessage } from "@azure/core-amqp";
55
import { EventData, populateIdempotentMessageAnnotations, toRheaMessage } from "./eventData";
66
import { ConnectionContext } from "./connectionContext";
77
import { MessageAnnotations, message, Message as RheaMessage } from "rhea-promise";
8-
import { Span, SpanContext } from "@azure/core-tracing";
98
import { isDefined, isObjectWithProperties } from "./util/typeGuards";
10-
import { OperationTracingOptions } from "@azure/core-tracing";
11-
import { convertTryAddOptionsForCompatibility } from "./diagnostics/tracing";
9+
import { OperationTracingOptions, TracingContext } from "@azure/core-tracing";
1210
import { instrumentEventData } from "./diagnostics/instrumentEventData";
1311
import { throwTypeErrorIfParameterMissing } from "./util/error";
1412
import { PartitionPublishingProperties } from "./models/private";
@@ -48,11 +46,6 @@ export interface TryAddOptions {
4846
* The options to use when creating Spans for tracing.
4947
*/
5048
tracingOptions?: OperationTracingOptions;
51-
52-
/**
53-
* @deprecated Tracing options have been moved to the `tracingOptions` property.
54-
*/
55-
parentSpan?: Span | SpanContext;
5649
}
5750

5851
/**
@@ -153,7 +146,7 @@ export class EventDataBatchImpl implements EventDataBatch {
153146
/**
154147
* List of 'message' span contexts.
155148
*/
156-
private _spanContexts: SpanContext[] = [];
149+
private _spanContexts: TracingContext[] = [];
157150
/**
158151
* The message annotations to apply on the batch envelope.
159152
* This will reflect the message annotations on the first event
@@ -256,7 +249,7 @@ export class EventDataBatchImpl implements EventDataBatch {
256249
* Gets the "message" span contexts that were created when adding events to the batch.
257250
* @internal
258251
*/
259-
get _messageSpanContexts(): SpanContext[] {
252+
get _messageSpanContexts(): TracingContext[] {
260253
return this._spanContexts;
261254
}
262255

@@ -377,7 +370,6 @@ export class EventDataBatchImpl implements EventDataBatch {
377370
*/
378371
public tryAdd(eventData: EventData | AmqpAnnotatedMessage, options: TryAddOptions = {}): boolean {
379372
throwTypeErrorIfParameterMissing(this._context.connectionId, "tryAdd", "eventData", eventData);
380-
options = convertTryAddOptionsForCompatibility(options);
381373

382374
const { entityPath, host } = this._context.config;
383375
const { event: instrumentedEvent, spanContext } = instrumentEventData(

0 commit comments

Comments
 (0)