Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
14 changes: 6 additions & 8 deletions common/config/rush/pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions sdk/eventhub/event-hubs/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,10 @@

### Other Changes

- Updated our `@azure/core-tracing` dependency to the latest version (1.0.0-preview.14)
- Notable changes include Removal of `@opentelemetry/api` as a transitive dependency and ensuring that the active context is properly propagated.
- 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.

## 5.8.0-beta.1 (2022-02-08)

### Features Added
Expand Down
2 changes: 1 addition & 1 deletion sdk/eventhub/event-hubs/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@
"@azure/core-amqp": "^3.1.0",
"@azure/core-asynciterator-polyfill": "^1.0.0",
"@azure/core-auth": "^1.3.0",
"@azure/core-tracing": "1.0.0-preview.13",
"@azure/core-tracing": "1.0.0-preview.14",
Comment thread
maorleger marked this conversation as resolved.
Outdated
"@azure/logger": "^1.0.0",
"buffer": "^6.0.0",
"is-buffer": "^2.0.3",
Expand Down
4 changes: 0 additions & 4 deletions sdk/eventhub/event-hubs/review/event-hubs.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,6 @@ import { OperationTracingOptions } from '@azure/core-tracing';
import { RetryMode } from '@azure/core-amqp';
import { RetryOptions } from '@azure/core-amqp';
import { SASCredential } from '@azure/core-auth';
import { Span } from '@azure/core-tracing';
import { SpanContext } from '@azure/core-tracing';
import { TokenCredential } from '@azure/core-auth';
import { WebSocketImpl } from 'rhea-promise';
import { WebSocketOptions } from '@azure/core-amqp';
Expand Down Expand Up @@ -374,8 +372,6 @@ export { TokenCredential }

// @public
export interface TryAddOptions {
// @deprecated (undocumented)
parentSpan?: Span | SpanContext;
Comment thread
maorleger marked this conversation as resolved.
Outdated
tracingOptions?: OperationTracingOptions;
}

Expand Down
29 changes: 15 additions & 14 deletions sdk/eventhub/event-hubs/src/diagnostics/instrumentEventData.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,10 @@
// Licensed under the MIT license.

import { EventData, isAmqpAnnotatedMessage } from "../eventData";
import {
extractSpanContextFromTraceParentHeader,
getTraceParentHeader,
isSpanContextValid,
} from "@azure/core-tracing";
import { TracingContext } from "@azure/core-tracing";
import { AmqpAnnotatedMessage } from "@azure/core-amqp";
import { OperationOptions } from "../util/operationOptions";
import { SpanContext } from "@azure/core-tracing";
import { createMessageSpan } from "./tracing";
import { toSpanOptions, tracingClient } from "./tracing";

/**
* @internal
Expand All @@ -29,7 +24,7 @@ export function instrumentEventData(
options: OperationOptions,
entityPath: string,
host: string
): { event: EventData; spanContext: SpanContext | undefined } {
): { event: EventData; spanContext: TracingContext | undefined } {
const props = isAmqpAnnotatedMessage(eventData)
? eventData.applicationProperties
: eventData.properties;
Expand All @@ -41,7 +36,11 @@ export function instrumentEventData(
return { event: eventData, spanContext: undefined };
}

const { span: messageSpan } = createMessageSpan(options, { entityPath, host });
const { span: messageSpan, updatedOptions } = tracingClient.startSpan(
"message",
options,
toSpanOptions({ entityPath, host }, "producer")
);
try {
if (!messageSpan.isRecording()) {
return {
Expand All @@ -50,8 +49,10 @@ export function instrumentEventData(
};
}

const traceParent = getTraceParentHeader(messageSpan.spanContext());
if (traceParent && isSpanContextValid(messageSpan.spanContext())) {
const traceParent = tracingClient.createRequestHeaders(
updatedOptions.tracingOptions?.tracingContext
)["traceparent"];
if (traceParent) {
const copiedProps = { ...props };

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

return {
event: eventData,
spanContext: messageSpan.spanContext(),
spanContext: updatedOptions.tracingOptions?.tracingContext,
};
} finally {
messageSpan.end();
Expand All @@ -77,11 +78,11 @@ export function instrumentEventData(
* @param eventData - An individual `EventData` object.
* @internal
*/
export function extractSpanContextFromEventData(eventData: EventData): SpanContext | undefined {
export function extractSpanContextFromEventData(eventData: EventData): TracingContext | undefined {
if (!eventData.properties || !eventData.properties[TRACEPARENT_PROPERTY]) {
return;
}

const diagnosticId = eventData.properties[TRACEPARENT_PROPERTY];
return extractSpanContextFromTraceParentHeader(diagnosticId);
return tracingClient.parseTraceparentHeader(diagnosticId);
}
145 changes: 23 additions & 122 deletions sdk/eventhub/event-hubs/src/diagnostics/tracing.ts
Original file line number Diff line number Diff line change
@@ -1,136 +1,37 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT license.

import {
Span,
SpanContext,
SpanKind,
SpanOptions,
context,
createSpanFunction,
setSpan,
setSpanContext,
} from "@azure/core-tracing";
import { createTracingClient, TracingSpanOptions, TracingSpanKind } from "@azure/core-tracing";
import { EventHubConnectionConfig } from "../eventhubConnectionConfig";
import { OperationOptions } from "../util/operationOptions";
import { TryAddOptions } from "../eventDataBatch";
import { packageJsonInfo } from "../util/constants";

const _createSpan = createSpanFunction({
/**
* The {@link TracingClient} that is used to add tracing spans.
*/
export const tracingClient = createTracingClient({
namespace: "Microsoft.EventHub",
packagePrefix: "Azure.EventHubs",
packageName: packageJsonInfo.name,
packageVersion: packageJsonInfo.version,
});

/**
* Creates an EventHubs specific span, with peer.address and message_bus.destination filled out.
* @internal
* Creates {@link TracingSpanOptions} from the provided data.
* @param eventHubConfig - The configuration object containing initial attributes to set on the span.
* @param spanKind - The {@link TracingSpanKind} for the newly created span.
* @returns a {@link TracingSpanOptions} that can be passed to a {@link TracingClient}
*/
export function createEventHubSpan(
operationName: string,
operationOptions: OperationOptions | undefined,
connectionConfig: Pick<EventHubConnectionConfig, "entityPath" | "host">,
additionalSpanOptions?: SpanOptions
): { span: Span; updatedOptions: OperationOptions } {
const { span, updatedOptions } = _createSpan(operationName, {
...operationOptions,
tracingOptions: {
...operationOptions?.tracingOptions,
spanOptions: {
// By passing spanOptions if they exist at runtime, we're backwards compatible with @azure/core-tracing@preview.13 and earlier.
...(operationOptions?.tracingOptions as any)?.spanOptions,
...additionalSpanOptions,
},
export function toSpanOptions(
eventHubConfig: Pick<EventHubConnectionConfig, "entityPath" | "host">,
spanKind?: TracingSpanKind
): TracingSpanOptions {
const spanOptions: TracingSpanOptions = {
spanAttributes: {
"message_bus.destination": eventHubConfig.entityPath,
"peer.address": eventHubConfig.host,
},
});

span.setAttribute("message_bus.destination", connectionConfig.entityPath);
span.setAttribute("peer.address", connectionConfig.host);

return {
span,
updatedOptions,
};
}

/**
* @internal
*/
export function createMessageSpan(
operationOptions: OperationOptions,
eventHubConfig: Pick<EventHubConnectionConfig, "entityPath" | "host">
): ReturnType<typeof createEventHubSpan> {
return createEventHubSpan("message", operationOptions, eventHubConfig, {
kind: SpanKind.PRODUCER,
});
}

/**
* Converts TryAddOptions into the modern shape (OperationOptions) when needed.
* (this is something we can eliminate at the next major release of EH _or_ when
* we release with the GA version of opentelemetry).
*
* @internal
*/
export function convertTryAddOptionsForCompatibility(tryAddOptions: TryAddOptions): TryAddOptions {
Copy link
Copy Markdown
Member Author

@maorleger maorleger Feb 9, 2022

Choose a reason for hiding this comment

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

What this does if I understand it correctly is that it supports a few obsolete span parenting options.

In general our policy has been that it's unnecessary to keep backwards compat for tracing options, which is what this function does, at least while tracing is in preview. parentSpan hasn't been a thing in quite some time, neither has spanOptions, and we rely exclusively on tracingContext throughout our code. I think it's safe to file this away but if you strongly disagree let me know

/* eslint-disable-next-line @typescript-eslint/ban-ts-comment */
// @ts-ignore: parentSpan is deprecated and this is compat code to translate it until we can get rid of it.
const legacyParentSpanOrSpanContext = tryAddOptions.parentSpan;

/*
Our goal here is to offer compatibility but there is a case where a user might accidentally pass
_both_ sets of options. We'll assume they want the OperationTracingOptions code path in that case.

Example of accidental span passing:

const someOptionsPassedIntoTheirFunction = {
parentSpan: span; // set somewhere else in their code
}

function takeSomeOptionsFromSomewhere(someOptionsPassedIntoTheirFunction) {

batch.tryAddMessage(message, {
// "runtime" blend of options from some other part of their app
...someOptionsPassedIntoTheirFunction, // parentSpan comes along for the ride...

tracingOptions: {
// thank goodness, I'm doing this right! (thinks the developer)
spanOptions: {
context: context
}
}
});
}

And now they've accidentally been opted into the legacy code path even though they think
they're using the modern code path.

This does kick the can down the road a bit - at some point we will be putting them in this
situation where things looked okay but their spans are becoming unparented but we can
try to announce this (and other changes related to tracing) in our next big rev.
*/

if (!legacyParentSpanOrSpanContext || tryAddOptions.tracingOptions) {
// assume that the options are already in the modern shape even if (possibly)
// they were still specifying `parentSpan`
return tryAddOptions;
}

const convertedOptions: TryAddOptions = {
...tryAddOptions,
tracingOptions: {
tracingContext: isSpan(legacyParentSpanOrSpanContext)
? setSpan(context.active(), legacyParentSpanOrSpanContext)
: setSpanContext(context.active(), legacyParentSpanOrSpanContext),
},
};

return convertedOptions;
}

function isSpan(possibleSpan: Span | SpanContext | undefined): possibleSpan is Span {
if (possibleSpan == null) {
return false;
if (spanKind) {
spanOptions.spanKind = spanKind;
}

const x = possibleSpan as Span;
return typeof x.spanContext === "function";
return spanOptions;
}
14 changes: 3 additions & 11 deletions sdk/eventhub/event-hubs/src/eventDataBatch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,8 @@ import { AmqpAnnotatedMessage } from "@azure/core-amqp";
import { EventData, populateIdempotentMessageAnnotations, toRheaMessage } from "./eventData";
import { ConnectionContext } from "./connectionContext";
import { MessageAnnotations, message, Message as RheaMessage } from "rhea-promise";
import { Span, SpanContext } from "@azure/core-tracing";
import { isDefined, isObjectWithProperties } from "./util/typeGuards";
import { OperationTracingOptions } from "@azure/core-tracing";
import { convertTryAddOptionsForCompatibility } from "./diagnostics/tracing";
import { OperationTracingOptions, TracingContext } from "@azure/core-tracing";
import { instrumentEventData } from "./diagnostics/instrumentEventData";
import { throwTypeErrorIfParameterMissing } from "./util/error";
import { PartitionPublishingProperties } from "./models/private";
Expand Down Expand Up @@ -48,11 +46,6 @@ export interface TryAddOptions {
* The options to use when creating Spans for tracing.
*/
tracingOptions?: OperationTracingOptions;

/**
* @deprecated Tracing options have been moved to the `tracingOptions` property.
*/
parentSpan?: Span | SpanContext;
}

/**
Expand Down Expand Up @@ -153,7 +146,7 @@ export class EventDataBatchImpl implements EventDataBatch {
/**
* List of 'message' span contexts.
*/
private _spanContexts: SpanContext[] = [];
private _spanContexts: TracingContext[] = [];
Comment thread
maorleger marked this conversation as resolved.
Outdated
/**
* The message annotations to apply on the batch envelope.
* This will reflect the message annotations on the first event
Expand Down Expand Up @@ -256,7 +249,7 @@ export class EventDataBatchImpl implements EventDataBatch {
* Gets the "message" span contexts that were created when adding events to the batch.
* @internal
*/
get _messageSpanContexts(): SpanContext[] {
get _messageSpanContexts(): TracingContext[] {
return this._spanContexts;
}

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

const { entityPath, host } = this._context.config;
const { event: instrumentedEvent, spanContext } = instrumentEventData(
Expand Down
Loading