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

Fix zero's health endpoint to return json response #8858

Merged
merged 10 commits into from
Jun 28, 2023

Conversation

jbhamra1
Copy link
Contributor

@jbhamra1 jbhamra1 commented Jun 7, 2023

Fixes #5517 Zero's /health endpoint just returns OK unlike Alpha's JSON output.

Problem

curl http://alpha-hostname:alpha-http-port/health returns a JSON output whereas, curl http://zero-hostname:zero-http-port/health returns a plain text "OK".

Solution

Added code to return a plain text output ("OK") for the "text/plain" to maintain the backward compatibility and return JSON output for "Accept: application/json" header.

Example Output:
$ curl http://localhost:58696/health
OK

$ curl -H "Accept: application/json" http://localhost:58696/health
{"instance":"zero","address":"zero1:5080","status":"healthy","version":"v23.0.0-16-g90139243f","uptime":9223372036,"lastEcho":1686137664}

@dgraph-bot dgraph-bot added the go Pull requests that update Go code label Jun 7, 2023
Copy link
Contributor

@mangalaman93 mangalaman93 left a comment

Choose a reason for hiding this comment

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

LGTM a few minor comments.

dgraph/cmd/zero/http.go Outdated Show resolved Hide resolved
dgraph/cmd/zero/http.go Outdated Show resolved Hide resolved
@mangalaman93 mangalaman93 changed the title ISSUE #5517: Zero's /health endpoint just returns OK unlike Alpha's J… Fix zero's health endpoint to return json response Jun 7, 2023
@mangalaman93 mangalaman93 marked this pull request as ready for review June 7, 2023 13:17
mangalaman93
mangalaman93 previously approved these changes Jun 7, 2023
Copy link
Contributor

@harshil-goel harshil-goel left a comment

Choose a reason for hiding this comment

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

Can you write some tests for it? Otherwise looks great.

Copy link
Contributor

@mangalaman93 mangalaman93 left a comment

Choose a reason for hiding this comment

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

LGTM, a few minor comments. Please print the error message in a separate Go code and make sure they look okay in the logs and make sense.

dgraph/cmd/zero/http.go Outdated Show resolved Hide resolved
dgraph/cmd/zero/http.go Show resolved Hide resolved
dgraph/cmd/zero/http.go Outdated Show resolved Hide resolved
dgraph/cmd/zero/zero_test.go Outdated Show resolved Hide resolved
dgraph/cmd/zero/http.go Outdated Show resolved Hide resolved
dgraph/cmd/zero/http.go Outdated Show resolved Hide resolved
dgraph/cmd/zero/http.go Show resolved Hide resolved
dgraph/cmd/zero/zero_test.go Outdated Show resolved Hide resolved
mangalaman93
mangalaman93 previously approved these changes Jun 23, 2023
dgraph/cmd/zero/zero_test.go Outdated Show resolved Hide resolved
dgraph/cmd/zero/zero_test.go Outdated Show resolved Hide resolved
dgraph/cmd/zero/zero_test.go Outdated Show resolved Hide resolved
go.mod Outdated Show resolved Hide resolved
dgraph/cmd/zero/zero_test.go Outdated Show resolved Hide resolved
dgraph/cmd/zero/zero_test.go Outdated Show resolved Hide resolved
@jbhamra1 jbhamra1 force-pushed the jassi/zero_health_issue_5517 branch from aebb829 to ee76626 Compare June 28, 2023 08:43
@mangalaman93 mangalaman93 merged commit 973efa3 into main Jun 28, 2023
9 of 10 checks passed
@mangalaman93 mangalaman93 deleted the jassi/zero_health_issue_5517 branch June 28, 2023 10:45
dgraph-bot pushed a commit that referenced this pull request Jul 7, 2023
Fixes #5517 Zero's /health endpoint just returns OK unlike Alpha's JSON
output.

## Problem
curl http://alpha-hostname:alpha-http-port/health returns a JSON output
whereas, curl http://zero-hostname:zero-http-port/health returns a plain
text "OK".

## Solution
Added code to return a plain text output ("OK") for the "text/plain" to
maintain the backward compatibility and return JSON output for "Accept:
application/json" header.

Example Output:
$ curl http://localhost:58696/health
OK

$ curl -H "Accept: application/json" http://localhost:58696/health

{"instance":"zero","address":"zero1:5080","status":"healthy","version":"v23.0.0-16-g90139243f","uptime":9223372036,"lastEcho":1686137664}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
go Pull requests that update Go code
Development

Successfully merging this pull request may close these issues.

Zero's /health endpoint returns just "Ok" where the same Alpha's endpoint returns a JSON
4 participants