Skip to content
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

Enable HTTP Health Check for OTelTraceSource and OTelMetricsSource #1546

Closed
dinujoh opened this issue Jun 28, 2022 · 4 comments
Closed

Enable HTTP Health Check for OTelTraceSource and OTelMetricsSource #1546

dinujoh opened this issue Jun 28, 2022 · 4 comments
Labels
enhancement New feature or request
Milestone

Comments

@dinujoh
Copy link
Member

dinujoh commented Jun 28, 2022

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.

@dlvenable
Copy link
Member

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?

@dinujoh
Copy link
Member Author

dinujoh commented Jun 28, 2022

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
Copy link
Member Author

dinujoh commented Jun 28, 2022

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.

@dlvenable
Copy link
Member

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 dlvenable added enhancement New feature or request and removed untriaged labels Jul 5, 2022
@dlvenable dlvenable added this to the v2.0 milestone Jul 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants