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

feat: add health check endpoint #3501

Merged
merged 23 commits into from
Oct 16, 2019
Merged
Changes from 1 commit
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
fd22534
feat: add healthcheck endpoint
vcrfxia Oct 7, 2019
6d10311
feat: switch to configexception
vcrfxia Oct 8, 2019
5abf820
feat: make healthcheck interval configurable
vcrfxia Oct 8, 2019
4cebe97
feat: update healthcheck response detail to be a class
vcrfxia Oct 8, 2019
34441bd
feat: healthcheck should not be authenticated
vcrfxia Oct 8, 2019
89297e6
Merge branch 'master' into healthcheck-endpoint
vcrfxia Oct 9, 2019
47818cf
refactor: split out KsqlTarget helper methods
vcrfxia Oct 9, 2019
d619f74
fix: use ServerInternalKsqlClient to get around auth issues
vcrfxia Oct 9, 2019
612f0c0
fix: make Check an interface
vcrfxia Oct 10, 2019
f795d34
fix: move cached response expiry into ResponseCache
vcrfxia Oct 10, 2019
5fa0002
docs: add java doc to ServerInternalKsqlClient
vcrfxia Oct 10, 2019
e8e0647
refactor: make health check two words
vcrfxia Oct 10, 2019
fc0bbba
docs: add docs for new endpoint
vcrfxia Oct 10, 2019
cae31e6
docs: jim's feedback
vcrfxia Oct 10, 2019
548ca3c
refactor: switch to Guava's cache implementation
vcrfxia Oct 14, 2019
3a754bd
fix: sergio's feedback
vcrfxia Oct 14, 2019
ef0797f
fix: checkstyle
vcrfxia Oct 14, 2019
2611061
fix: fix bug in getting entity from response. add integration test
vcrfxia Oct 15, 2019
6d2103b
test: vinoth's feedback
vcrfxia Oct 15, 2019
a678d9e
refactor: switch to LoadingCache
vcrfxia Oct 15, 2019
01cf409
style: checkstyle
vcrfxia Oct 15, 2019
8b176f3
fix: findbugs
vcrfxia Oct 15, 2019
0db13de
fix: make check names public so clients don't have to hardcode
vcrfxia Oct 16, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
fix: make Check an interface
  • Loading branch information
vcrfxia committed Oct 10, 2019
commit 612f0c017197500dc3557434b93fe36f95b45d40
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,8 @@
public class HealthcheckAgent {
stevenpyzhang marked this conversation as resolved.
Show resolved Hide resolved

private static final List<Check> DEFAULT_CHECKS = ImmutableList.of(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be a bit more abstract. You just need an interface like:

interface Check {
    String name();
    HealthcheckResponseDetail check();
}

The current Check class can just be an implementation of this (e.g. ExecuteStatementCheck). This way we can support more diverse extensions down the road.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done, though I updated the signature of the check() method to take a KSQL client and server endpoint, to avoid needing to pass those into each of the individual checks.

new Check("metastore", "list streams; list tables; list queries;"),
new Check("kafka", "list topics extended;")
new ExecuteStatementCheck("metastore", "list streams; list tables; list queries;"),
new ExecuteStatementCheck("kafka", "list topics extended;")
);

private final SimpleKsqlClient ksqlClient;
Expand All @@ -53,7 +53,7 @@ public HealthcheckResponse checkHealth() {
final Map<String, HealthcheckResponseDetail> results = DEFAULT_CHECKS.stream()
.collect(Collectors.toMap(
Check::getName,
check -> new HealthcheckResponseDetail(isSuccessful(check.getKsqlStatement()))
check -> check.check(ksqlClient, serverEndpoint)
));
final boolean allHealthy = results.values().stream()
.map(HealthcheckResponseDetail::getIsHealthy)
Expand All @@ -62,12 +62,6 @@ public HealthcheckResponse checkHealth() {
return new HealthcheckResponse(allHealthy, results);
}

private boolean isSuccessful(final String ksqlStatement) {
final RestResponse<KsqlEntityList> response =
ksqlClient.makeKsqlRequest(serverEndpoint, ksqlStatement);
return response.isSuccessful();
}

private static URI getServerAddress(final KsqlRestConfig restConfig) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't actually needed anymore since the ServerInternalKsqlClient used by the HealthcheckAgent ignores the serverEndpoint parameter in makeKsqlRequest() but I've left this code here in case we'd like to switch back to using a real KSQL client in the future. If preferable I can simply delete it for now instead.

final List<String> listeners = restConfig.getList(RestConfig.LISTENERS_CONFIG);
final String address = listeners.stream()
Expand All @@ -89,21 +83,34 @@ private static RuntimeException invalidAddressException(
return new ConfigException(RestConfig.LISTENERS_CONFIG, serverAddresses, message);
}

private static class Check {
private interface Check {
String getName();

HealthcheckResponseDetail check(SimpleKsqlClient ksqlClient, URI serverEndpoint);
}

private static class ExecuteStatementCheck implements Check {
private final String name;
private final String ksqlStatement;

Check(final String name, final String ksqlStatement) {
ExecuteStatementCheck(final String name, final String ksqlStatement) {
this.name = Objects.requireNonNull(name, "name");
this.ksqlStatement = Objects.requireNonNull(ksqlStatement, "ksqlStatement");
}

String getName() {
@Override
public String getName() {
return name;
}

String getKsqlStatement() {
return ksqlStatement;
@Override
public HealthcheckResponseDetail check(
final SimpleKsqlClient ksqlClient,
final URI serverEndpoint
) {
final RestResponse<KsqlEntityList> response =
ksqlClient.makeKsqlRequest(serverEndpoint, ksqlStatement);
return new HealthcheckResponseDetail(response.isSuccessful());
}
}
}