Allow conditionally disabling propagation injection.

[pedrolcn]

Is your feature request related to a problem? Please describe. cloud-trace by default injects trace-propagation headers on all outgoing HTTP requests, this has caused problems with external services which reject requests containing unexpected headers.

Describe the solution you'd like It should be possible to pass in the configurations object a function (with a signature somwhat like (options: http.RequestOptions) => boolean) which should be called at runtime to decide if the propagation headers should be injected.

Describe alternatives you've considered A less flexible alternative although probably simpler to implement would be to allow passing in the configurations a block-list of domains for which propagation headers should NOT be injected, or maybe an allow-list of domains for which the propagation traces SHOULD be injected

Additional context The solution currently employed to circumvent this problem is to not inject propagation headers at all, which solves the problem but prevents us from propagating traces on RPCs to internal services.

[dnoe]

I'm part of the team at Google which has recently taken over the Cloud Trace project. Sorry we are only looking into this bug now. Our team is tasked with maintaining this node.js library, but we aren't JavaScript or Node.js experts. If you think you might be able to create a PR that adds the Boolean you want that would be greatly appreciated! I think this sounds like a reasonable configurable, we just need to implement it and our team is short on both time and Node.js knowledge.

I'm a little confused why the block list you mention would be less flexible, it seems more powerful and flexible to me. Can you elaborate?

Thanks!

[pedrolcn]

I'm a little confused why the block list you mention would be less flexible, it seems more powerful and flexible to me. Can you elaborate?

If we go with the domain block/allowlist the decision wether or not to inject the propagation headers can only be done based on the request's domain, if we expect a function (opts: http.RequestOptions) => boolean then we can decide to inject the headers or not based on any property of the HTTP request, such as the path, if a given header is present, the request method, etc.

While it is definitely more flexible/general I'm not sure that such flexibility is needed, for the use case which motivated me to open this issue a domain blocklist would be more than enough, and certainly would be easier to implement. Furthermore such a function would pose a composability problem, as it would need to contain logic for all cases where we do not want to inject propagation headers and inject for all others, a list is more maintainable.

How about we go with a blocklist of domains to which propagation headers should not be injected? I'm also a bit short on time, but I'll try and come up with a PR in a timely manner

[michaelsafyan]

Thanks for volunteering to code this up. From my perspective, I think a more general API would be preferable. And it would also be more consistent with the existing propagation and tracePolicy config parameters to use an object with a method that decides whether to inject trace propagation headers. If that were the approach, a DomainBlocklist implementation of that interface could be created without limiting the broader flexibility of the configuration option.

[dnoe]

Yes, as Michael mentioned, this would be very helpful if you can give us a PR. If you'd like to discuss any design decisions, questions, or proposals with the team before you get started, this issue is the best place to have that conversation. Thank you so much!

[aabmass]

✋ I can take ownership of this issue but also not sure when I'll get around to a fix. If anyone would like to submit a PR, please drop a comment!

[aabmass]

@pedrolcn I'm closing this issue for now. If it's still needed, please reply.

Happy to accept PRs of course :)