-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
[confighttp]: add an option to add span formatter #11230
base: main
Are you sure you want to change the base?
Changes from all commits
cff8392
ce61bb5
c871eaf
06d3361
f2641b8
e4b5fb7
9171bb7
63d77c8
dffbad7
e6ab7d5
6018385
4fe1105
985a5d9
43d9d0d
f778f3a
e2d4dca
d1e3078
2deac0e
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 |
---|---|---|
@@ -0,0 +1,25 @@ | ||
# Use this changelog template to create an entry for release notes. | ||
|
||
# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix' | ||
change_type: enhancement | ||
|
||
# The name of the component, or a single word describing the area of concern, (e.g. otlpreceiver) | ||
component: confighttp | ||
|
||
# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). | ||
note: Add an option to add prefix for span name for components | ||
|
||
# One or more tracking issues or pull requests related to the change | ||
issues: [9382] | ||
|
||
# (Optional) One or more lines of additional information to render under the primary note. | ||
# These lines will be padded with 2 spaces and then inserted directly into the document. | ||
# Use pipe (|) for multiline entries. | ||
subtext: | ||
|
||
# Optional: The change log or logs in which this entry should be included. | ||
# e.g. '[user]' or '[user, api]' | ||
# Include 'user' if the change is relevant to end users. | ||
# Include 'api' if there is a change to a library API. | ||
# Default: '[user]' | ||
change_logs: [] | ||
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -379,6 +379,7 @@ func (hss *ServerConfig) ToListener(ctx context.Context) (net.Listener, error) { | |
type toServerOptions struct { | ||
errHandler func(w http.ResponseWriter, r *http.Request, errorMsg string, statusCode int) | ||
decoders map[string]func(body io.ReadCloser) (io.ReadCloser, error) | ||
formater func(string, *http.Request) string | ||
} | ||
|
||
// ToServerOption is an option to change the behavior of the HTTP server | ||
|
@@ -412,11 +413,21 @@ func WithDecoder(key string, dec func(body io.ReadCloser) (io.ReadCloser, error) | |
}) | ||
} | ||
|
||
// WithSpanFormatter specifies which formater to use for span. | ||
// Ideally, this prefix in span name should be the component's ID. | ||
func WithSpanFormatter(formater func(string, *http.Request) string) ToServerOption { | ||
return toServerOptionFunc(func(opts *toServerOptions) { | ||
opts.formater = formater | ||
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. if formater != nil {
// ....
} |
||
}) | ||
} | ||
|
||
// ToServer creates an http.Server from settings object. | ||
func (hss *ServerConfig) ToServer(_ context.Context, host component.Host, settings component.TelemetrySettings, handler http.Handler, opts ...ToServerOption) (*http.Server, error) { | ||
internal.WarnOnUnspecifiedHost(settings.Logger, hss.Endpoint) | ||
|
||
serverOpts := &toServerOptions{} | ||
serverOpts := &toServerOptions{ | ||
formater: PrefixFormatter(""), // use empty-prefix formater by default | ||
} | ||
for _, o := range opts { | ||
o.apply(serverOpts) | ||
} | ||
|
@@ -464,14 +475,11 @@ func (hss *ServerConfig) ToServer(_ context.Context, host component.Host, settin | |
otelOpts := []otelhttp.Option{ | ||
otelhttp.WithTracerProvider(settings.TracerProvider), | ||
otelhttp.WithPropagators(otel.GetTextMapPropagator()), | ||
otelhttp.WithSpanNameFormatter(func(_ string, r *http.Request) string { | ||
return r.URL.Path | ||
}), | ||
otelhttp.WithSpanNameFormatter(serverOpts.formater), | ||
otelhttp.WithMeterProvider(getLeveledMeterProvider(settings)), | ||
} | ||
|
||
// Enable OpenTelemetry observability plugin. | ||
// TODO: Consider to use component ID string as prefix for all the operations. | ||
handler = otelhttp.NewHandler(handler, "", otelOpts...) | ||
|
||
// wrap the current handler in an interceptor that will add client.Info to the request's context | ||
|
@@ -556,6 +564,15 @@ func maxRequestBodySizeInterceptor(next http.Handler, maxRecvSize int64) http.Ha | |
}) | ||
} | ||
|
||
func PrefixFormatter(prefix string) func(string, *http.Request) string { | ||
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. why public? |
||
return func(_ string, r *http.Request) string { | ||
if len(prefix) > 0 { | ||
return fmt.Sprintf("%s:%s", prefix, r.URL.Path) | ||
} | ||
return r.URL.Path | ||
} | ||
} | ||
|
||
func getLeveledMeterProvider(settings component.TelemetrySettings) metric.MeterProvider { | ||
if configtelemetry.LevelDetailed <= settings.MetricsLevel { | ||
return settings.MeterProvider | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -162,7 +162,10 @@ func (r *otlpReceiver) startHTTPServer(ctx context.Context, host component.Host) | |
} | ||
|
||
var err error | ||
if r.serverHTTP, err = r.cfg.HTTP.ToServer(ctx, host, r.settings.TelemetrySettings, httpMux, confighttp.WithErrorHandler(errorHandler)); err != nil { | ||
if r.serverHTTP, err = r.cfg.HTTP.ToServer(ctx, host, r.settings.TelemetrySettings, httpMux, | ||
confighttp.WithErrorHandler(errorHandler), | ||
confighttp.WithSpanFormatter(confighttp.PrefixFormatter(r.settings.ID.String())), | ||
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. Is this a breaking change? If this is merged will the spans that the otlpreceiver creates about itself have new names? 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. @TylerHelmuth Is your concern that we should not do this if it is a breaking change? 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. If we want to do this, we can use #11769 now :) |
||
); err != nil { | ||
return err | ||
} | ||
|
||
|
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.
api