we have the use case of extracting the request body for our post/put methods.
we are trying get that from the getter @Getter private final FullHttpRequest h1Request in XrpcRequest.java . We are thinking to use getH1Request().content().
But the getH1Request is returning null.

We need someway of passing the request body to the service handlers as a byte buffer or a strongly typed object (similar to spring controllers where we get body as an object @RequestBody myobject) or even as string.

for example, for the path:
/people/{person}

the following uri
/people/patrick?exampleParam=something

produces the following context:
{person: "patrick?exampleParam=something"}

we expect that it would be:
{person: "patrick"}

 Handler personHandler =
        context -> {
          Person p = new Person(context.variable("person"));
          people.add(p);

          return Recipes.newResponseOk("");
        };

from Example.java

if your url is something like /people/Samwise%20Gamgee, context.variable("person") will equal Samwise%20Gamgee. It may be more friendly if xrpc does the Url decoding.

A Null pointer is thrown when return a status of 202 is returned from a handler.
I'm guessing that any response code not registered in Router.configResponseCodeMeters() will
cause this issue.

9969 [xrpc-worker-1] WARN  i.n.c.DefaultChannelPipeline - An exceptionCaught() event was fired, and it reached at the tail of the pipeline. It usually means the last handler in the pipeline did not handle the exception.
java.lang.NullPointerException: null
	at com.nordstrom.xrpc.server.UrlRouter.channelRead(UrlRouter.java:78)
	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:362)
	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:348)
	at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:340)
	at io.netty.handler.codec.MessageToMessageDecoder.channelRead(MessageToMessageDecoder.java:102)
	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:362)
	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:348)
	at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:340)
	at io.netty.channel.CombinedChannelDuplexHandler$DelegatingChannelHandlerContext.fireChannelRead(CombinedChannelDuplexHandler.java:438)
	at io.netty.handler.codec.ByteToMessageDecoder.fireChannelRead(ByteToMessageDecoder.java:310)
	at io.netty.handler.codec.ByteToMessageDecoder.channelRead(ByteToMessageDecoder.java:284)
	at io.netty.channel.CombinedChannelDuplexHandler.channelRead(CombinedChannelDuplexHandler.java:253)
	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:362)
	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:348)
	at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:340)
	at com.nordstrom.xrpc.logging.ExceptionLogger.channelRead(ExceptionLogger.java:29)
	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:362)
	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:348)
	at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:340)
	at io.netty.handler.ssl.SslHandler.unwrap(SslHandler.java:1380)
	at io.netty.handler.ssl.SslHandler.decodeNonJdkCompatible(SslHandler.java:1171)
	at io.netty.handler.ssl.SslHandler.decode(SslHandler.java:1196)
	at io.netty.handler.codec.ByteToMessageDecoder.decodeRemovalReentryProtection(ByteToMessageDecoder.java:489)
	at io.netty.handler.codec.ByteToMessageDecoder.callDecode(ByteToMessageDecoder.java:428)
	at io.netty.handler.codec.ByteToMessageDecoder.channelRead(ByteToMessageDecoder.java:265)
	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:362)
	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:348)
	at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:340)
	at io.netty.channel.ChannelInboundHandlerAdapter.channelRead(ChannelInboundHandlerAdapter.java:86)
	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:362)
	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:348)
	at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:340)
	at io.netty.channel.ChannelInboundHandlerAdapter.channelRead(ChannelInboundHandlerAdapter.java:86)
	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:362)
	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:348)
	at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:340)
	at com.nordstrom.xrpc.server.ConnectionLimiter.channelRead(ConnectionLimiter.java:55)
	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:362)
	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:348)
	at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:340)
	at io.netty.handler.timeout.IdleStateHandler.channelRead(IdleStateHandler.java:286)
	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:362)
	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:348)
	at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:340)
	at io.netty.channel.DefaultChannelPipeline$HeadContext.channelRead(DefaultChannelPipeline.java:1359)
	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:362)
	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:348)
	at io.netty.channel.DefaultChannelPipeline.fireChannelRead(DefaultChannelPipeline.java:935)
	at io.netty.channel.kqueue.AbstractKQueueStreamChannel$KQueueStreamUnsafe.readReady(AbstractKQueueStreamChannel.java:543)
	at io.netty.channel.kqueue.AbstractKQueueChannel$AbstractKQueueUnsafe.readReady(AbstractKQueueChannel.java:402)
	at io.netty.channel.kqueue.KQueueEventLoop.processReady(KQueueEventLoop.java:195)
	at io.netty.channel.kqueue.KQueueEventLoop.run(KQueueEventLoop.java:269)
	at io.netty.util.concurrent.SingleThreadEventExecutor$5.run(SingleThreadEventExecutor.java:886)
	at java.base/java.lang.Thread.run(Thread.java:844)

It's possible with netty to send an Http2DataFrame before h2 has finished it's handshake. We need to ensure that outbound Http2DataFrames are queued until the handshake is complete.

we have the use case of extracting the query string parameters in the request
we are trying get that from the getter @Getter private final FullHttpRequest h1Request in XrpcRequest.java
But this is returning null.

We need someway of passing the query params as a hash map to the service handlers.

There is currently an NPE that is thrown at startup when the health checks are being scheduled against a worker thread pool that doesn't exist yet. This issue is being filed to fix that bug as well as simplify the invocation of the admin handlers.

Exception in thread "main" java.lang.NullPointerException
	at com.nordstrom.xrpc.server.Router.scheduleHealthChecks(Router.java:130)
	at com.nordstrom.xrpc.server.Router.scheduleHealthChecks(Router.java:126)
	at com.nordstrom.xrpc.demo.Example.main(Example.java:156)

The background health check results are silently discarded, which isn't very helpful.

These should be stored in a place that can be used by the /health handler.

The issue here is that the current approach requires that, for a release to happen, the person running a release needs to have push permissions to master (needs to be a project admin). This isn't great. Having a shared release branch would fix this problem: Any person with normal write permissions could trigger a release.
See discussion in #82

https://github.com/openzipkin/brave/tree/master/brave

See ^ for impl details. The first pass should leverage the OkHttp reporter and as a feature add, we should create a XrpcClient Reporter

The health registry function only adds a check into a local map.

The checks are only registered when the background health checker starts up, which can be disabled by a boolean flag.

These should always be registered.

The HTTP/2 implementation has some possible / probable bugs:

1. Data frames data is not stored or saved unless endOfStream is set. It looks like this will drop all but the last frame received.
2. All bytes are marked as processed even if they are ignored due to endOfStream being set. This looks incorrect.
3. Data handling assumes a request is set, which will probably throw an exception for HTTP/2 requests that don't match a handler.
4. Error handling doesn't return, which results in two responses being written (not found and server error).

Maybe this: https://github.com/hierynomus/license-gradle-plugin

eg :-

router.addRoute("/people/{person}", personHandler, HttpMethod.GET);
router.addRoute("/people/{person}", someotherhandler, HttpMethod.PUT);

This file has merge conflicts in the xrpc master -
https://github.com/Nordstrom/xrpc/blob/master/src/main/java/com/nordstrom/xrpc/client/XUrl.java

In the imports , we have merge conflicts which causes build errors

Please see #51 for context

Currently there is no demo of config overrides, and the config loading heavily relies on the classpath.

We should use a scoped class resource, and provide an override example.

#46 (review)

#46

Include a link to netty native docs in the new class:
https://netty.io/wiki/native-transports.html

https://github.com/Nordstrom/xrpc/pull/38/files#r157036367
[#38]

Jackson is already an xrpc dependency; the demo should use this instead of Moshi.

It's also easier-to-use and faster.

Gradle has release plugins available to handle the (increment version + commit, tag repository, build release jar, commit snapshot version, push to github) workflow.

We should add one here (and publish an 0.1 version).

Current Exp Backoff is limited to timeouts. A new one should be added to take into consideration errors in responses. This should have different configuration options than the timeout based backoffs. We should potentially look into adding a (optional) Dead Letter Queue for failed requests.

#38 (comment)

"/info" route needs to be added which responds with which git commit id, app version number and commit authors. This info could be configurable via variables in the app.conf

This line should be matching on the path ONLY. This matches query parameters, since it hits the whole URI.

Respect the X-Forwarded-For header during rate limiting. This is a problem as we are currently rate limiting prior to the codec.

Currently in Router.java , we have our custom netty channel pipelines like rate limiter, connection limiter etc.
Users should get the ability to add additional pipelines like global exception handler , authorizer etc and should get the ability to specify the order of execution of pipelines.

Feature request: Have healthchecks exposed somewhere.

See Dropwizard's metrics API.

Tracking a discussion.

see #55 for context.

A third option is adopting a hard limit / soft limit where soft limit responds and hard limit drops silently.

Create an Http/2 client.

• This could be implemented as a single client that auto-negotiates protocols.
• This could be implemented as 2 separate clients, with the appropriate interfaces
• This could be a simple wrapper around okhttp3

We need an ability to specify the CORS headers or any additional response headers.
Currently we are doing that in Recipes.java. We need to expose that as parameter in the method so that we can pass custom response header to the client.

For CORS enabling , we just added response.headers().set(ACCESS_CONTROL_ALLOW_ORIGIN, "*");

public static FullHttpResponse newResponse(
HttpResponseStatus status, ByteBuf buffer, ContentType contentType) {
FullHttpResponse response = new DefaultFullHttpResponse(HttpVersion.HTTP_1_1, status, buffer);

response.headers().set(CONTENT_TYPE, contentType.value);
response.headers().setInt(CONTENT_LENGTH, buffer.readableBytes());
response.headers().set(ACCESS_CONTROL_ALLOW_ORIGIN, "*");

return response;

}

see #92 discussion

We have per-handler meters, and should add per-handler Timers.

We should also come up with a good naming scheme for these, possibly fixing #104 first.

the following need to be implemented

'/info' -> should expose version number, git commit number, etc
'/metrics' -> should return the metrics reporters in JSON format
'/health' -> should expose a summary of downstream health checks
'/ping' -> should respond with a 200-OK status code and the text 'PONG'
'/ready' -> should expose a Kubernetes or ELB specific healthcheck for liveliness
'/restart' -> restart service (should be restricted to approved devs / tooling)
'/killkillkill' -> shutdown service (should be restricted to approved devs / tooling)```

this issue supersedes #52  

#38 (comment)

build.gradle has some dependencies on publication properties being set in order to run basic builds. This should be fixed.

Replace or enhance the wrk regression script with JMH benchmarks... See https://blog.codecentric.de/en/2017/10/performance-measurement-with-jmh-java-microbenchmark-harness/

Expected behavior

Consistent naming between metrics

Actual behavior

Inconsistent naming between metrics

Steps to reproduce

/metrics endpoint

Minimal yet complete reproducer code (or URL to code)

#102 (comment)

xrpc version

0.2.1

Sample output of metrics endpoint

{
  "version" : "3.1.3",
  "gauges" : { },
  "counters" : {
    "com.nordstrom.xrpc.server.Router.Active Connections" : {
      "count" : 1
    }
  },
  "histograms" : { },
  "meters" : {
    "Hard Rate Limits" : {
      "count" : 0,
      "m15_rate" : 0.0,
      "m1_rate" : 0.0,
      "m5_rate" : 0.0,
      "mean_rate" : 0.0,
      "units" : "events/second"
    },
    "com.nordstrom.xrpc.server.Router.requests.Rate" : { 
      "count" : 2,
      "m15_rate" : 4.405910322420641E-4,
      "m1_rate" : 1.5708635764593015E-8,
      "m5_rate" : 2.0783399443213604E-4,
      "mean_rate" : 0.0023767358720068054,
      "units" : "events/second"
    },
    "requests" : {
      "count" : 0,
      "m15_rate" : 0.0,
      "m1_rate" : 0.0,
      "m5_rate" : 0.0,
      "mean_rate" : 0.0,
      "units" : "events/second"
    },
    "responseCodes.accepted" : {
      "count" : 0,
      "m15_rate" : 0.0,
      "m1_rate" : 0.0,
      "m5_rate" : 0.0,
      "mean_rate" : 0.0,
      "units" : "events/second"
    },
    "responseCodes.badRequest" : {
      "count" : 0,
      "m15_rate" : 0.0,
      "m1_rate" : 0.0,
      "m5_rate" : 0.0,
      "mean_rate" : 0.0,
      "units" : "events/second"
    },
    "responseCodes.created" : {
      "count" : 0,
      "m15_rate" : 0.0,
      "m1_rate" : 0.0,
      "m5_rate" : 0.0,
      "mean_rate" : 0.0,
      "units" : "events/second"
    },
    "responseCodes.forbidden" : {
      "count" : 0,
      "m15_rate" : 0.0,
      "m1_rate" : 0.0,
      "m5_rate" : 0.0,
      "mean_rate" : 0.0,
      "units" : "events/second"
    },
    "responseCodes.noContent" : {
      "count" : 0,
      "m15_rate" : 0.0,
      "m1_rate" : 0.0,
      "m5_rate" : 0.0,
      "mean_rate" : 0.0,
      "units" : "events/second"
    },
    "responseCodes.notFound" : {
      "count" : 0,
      "m15_rate" : 0.0,
      "m1_rate" : 0.0,
      "m5_rate" : 0.0,
      "mean_rate" : 0.0,
      "units" : "events/second"
    },
    "responseCodes.ok" : {
      "count" : 1,
      "m15_rate" : 4.405910322420641E-4,
      "m1_rate" : 1.5708635764593015E-8,
      "m5_rate" : 2.0783399443213604E-4,
      "mean_rate" : 0.0011882757510781091,
      "units" : "events/second"
    },
    "responseCodes.serverError" : {
      "count" : 0,
      "m15_rate" : 0.0,
      "m1_rate" : 0.0,
      "m5_rate" : 0.0,
      "mean_rate" : 0.0,
      "units" : "events/second"
    },
    "responseCodes.tooManyRequests" : {
      "count" : 0,
      "m15_rate" : 0.0,
      "m1_rate" : 0.0,
      "m5_rate" : 0.0,
      "mean_rate" : 0.0,
      "units" : "events/second"
    },
    "responseCodes.unauthorized" : {
      "count" : 0,
      "m15_rate" : 0.0,
      "m1_rate" : 0.0,
      "m5_rate" : 0.0,
      "mean_rate" : 0.0,
      "units" : "events/second"
    }
  },
  "timers" : {
    "Request Latency" : {
      "count" : 1,
      "max" : 164.399295,
      "mean" : 164.399295,
      "min" : 164.399295,
      "p50" : 164.399295,
      "p75" : 164.399295,
      "p95" : 164.399295,
      "p98" : 164.399295,
      "p99" : 164.399295,
      "p999" : 164.399295,
      "values" : [ 164.399295 ],
      "stddev" : 0.0,
      "m15_rate" : 4.405910322420641E-4,
      "m1_rate" : 1.5708635764593015E-8,
      "m5_rate" : 2.0783399443213604E-4,
      "mean_rate" : 0.0011883696639821677,
      "duration_units" : "milliseconds",
      "rate_units" : "calls/second"
    }
  }
}

Metrics should be exposed on a (possibly-configurable) HTTP endpoint.

I think the newResponse method in Recipes.java should return
a FullHttpResponse object and not a DefaultFullHttpResponse.
Http2Handler typecasts event handler responses to FullHttpResponse leading to an exception

if a rate limit is exceeded the client connection should receive a 429 and the connection should be dropped.

Testing github integration.

The router won't let you bind different handlers to different HTTP methods - for example, if you wanted to support GET for lookups and POST for writes.