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

why googclient_traceparent, why not just traceparent #2011

Open
naseemkullah opened this issue Feb 6, 2025 · 4 comments
Open

why googclient_traceparent, why not just traceparent #2011

naseemkullah opened this issue Feb 6, 2025 · 4 comments
Labels
api: pubsub Issues related to the googleapis/nodejs-pubsub API. priority: p3 Desirable enhancement or fix. May not be included in next release. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.

Comments

@naseemkullah
Copy link

export const modernAttributeName = 'googclient_traceparent';

I noticed that the client sets + extracts trace context using googclient_traceparent instead of the standard traceparent. This makes it incompatible with vanilla OpenTelemetry context propagation, requiring manual extraction and re-injection.

  • Is there a specific reason for this customization?
  • Would it be possible to use the standard traceparent key, or at least provide an option to enable it?

This would improve interoperability with OpenTelemetry out of the box.

Thanks!

@product-auto-label product-auto-label bot added the api: pubsub Issues related to the googleapis/nodejs-pubsub API. label Feb 6, 2025
@hongalex
Copy link
Member

hongalex commented Feb 7, 2025

Our current policy is that any attributes created by the client library must be preprended with goog to differentiate them from user provided attributes. These attributes are also reserved such that user-defined attributes starting with googclient are not allowed.

A lesser reason is that we want to avoid taking up attribute keys in the user space as that is technically a breaking change. For example, if some people were setting traceparent before. Of course, we could get around this by checking to make sure traceparent is empty before having the library inject this value, and/or to look into an option to enable traceparent.

Could you describe your desired use case a bit more? I'm guessing you have some process downstream from receiving a message that would be able to extract the Pub/Sub message if you're using pull? But in that case, you will still need to manually read from Pub/Sub attributes. Alternatively, is it that you're using push subscriptions?

@naseemkullah
Copy link
Author

Hey, thanks for explaining!

Re: current process
The team may want to revisit this if they consider OpenTelemetry an industry standard and want to align with a more interoperable approach.

Agree with your solution to the lesser reason.

My use case is a real-world potpourri languages and instrumentation libraries, where the thing stitching spans together is the W3C Trace Context (traceparent/tracestate, without the googclient_ prefix.

The goal is to create a span anytime a subscriber attempts to handle and ack/nack a message - but I admit this isn't strictly tied to what key names are used for trace propagation. The key issue however, is referencing the publisher's span, which does follow W3C standards(traceparent/tracestate) —either by keeping it under the same trace or, preferably, using a span link (to avoid bloated traces in fan-out scenarios).

Anyway, I have a workaround—manual context extraction and span creation with span links—but this could be built into the client.

Proposal:

  1. Add support for injecting/extracting traceparent/tracestate without the googclient_ prefix
  2. Allow users to configure whether the extracted context is used to:
    a. create a new span in the same trace or
    b. to (extract span context from and) be added as a link to the automatically created subscriber span

@feywind
Copy link
Collaborator

feywind commented Feb 10, 2025

Thanks for the suggestions! Our context propagation stuff is still up for improvement, for sure. The general idea for the support we've got now is that it's a mechanism to plumb trace IDs through Pub/Sub, between the client libraries. Interoperating with other pieces of the ecosystem seems like a good goal in general. For example, last I heard, there is a desire to be able to pass on the trace IDs through headers in push subscriptions, so it would just connect transparently.

Is the basic desire to be able to have more control over the tree of produced spans? Like being able to parent them under another span that's user-controlled?

@feywind feywind added type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design. priority: p3 Desirable enhancement or fix. May not be included in next release. labels Feb 11, 2025
@naseemkullah
Copy link
Author

The basic desire is twofold:

  1. Use standard W3C trace context: Have the client inject/extract the widely adopted W3C key names (traceparent/tracestate) instead of Google-specific ones (i.e., without the googclient_ prefix).

  2. Gain more control over context extraction behaviour:
    a. Parent-child hierarchy: Continue the current approach where the message handling span is a child of the received message’s span context.
    b. Span links: Optionally, add the received message’s context as a span link to the message handling span. This is useful in high-fanout scenarios to prevent massive traces and, from what I gather, is the recommended approach for correlating producer and consumer spans.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: pubsub Issues related to the googleapis/nodejs-pubsub API. priority: p3 Desirable enhancement or fix. May not be included in next release. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.
Projects
None yet
Development

No branches or pull requests

3 participants