nodejs-pubsub
nodejs-pubsub copied to clipboard
feat: add support for OTel context propagation and harmonized spans
This PR adds support for OTel context propagation using the standard W3C headers as Pub/Sub attributes. It also harmonizes the spans we're using with the planned spans in other language libraries.
Fixes https://github.com/googleapis/nodejs-pubsub/issues/1389
(This is the 4.x version of https://github.com/googleapis/nodejs-pubsub/pull/1659 - I'll still be backporting to there, but the changes will go here first.)
Here is the summary of changes.
You are about to add 2 region tags.
-
samples/openTelemetryTracing.js:35, tag
opentelemetry_tracing
-
samples/typescript/openTelemetryTracing.ts:31, tag
opentelemetry_tracing
You are about to delete 1 region tag.
-
samples/openTelemetryTracing.js:40, tag
opentelemetry_tracing
This comment is generated by snippet-bot.
If you find problems with this result, please file an issue at:
https://github.com/googleapis/repo-automation-bots/issues.
To update this comment, add snippet-bot:force-run
label or use the checkbox below:
- [ ] Refresh this comment
This needs a bit more work due to some design changes in the spans.
@omBratteng Glad to see a practical use case, thanks for linking :D
@feywind looking forward to this getting merged 😎
If I understand the new changes, there's no attributes added to the receiver span, as it is expected that they are set on the sender side? https://github.com/feywind/nodejs-pubsub/blob/86491230c918dac24eb3f4bf5be8b5005267f8aa/src/telemetry-tracing.ts#L254-L277
FYI here are the changes for the latest spec updates, mostly done: https://github.com/feywind/nodejs-pubsub/pull/4
FYI here are the changes for the latest spec updates, mostly done: feywind#4
Awesome! Exciting to start using it :)
Hi @feywind, by when can i expect a stable release for this? It would be an extremely important piece which would help to avoid unnecessary patching from our end to achieve the context propagation.
Hi @feywind, by when can i expect a stable release for this? It would be an extremely important piece which would help to avoid unnecessary patching from our end to achieve the context propagation.
I completely agree with you. The best I can say for full stable release (which has to include doc updates) is "aiming for 2024Q1". I'm hoping to get a preview release much sooner.
Looking forward to preview release
@feywind @kamalaboulhosn is there anything we can do to help this get merged?
If not, can a smaller incremental change where the w3c context is propagated be merged sooner?
The googclient_OpenTelemetrySpanContext
is not very useful when working with other services that use w3c context.
Thanks!
@naseemkullah The delay has been annoying for sure. The good news is, this is being reviewed in pieces and there will hopefully be a beta release soon.
There's one more of these in the review queue right now: https://github.com/feywind/nodejs-pubsub/pull/5
Some fabulous people on our side have been working to harmonize our span design a bit more with the OTel working group's design in progress for message tracing. :)
@hongalex The sample needs updating, but this should more or less be the finished PR now. Thanks for all the review help.
This is now testable in beta form. I installed in a test project like so:
npm i --save @google-cloud/pubsub@otel-beta
The resulting package.json looks like this:
{
"dependencies": {
"@google-cloud/opentelemetry-cloud-trace-exporter": "^2.1.0",
"@google-cloud/pubsub": "^4.3.3-otel-beta.1",
"@opentelemetry/sdk-trace-node": "^1.23.0",
"@opentelemetry/semantic-conventions": "^1.23.0"
}
}
And then you can take a look at this sample for how to use it from JS, or this sample for how to use it from TS.
We've got more reviewing to do, and merges aren't going to happen for at least a week, but this may at least make it easier to do some testing if you like.
Further updates (for now) will come in as 4.3.3-otel-beta.2
, etc, and will update the tag otel-beta
on npm.