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

Adding opentracing #3

Open
Apologiz opened this issue Jun 18, 2020 · 5 comments
Open

Adding opentracing #3

Apologiz opened this issue Jun 18, 2020 · 5 comments

Comments

@Apologiz
Copy link

I would like to contribute and add a request tracing - https://opentracing.io/

@Apologiz
Copy link
Author

I did it :)
#4

@christianhujer
Copy link
Member

I'd like to understand the benefits of doing that on a health-check API. Can you elaborate on why you need/want this feature, and how users benefit from it?

@Apologiz
Copy link
Author

This functionality allows you to monitor the status of services through Jaeger or Zipkin.
For example, if you do periodic health checks, you can track the fall of an external service and register errors in Jaeger.

// Implements ChecksProvider
type healthCheckServices struct {
  conn grpc.ClientConnInterface
}

// TODO: 'ctx context.Context' is needed because it contains a trace key.
func (hc *healthCheckServices) HealthChecks(ctx context.Context) map[string][]health.Checks {
  return map[string][]health.Checks{
    "services": {
      hc.serviceCheck(ctx),
    },
  }
}

// helper
func (hc *healthCheckServices) serviceCheck(ctx context.Context) health.Checks {
  checks := health.Checks{
    ComponentID:   "my_serivce",
    ComponentType: "grpc",
    Time:          time.Now().Format(config.TimeFormatISO8601),
  }
  client := grpc_health_v1.NewHealthClient(hc.conn)
  check, err := client.Check(ctx, &grpc_health_v1.HealthCheckRequest{})
  if err != nil {
    checks.Status = health.Fail
    checks.Output = fmt.Sprintf("Error: %s", err.Error())
    otlog.TraceError(ctx, zap.Error(err))
  }
  if check != nil {
    if check.GetStatus() == grpc_health_v1.HealthCheckResponse_SERVING {
      checks.Status = health.Pass
    } else {
      checks.Status = health.Fail
      checks.Output = fmt.Sprintf("Status: %s", check.GetStatus().String())
      otlog.TraceError(ctx, zap.Error(err))
    }
  }
  return checks
}

// TODO: I think it's worth putting methods in different interfaces
func (*healthCheckServices) AuthorizeHealth(r *http.Request) bool {
  return true
}

@christianhujer
Copy link
Member

christianhujer commented Jun 26, 2020

Isn't that repeated unnecessarily?
Either, the service should in regular scheduled intervals report its health automatically (push).
Or, the service should in regular intervals be asked for its health (pull).
Both at the same time doesn't make sense.

The upcoming health check RFC is pull by design. The reason is that changes in the health monitoring strategy, like the polling frequency, will not require a reconfiguration of the already deployed and running service.

@Apologiz
Copy link
Author

Tracing is needed not for polling services, but for tracing calls to dependent services

By example:

gateway
- healtz
  - external service

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants