devmadhuu commented on code in PR #7019: URL: https://github.com/apache/ozone/pull/7019#discussion_r1703646831
########## hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/ReconContextEndpoint.java: ########## @@ -0,0 +1,75 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with this + * work for additional information regarding copyright ownership. The ASF + * licenses this file to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * <p/> + * http://www.apache.org/licenses/LICENSE-2.0 + * <p/> + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations under + * the License. + */ + +package org.apache.hadoop.ozone.recon.api; + +import org.apache.hadoop.ozone.recon.ReconContext; + +import javax.inject.Inject; +import javax.ws.rs.GET; +import javax.ws.rs.Path; +import javax.ws.rs.Produces; +import javax.ws.rs.core.MediaType; +import javax.ws.rs.core.Response; + +/** + * REST API to expose ReconContext information for admin users. + */ +@Path("/reconcontext") Review Comment: ```suggestion @Path("/recon") ``` ########## hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/ReconContextEndpoint.java: ########## @@ -0,0 +1,75 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with this + * work for additional information regarding copyright ownership. The ASF + * licenses this file to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * <p/> + * http://www.apache.org/licenses/LICENSE-2.0 + * <p/> + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations under + * the License. + */ + +package org.apache.hadoop.ozone.recon.api; + +import org.apache.hadoop.ozone.recon.ReconContext; + +import javax.inject.Inject; +import javax.ws.rs.GET; +import javax.ws.rs.Path; +import javax.ws.rs.Produces; +import javax.ws.rs.core.MediaType; +import javax.ws.rs.core.Response; + +/** + * REST API to expose ReconContext information for admin users. + */ +@Path("/reconcontext") +@Produces(MediaType.APPLICATION_JSON) +public class ReconContextEndpoint { + + private final ReconContext reconContext; + + @Inject + public ReconContextEndpoint(ReconContext reconContext) { + this.reconContext = reconContext; + } + + /** + * API to get the overall health status of Recon. + * + * @return HTTP Response containing the health status and other info. + */ + @GET + @Path("/status") + public Response getReconContextStatus() { + return Response.ok(reconContext.toMap()).build(); + } + + /** + * API to get detailed error information from ReconContext. + * + * @return HTTP Response containing the list of errors recorded during startup. + */ + @GET + @Path("/errors") + public Response getReconErrors() { + return Response.ok(reconContext.getErrors()).build(); Review Comment: We should be flexible to include all errors in recon a part from what we added in Recon Context. E.g this will include HeatMap error when health not good. ########## hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/ReconContextEndpoint.java: ########## @@ -0,0 +1,75 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with this + * work for additional information regarding copyright ownership. The ASF + * licenses this file to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * <p/> + * http://www.apache.org/licenses/LICENSE-2.0 + * <p/> + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations under + * the License. + */ + +package org.apache.hadoop.ozone.recon.api; + +import org.apache.hadoop.ozone.recon.ReconContext; + +import javax.inject.Inject; +import javax.ws.rs.GET; +import javax.ws.rs.Path; +import javax.ws.rs.Produces; +import javax.ws.rs.core.MediaType; +import javax.ws.rs.core.Response; + +/** + * REST API to expose ReconContext information for admin users. + */ +@Path("/reconcontext") +@Produces(MediaType.APPLICATION_JSON) +public class ReconContextEndpoint { + + private final ReconContext reconContext; + + @Inject + public ReconContextEndpoint(ReconContext reconContext) { + this.reconContext = reconContext; + } + + /** + * API to get the overall health status of Recon. + * + * @return HTTP Response containing the health status and other info. + */ + @GET + @Path("/status") + public Response getReconContextStatus() { + return Response.ok(reconContext.toMap()).build(); + } + + /** + * API to get detailed error information from ReconContext. + * + * @return HTTP Response containing the list of errors recorded during startup. + */ + @GET + @Path("/errors") Review Comment: We need not to have 3 API end points, IMO, only `/health` can be separate and we can have combined one for errors and overall status info. If errors are there, then also we might need overall `ReconContext` info and if no errors, then also we might need other information from `ReconContext`. Basically the plan is to utilize the context object for all types of information, could it be INFO, WARNING, ERROR etc. ########## hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/ReconContextEndpoint.java: ########## @@ -0,0 +1,75 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with this + * work for additional information regarding copyright ownership. The ASF + * licenses this file to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * <p/> + * http://www.apache.org/licenses/LICENSE-2.0 + * <p/> + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations under + * the License. + */ + +package org.apache.hadoop.ozone.recon.api; + +import org.apache.hadoop.ozone.recon.ReconContext; + +import javax.inject.Inject; +import javax.ws.rs.GET; +import javax.ws.rs.Path; +import javax.ws.rs.Produces; +import javax.ws.rs.core.MediaType; +import javax.ws.rs.core.Response; + +/** + * REST API to expose ReconContext information for admin users. + */ +@Path("/reconcontext") +@Produces(MediaType.APPLICATION_JSON) +public class ReconContextEndpoint { + + private final ReconContext reconContext; + + @Inject + public ReconContextEndpoint(ReconContext reconContext) { + this.reconContext = reconContext; + } + + /** + * API to get the overall health status of Recon. + * + * @return HTTP Response containing the health status and other info. + */ + @GET + @Path("/status") + public Response getReconContextStatus() { + return Response.ok(reconContext.toMap()).build(); + } + + /** + * API to get detailed error information from ReconContext. + * + * @return HTTP Response containing the list of errors recorded during startup. + */ + @GET + @Path("/errors") + public Response getReconErrors() { + return Response.ok(reconContext.getErrors()).build(); + } + + /** + * API to get the health status of Recon as a simple boolean. + * + * @return HTTP Response containing the health status. + */ + @GET + @Path("/health") + public Response isReconHealthy() { + return Response.ok(reconContext.isHealthy().get()).build(); Review Comment: This cannot be just directly interpreted by `ReconContext` health info being stored. It should combine overall Recon health like if we have any other alerts and notifications which we are raising in alerts/notifications bar.So Recon health can fall into `Green`, `Amber`, `Red` based on other alerts/notifications for other modules in future. So as of now we have HeatMap health as well which dictates the overall health of Recon. I would suggest to have a doc where we can discuss, how this info will be useful to admin users, irrespective of whether we are going to display over UI or not. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
