-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
feat: informative user-agent for otlp exporters #3970
Changes from 13 commits
3d72c7b
fa5942c
13ad211
8fd2bd7
c945d07
efd1fc1
672b7f6
0a86588
33bc09f
386a70b
ab6adb1
0b44ddf
511b636
f5bddaf
b201e25
e232439
0ac32d6
a8bd4ee
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -57,9 +57,14 @@ const ( | |
) | ||
|
||
// Crete new exporter. | ||
func newExporter(cfg config.Exporter, logger *zap.Logger) (*exporter, error) { | ||
func newExporter(cfg config.Exporter, logger *zap.Logger, buildInfo component.BuildInfo) (*exporter, error) { | ||
oCfg := cfg.(*Config) | ||
|
||
// Configure default user-agent header | ||
oCfg.HTTPClientSettings.CustomRoundTripper = func(next http.RoundTripper) (http.RoundTripper, error) { | ||
return newUserAgentRoundTripper(next, exporterhelper.DefaultUserAgent(buildInfo)), nil | ||
} | ||
|
||
if oCfg.Endpoint != "" { | ||
_, err := url.Parse(oCfg.Endpoint) | ||
if err != nil { | ||
|
@@ -213,3 +218,32 @@ func readResponse(resp *http.Response) *status.Status { | |
|
||
return respStatus | ||
} | ||
|
||
type userAgentRoundTripper struct { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we really need a roundtripper? Since this is private, we should just simply add a header to the request when creating the request instead of cloning etc., as we need to do for a proper roundtripper. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I did consider just adding the header when request is created, which is easier but more error-prone (need to remember to add the header again, if there's ever a new codepath that creates requests). That being said, I expect the exporter is fairly stable at this point, so this should be OK? f5bddaf |
||
next http.RoundTripper | ||
userAgent string | ||
} | ||
|
||
func newUserAgentRoundTripper(next http.RoundTripper, userAgent string) *userAgentRoundTripper { | ||
return &userAgentRoundTripper{ | ||
next, | ||
userAgent, | ||
} | ||
} | ||
|
||
func (r *userAgentRoundTripper) RoundTrip(req *http.Request) (*http.Response, error) { | ||
body, readErr := ioutil.ReadAll(req.Body) | ||
closeErr := req.Body.Close() | ||
if readErr != nil { | ||
return nil, readErr | ||
} | ||
if closeErr != nil { | ||
return nil, closeErr | ||
} | ||
requestClone := req.Clone(req.Context()) | ||
requestClone.Body = ioutil.NopCloser(bytes.NewReader(body)) | ||
if requestClone.Header.Get("User-Agent") == "" { | ||
requestClone.Header.Set("User-Agent", r.userAgent) | ||
} | ||
return r.next.RoundTrip(requestClone) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let's move this to otlpexporter for the moment or an internal package. Prefer to limit the public API.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
✅ Moved to each exporter f5bddaf