[tracing] Tracing via custom pipeline policy #31157
Conversation
github-actions
bot
added
Azure.Core
OpenTelemetryInstrumentation
test-utils
|
API change check API changes are not detected in this pull request. |
| }, | ||
| "devDependencies": { | ||
| "@azure/eslint-plugin-azure-sdk": "^3.0.0", |
There was a problem hiding this comment.
enable prettier support in VSCode
| // Copyright (c) Microsoft Corporation. | ||
| // Licensed under the MIT license. | ||
|
|
||
| export const SDK_VERSION: string = "1.0.0-beta.2"; |
There was a problem hiding this comment.
You'll need to add this to the constantPaths entry in package.json to allow the azure sdk bot to automatically bump the version here
| /** | ||
| * Copied over verbatim from core-tracing | ||
| */ | ||
| function traceAsync< |
There was a problem hiding this comment.
Probably not needed unless you plan to use it
| namespace: "Microsoft.CognitiveServices", | ||
| packageName: "ai-inference-rest", | ||
| }), | ||
| traceAsync, |
There was a problem hiding this comment.
Augmenting the core-level client. If you remove the traceAsync option you'll be able to just say const traceClient = createTracingClient({ .... }) etc
| }); | ||
| } | ||
|
|
||
| export const tracingPolicy: PipelinePolicy = { |
There was a problem hiding this comment.
Here's your custom policy - you now have access to:
- The pipeline request
- The pipeline response (if you
await next()) - The tracing client
- Any package specific details
Change it however you'd like. The downside is that you're now working at a lower level so you only have the raw request body / headers / etc. You'll need to do some work to get it to work for your scenario but everything should be available
| // } | ||
| const operationName = getOperationName(url.pathname); | ||
| const name = `${operationName}`.trim(); | ||
| const { span, updatedOptions } = traceClient.startSpan(name, { |
There was a problem hiding this comment.
Consider using traceClient.withSpan if you can avoid the onStartTracing and onEndTracing calls or call them at the right time. It does a lot of this for you.
| tracingOptions: pipelineRequest.tracingOptions, | ||
| }); | ||
| pipelineRequest.tracingOptions = updatedOptions.tracingOptions; | ||
| onStartTracing?.(span, [{ ...pipelineRequest, headers: {} }, name, pipelineRequest.url]); |
There was a problem hiding this comment.
headers need to be updated here since the core-rest-pipeline headers are incompatible with the core-client-rest headers. You'll just need to update some of the types (here and everywhere else I am ignoring headers for the proof-of-concept)
| onStartTracing?.(span, [{ ...pipelineRequest, headers: {} }, name, pipelineRequest.url]); | ||
|
|
||
| return traceClient | ||
| .withContext(updatedOptions.tracingOptions.tracingContext, () => next(pipelineRequest)) |
There was a problem hiding this comment.
strongly recommend using async/await here instead of chaining - I left it as close as possible to the original code to avoid confusion.
| assert.isNotNull(client.pipeline); | ||
| }); | ||
|
|
||
| it.only("tracing should work", async function () { |
There was a problem hiding this comment.
Only kept the basic test here, once you "productionalize" this code you can fix the tests to your liking
| @@ -90,6 +90,7 @@ export interface TracingContext { | |||
|
|
|||
| // @public | |||
| export interface TracingSpan { | |||
| addEvent(name: string, attributes?: Record<string, unknown>, startTime?: Date): void; | |||
There was a problem hiding this comment.
I will add this and other core-tracing changes separately - but I may need some time to work out the right API shape
|
Closing in favor of #31160 |
very rough around the edges, but this is a proof of concept that can hopefully help you get started. I will add comments explaining what I plan to add in a separate PR but hopefully you can use the logic in ai-inference to get going.
FWIW I left as much of your original code unchanged so it is easier for you to understand - feel free to ask me if you'd like me to give it a thorough review
Based off of #30800