Add jdk.httpserver instrumentation#13243
Conversation
|
|
|
|
||
| @Override | ||
| public String getHttpRoute(HttpExchange exchange) { | ||
| return exchange.getHttpContext().getPath(); |
There was a problem hiding this comment.
Route must have a low cardinality. Often it is not possible to get route for a low level server framework. Just return null here.
There was a problem hiding this comment.
I don't follow, exchange.getHttpContext().getPath() gives you the configured path for the route, not what is currently matched
There was a problem hiding this comment.
that is, if you do
HttpServer server = HttpServer.create(new InetSocketAddress(port), 0);
server.createContext("/path/" ctx ->{});and send a request to /path/something, you'll get /path/ from exchange.getHttpContext().getPath()
|
@laurit perchance do you know why it's failing now? |
I haven't followed the discussions above, but looks like its: you could opt-out of the http pipelining test initially at least, look for |
|
what's the best way to search these error logs, I'm having trouble locating the actual tests that are failing. |
9c36483 to
10b3679
Compare
|
So I add a Readme and the pipeline fails? |
we have been doing some compliance work in the repo recently which introduced a couple of new required PR checks, and so just need to rebase to |
trask
left a comment
There was a problem hiding this comment.
thanks @SentryMan @laurit! one suggestion on the package name
Adds instrumentation for the
jdk.httpservermodule.will resolve #13216 when I get the agent working