feat(rack): add public_request config option - #2493
Conversation
|
|
| # | ||
| # @param [Rack::Request::Env] The incoming HTTP request environment | ||
| # @return [Boolean] | ||
| option :public_request, default: nil, validate: :callable |
There was a problem hiding this comment.
Does the option have to be callable? From the test case, it seems only testing true/false (most time options tend to avoid use callable).
There was a problem hiding this comment.
I was thinking that in some cases the endpoint might have both public and private ingress so based on some headers or other information from the request the configuration could determine how to create the span.
There was a problem hiding this comment.
In that case, I am thinking maybe it's better to have an array that include list of allowing public_request e.g. untraced_hosts
There was a problem hiding this comment.
Not sure I follow, would the array have a list of where the request originated from or a list of endpoints to determine how to create the span?
In case it helps - to add the use case I was thinking before is that for different Rack applications they might configure what is considered a public vs. private request on the URL path, headers, or other request information so having the callable seemed the most flexible way to meet the various ways the public/private distinction could come up. Is the concern with the callable that it's more difficult to configure?
There was a problem hiding this comment.
Yes, that's the only concern since callable gives too much freedom.
Although, it doesn't mean it can't be callable.
Could you add more test case where shows the callable is necessary.
thompson-tomo
left a comment
There was a problem hiding this comment.
I am wondering if this is not something we should be raising at the spec level for a consistent experience.
The nicest solution for me is if we could have a remoteContextAnalyzer which is triggered when creating a span. The analyzer would decide based on context & attributes if it should be a parent, link or dropped.
This would enable all instrumentation to benefit etc.
From this post #1985
This change adds a new configuration option for the rack instrumentation called
public_requestthat mirrors the behavior in the Go SDK above. By default, this option has the same behavior the rack instrumentation has currently, which is to create a child association to the incoming span. I called the optionpublic_requestinstead of something likepublic_endpointbecause the configuration runs for each request and a given endpoint may be used publicly and privately, but it depends on the request to determine whether it's one or the other. I didn't see a previous example of documenting an option for acallableso added some information that also mirrors the Go otelhttp SDK, but can update that to match what's expected from the maintainers.