data-prepper icon indicating copy to clipboard operation
data-prepper copied to clipboard

Enable HTTP Health Check for OTelTraceSource and OTelMetricsSource

Open dinujoh opened this issue 2 years ago • 4 comments

Is your feature request related to a problem? Please describe. As a customer using DataPrepper to ingest OTel Trace and Metrics data using HTTP (without gRPC wire format), i want to support HTTP health check in OTelTraceSource and OTelMetricsSource because i would want to do health check on the source using HTTP and not using gRPC.

Describe the solution you'd like Introduce new configuration to the OTelTraceSource and OTelMetricsSource health_check_service_type. Possible values are GRPC and HTTP, default is GRPC (backward compatible). When health_check_service_type is GRPC existing HealthGrpcService will be used. When health_check_service_type is HTTP, HealthCheckService will be used that responds with HTTP status "200 OK" if the server is healthy and can accept requests and HTTP status "503 Service Not Available" if the server is unhealthy and cannot accept requests.

source:
    otel_trace_source:
      health_check_service: true
      health_check_service_type: HTTP
      proto_reflection_service: true

Describe alternatives you've considered (Optional) Another alternative is support both GRPC and HTTP health check on OTelTraceSource and OTelMetricsSource. Additional configuration parameter will be added to enable HTTP health check. The current health_check_service will be used to enable GRPC health check and new configuration http_health_check_service will be used to enable HTTP health check.

dinujoh avatar Jun 28 '22 16:06 dinujoh

I'm interested in pursuing something along the lines of the alternative solution. I think it would be based on the logic of both having unframed requests enabled and health checks enabled.

e.g.

if( unframed_requests == true and health_check_service == true) {
  enableHttpHealthCheck()
}

What do you think?

dlvenable avatar Jun 28 '22 17:06 dlvenable

Yeah, this would be one less configuration to add. We would rely on unframed_requests configuration to enable HTTP health check.

if( health_check_service == true) {
  if( unframed_requests == true ) {
     enableHttpHealthCheck()
  } else {
     enableGRPCHealthCheck()
  }
}

or have option to enable HTTP health check on top of GRPC health check:

if( health_check_service == true) {
  enableGRPCHealthCheck();
  
  if( unframed_requests == true ) {
     enableHttpHealthCheck()
  }
}

dinujoh avatar Jun 28 '22 17:06 dinujoh

I'm leaning towards having option to enable HTTP health check on top of GRPC health check:

if( health_check_service == true) {
  enableGRPCHealthCheck();
  
  if( unframed_requests == true ) {
     enableHttpHealthCheck()
  }
}

This also aligns with use of unframed request option which is intended to be used to gradually migrate an existing HTTP POST based API to gRPC, so it helps to have both gRPC and HTTP health check.

dinujoh avatar Jun 28 '22 19:06 dinujoh

Yes, I think if you are enabling unframed requests, you are still supporting gRPC. So, we should have the health check on both HTTP and gRPC. This makes the most sense to me.

This is the logic to use:

if( health_check_service == true) {
  enableGRPCHealthCheck();
  
  if( unframed_requests == true ) {
     enableHttpHealthCheck()
  }
}

dlvenable avatar Jun 28 '22 20:06 dlvenable