[MGNLREST-259] Missing content type response for .rest/status services health check Created: 08/Jul/20  Updated: 10/Jul/20  Resolved: 10/Jul/20

Status: Closed
Project: Magnolia REST Framework
Component/s: None
Affects Version/s: None
Fix Version/s: 2.2.1

Type: Bug Priority: Neutral
Reporter: Viet Nguyen Assignee: Maxime Michel
Resolution: Fixed Votes: 0
Labels: maintenance
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
causality
relation
Template:
Acceptance criteria:
Empty
Task DoD:
[ ]* Doc/release notes changes? Comment present?
[ ]* Downstream builds green?
[ ]* Solution information and context easily available?
[ ]* Tests
[ ]* FixVersion filled and not yet released
[ ]  Architecture Decision Record (ADR)
Bug DoR:
[ ]* Steps to reproduce, expected, and actual results filled
[ ]* Affected version filled
Date of First Response:
Sprint: Maintenance 15

 Description   

Customers using REST services health check seeing that the server log was flooded by multiple warnings which telling this service response have no content type set.
Please consider adding proper content type in the response of this health check endpoint.



 Comments   
Comment by Maxime Michel [ 08/Jul/20 ]

Sorry about that. The endpoint does produce a content-type: https://git.magnolia-cms.com/projects/MODULES/repos/rest/browse/magnolia-rest-services/src/main/java/info/magnolia/rest/service/status/StatusEndpoint.java#68

Is it maybe not picked up because the body is empty? Somebody would need to investigate it thorougly.

Comment by Maxime Michel [ 09/Jul/20 ]

First observation: returning any content ("hello world") in the response stops the ContentTypeFilter from issuing a warning.

Comment by Maxime Michel [ 09/Jul/20 ]

I see people suggest that text/plain rather than application/json should be used for empty responses, as an empty body is not valid JSON, but it's valid text. This however doesn't solve the issue.

Comment by Viet Nguyen [ 09/Jul/20 ]

We could also consider giving more informative response in case of OK, instead of empty string. Such as free heapsize or cpu%, just my 2 cents

Comment by Maxime Michel [ 09/Jul/20 ]

org.jboss.resteasy.core.ServerResponseWriter#getResponseMediaType returns null when no entity (content) is provided. So either we provide dummy content, or we add a ContentTypeFilter bypass.

Comment by Maxime Michel [ 09/Jul/20 ]

That's a good idea viet.nguyen. Let me discuss that with SREs and core developers, and see what we would like to do there.

Comment by Maxime Michel [ 09/Jul/20 ]

After discussions, I stumbled upon DEV-1545. The intent of that ticket is to provide customizable parameters that the health check can return. I'm told a health check's utility depends largely on the project. So I would wait for that ticket's research before adding CPU or RAM usage here.

Instead, to fix this ticket, I would either respond {}, or add a ContentTypeFilter bypass.

Comment by Espen Jervidalo [ 09/Jul/20 ]

Running `curl -vv http://localhost:8080/.rest/status` I get this in the logs:

2020-07-09 16:28:24,670 WARN  info.magnolia.cms.filters.ContentTypeFilter       : Response is not committed yet. Setting content type: text/html.
2020-07-09 16:28:46,682 WARN  info.magnolia.cms.filters.ContentTypeFilter       : Content type for http://localhost:8080/.rest/status is not set.

In ContentTypeFilter

log.warn("Content type for {} is not set.", originalUrl);
if (!response.isCommitted()) { //if the content type was not set (e.g. in renderer) and response is not committed, set content type so we don't produce a response without the content type
    log.warn("Response is not committed yet. Setting content type: {}.", mimeType);
    response.setContentType(mimeType);
}

I don't know to what extent it's necessary to set a fallback at all. But logging it on each and every request seems unnecessary. I don't know what best practice is either. But those warns and errors and infos per request spoil the logs.

Maybe it's the log aggregator's job to filter/consolidate that.. :shrug:

Another thing: When opening the endpoint in the browser it prompts a download dialog for an empty file.

Generated at Mon Feb 12 06:58:09 CET 2024 using Jira 9.4.2#940002-sha1:46d1a51de284217efdcb32434eab47a99af2938b.