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

propose: contrib/net/http: add conditional propagation logic to roundTripper configuration #2823

Open
eyasy1217 opened this issue Aug 20, 2024 · 6 comments
Assignees
Labels
apm:ecosystem contrib/* related feature requests or bugs enhancement quick change/addition that does not need full team approval

Comments

@eyasy1217
Copy link
Contributor

I want to determine whether to propagate or not for each api I call.
For example, when calling a external api, no propagation is performed.
I assume the following code.

// https://github.com/DataDog/dd-trace-go/blob/e4985df5f9ef6a5b1f5cc9f72b03ddddcb4af0bc/contrib/net/http/option.go#L138
type roundTripperConfig struct {
	// ...
+	propagationFunc func(*http.Request) bool
	// ...
}


func (rt *roundTripper) RoundTrip(req *http.Request) (res *http.Response, err error) {
	// ...

	// https://github.com/DataDog/dd-trace-go/blob/e4985df5f9ef6a5b1f5cc9f72b03ddddcb4af0bc/contrib/net/http/roundtripper.go#L73
-	if rt.cfg.propagation {
+	if rt.cfg.propagation && rt.cfg.propagationFunc(r2) {
@eyasy1217 eyasy1217 added the enhancement quick change/addition that does not need full team approval label Aug 20, 2024
@github-actions github-actions bot added apm:ecosystem contrib/* related feature requests or bugs needs-triage New issues that have not yet been triaged labels Aug 20, 2024
@darccio darccio removed the needs-triage New issues that have not yet been triaged label Aug 26, 2024
@darccio
Copy link
Member

darccio commented Aug 26, 2024

Hello @eyasy1217, thanks for reaching out. We'll review this during our next gardening session.

@darccio
Copy link
Member

darccio commented Sep 3, 2024

@eyasy1217 What would be the use case? We found it interesting and we are willing to implement it, but we would like to understand the use case. For instance, is it to avoid sending propagation headers to a third party API that isn't able to handle correctly unknown headers?

@eyasy1217
Copy link
Contributor Author

It's not such a big use case.
Same reason as #1166 .

@davekerr-orum
Copy link

we have encountered third party api's that limit the # of headers we can send in our requests, being able to disable the propagation headers for that specific request would be useful addition

@mtoffl01
Copy link
Contributor

mtoffl01 commented Nov 4, 2024

@davekerr-orum and/or @eyasy1217 , are you able to share the third party API(s) enforcing this limit? An example would be useful for us to better understand the scope of this problem.

@mtoffl01 mtoffl01 self-assigned this Nov 4, 2024
@rarguelloF
Copy link
Contributor

For anyone interested, you can achieve the desired behavior by chaining your our custom transport with the Datadog one.

Example:

type removeDDHeadersTransport struct {
	base http.RoundTripper
}

func (i *removeDDHeadersTransport) RoundTrip(req *http.Request) (*http.Response, error) {
	for name, _ := range req.Header {
		if strings.HasPrefix(name, "X-Datadog") {
		    fmt.Printf("removing Datadog header: %s\n", name)
			req.Header.Del(name)
		}
	}
	return i.base.RoundTrip(req)
}


func main() {
    // ... initialize the tracer etc.
   	rt := httptrace.WrapRoundTripper(&removeDDHeadersTransport{base: http.DefaultTransport})
	client := &http.Client{Transport: rt}

   // do stuff with client
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
apm:ecosystem contrib/* related feature requests or bugs enhancement quick change/addition that does not need full team approval
Projects
None yet
Development

No branches or pull requests

5 participants