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

Add metrics, health, fault tolerance, opentracing #113

Closed
wants to merge 9 commits into from
Closed

Add metrics, health, fault tolerance, opentracing #113

wants to merge 9 commits into from

Conversation

evie-lau
Copy link

@evie-lau evie-lau commented Oct 1, 2018

Reimplemented the existing health endpoint (/map/v1/health) using MP Health.

It seems the checks used don't actually reflect the state of the other services.

  • e.g. - kafka check remains the same after bringing down kafka

This change is Reviewable

Copy link
Member

@BarDweller BarDweller left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 3 files at r2.
Reviewable status: 1 of 6 files reviewed, 1 unresolved discussion (waiting on @kevinwlau)


map-wlpcfg/servers/gameon-map/server.xml, line 14 at r2 (raw file):

    </httpEndpoint>

    <quickStartSecurity userName="admin" userPassword="adminpwd"/>

Curious... what exactly is this protecting / why is this required?

Wondering if we shouldn't have this be read from an env var so we can have production/dev versions of the config, rather than having a common value checked into open github ?

Copy link
Member

@BarDweller BarDweller left a comment

Choose a reason for hiding this comment

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

Reviewed 2 of 5 files at r1, 1 of 3 files at r2.
Reviewable status: 4 of 6 files reviewed, 2 unresolved discussions (waiting on @kevinwlau)


map-app/src/main/java/org/gameontext/map/SitesResource.java, line 164 at r2 (raw file):

        tags = "label=createRoom")
    public Response createRoom(
            @ApiParam(value = "New room attributes", required = true) @QueryParam("room") RoomInfo newRoom) {

Why did this gain QueryParam?

Copy link
Author

@evie-lau evie-lau left a comment

Choose a reason for hiding this comment

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

Reviewable status: 4 of 6 files reviewed, 2 unresolved discussions (waiting on @kevinwlau)


map-app/src/main/java/org/gameontext/map/SitesResource.java, line 164 at r2 (raw file):

Previously, BarDweller (Ozzy Osborne) wrote…

Why did this gain QueryParam?

I had thought the generated API docs was missing parameters. On further inspection, nothing is passed through the URL, so I'll remove this.


map-wlpcfg/servers/gameon-map/server.xml, line 14 at r2 (raw file):

Previously, BarDweller (Ozzy Osborne) wrote…

Curious... what exactly is this protecting / why is this required?

Wondering if we shouldn't have this be read from an env var so we can have production/dev versions of the config, rather than having a common value checked into open github ?

MP Metrics endpoint is behind HTTPS, and this was a quick way to get setup. I'll look into other ways to configure it.

Copy link
Member

@BarDweller BarDweller left a comment

Choose a reason for hiding this comment

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

Reviewable status: 4 of 6 files reviewed, 2 unresolved discussions (waiting on @BarDweller)


map-wlpcfg/servers/gameon-map/server.xml, line 14 at r2 (raw file):

Previously, kevinwlau (Kevin Lau) wrote…

MP Metrics endpoint is behind HTTPS, and this was a quick way to get setup. I'll look into other ways to configure it.

Is it configurable to avoid HTTPS? (for example when we deploy to kube where all traffic from the pod is https handled by istio, and should be plain http inside the pod).

When using docker locally, our services are behind haproxy, which handles any https to the client.

@evie-lau
Copy link
Author


map-wlpcfg/servers/gameon-map/server.xml, line 14 at r2 (raw file):

Previously, BarDweller (Ozzy Osborne) wrote…

Is it configurable to avoid HTTPS? (for example when we deploy to kube where all traffic from the pod is https handled by istio, and should be plain http inside the pod).

When using docker locally, our services are behind haproxy, which handles any https to the client.

I don't think we can avoid HTTPS for this, MP Metrics requires SSL.

@evie-lau evie-lau changed the title Add metrics and health Add metrics, health, fault tolerance, opentracing Oct 24, 2018
@evie-lau
Copy link
Author

In order to use OpenTracing with Zipkin, we need to download and unzip the user feature opentracingZipkin and copy it to the docker container.

@ebullient
Copy link
Member

Rebased in #117 to drop tracing (pending jaeger)

@ebullient ebullient closed this Feb 25, 2019
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

Successfully merging this pull request may close these issues.

4 participants