Skip to content

Commit

Permalink
fix: Add server side logging for swagger handling code (#2328)
Browse files Browse the repository at this point in the history
* fix: add proper server side logging for swagger handling

Signed-off-by: Carson Cook <carson.cook@ibm.com>

* Misc grammar improvements

Signed-off-by: Carson Cook <carson.cook@ibm.com>

* Fix unit test

Signed-off-by: Carson Cook <carson.cook@ibm.com>

* Organize unit tests

Signed-off-by: Carson Cook <carson.cook@ibm.com>
  • Loading branch information
CarsonCook committed Apr 28, 2022
1 parent 502ba3c commit 7b0455d
Show file tree
Hide file tree
Showing 8 changed files with 496 additions and 433 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
*/
package org.zowe.apiml.apicatalog.services.cached;

import lombok.extern.slf4j.Slf4j;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.stereotype.Service;
import org.zowe.apiml.apicatalog.services.cached.model.ApiDocCacheKey;
Expand All @@ -30,6 +31,7 @@
*/

@Service
@Slf4j
public class CachedApiDocService {
private static final String DEFAULT_API_KEY = "default";

Expand Down Expand Up @@ -73,9 +75,12 @@ public String getApiDocForService(final String serviceId, final String apiVersio
}
} catch (Exception e) {
//if there's not apiDoc in cache
String logMessage = String.format("Exception updating API doc in cache for '%s %s'", serviceId, apiVersion);
if (apiDoc == null) {
log.error("No API doc available for '{} {}': {}", serviceId, apiVersion, logMessage, e);
throw new ApiDocNotFoundException(exceptionMessage.apply(serviceId));
}
log.debug(logMessage, e);
}
return apiDoc;
}
Expand Down Expand Up @@ -116,9 +121,12 @@ public String getDefaultApiDocForService(final String serviceId) {
}
} catch (Exception e) {
//if there's not apiDoc in cache
String logMessage = String.format("Exception updating default API doc in cache for '%s'.", serviceId);
if (apiDoc == null) {
log.error("No default API doc available for service '{}': {}", serviceId, logMessage, e);
throw new ApiDocNotFoundException(exceptionMessage.apply(serviceId));
}
log.debug(logMessage,e);
}
return apiDoc;
}
Expand Down Expand Up @@ -152,9 +160,12 @@ public List<String> getApiVersionsForService(final String serviceId) {
}
} catch (Exception e) {
// if no versions in cache
String logMessage = String.format("Exception updating API versions in cache for '%s'", serviceId);
if (apiVersions == null) {
log.error("No API versions available for service '{}': {}", serviceId, logMessage, e);
throw new ApiVersionNotFoundException("No API versions were retrieved for the service " + serviceId + ".");
}
log.debug(logMessage, e);
}
return apiVersions;
}
Expand Down Expand Up @@ -184,6 +195,7 @@ public String getDefaultApiVersionForService(final String serviceId) {
defaultVersion = version;
}
} catch (Exception e) {
log.error("No default API version available for service '{}'", serviceId, e);
throw new ApiVersionNotFoundException("Error trying to find default API version");
}
return defaultVersion;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import com.netflix.appinfo.InstanceInfo;
import lombok.NonNull;
import lombok.RequiredArgsConstructor;
import lombok.extern.slf4j.Slf4j;
import org.springframework.http.*;
import org.springframework.stereotype.Service;
import org.springframework.web.client.RestTemplate;
Expand Down Expand Up @@ -39,6 +40,7 @@
*/
@Service
@RequiredArgsConstructor
@Slf4j
public class APIDocRetrievalService {

private final RestTemplate restTemplate;
Expand All @@ -57,6 +59,7 @@ public class APIDocRetrievalService {
* @throws ApiVersionNotFoundException if the API versions cannot be loaded
*/
public List<String> retrieveApiVersions(@NonNull String serviceId) {
log.debug("Retrieving API versions for service '{}'", serviceId);
InstanceInfo instanceInfo;

try {
Expand All @@ -70,6 +73,8 @@ public List<String> retrieveApiVersions(@NonNull String serviceId) {
for (ApiInfo apiInfo : apiInfoList) {
apiVersions.add(apiInfo.getApiId() + " v" + apiInfo.getVersion());
}

log.debug("For service '{}' found API versions '{}'", serviceId, apiVersions);
return apiVersions;
}

Expand All @@ -83,6 +88,7 @@ public List<String> retrieveApiVersions(@NonNull String serviceId) {
* @return default API version in the format v{majorVersion}, or null.
*/
public String retrieveDefaultApiVersion(@NonNull String serviceId) {
log.debug("Retrieving default API version for service '{}'", serviceId);
InstanceInfo instanceInfo;

try {
Expand All @@ -98,7 +104,9 @@ public String retrieveDefaultApiVersion(@NonNull String serviceId) {
return "";
}

return defaultApiInfo.getApiId() + " v" + defaultApiInfo.getVersion();
String defaultVersion = defaultApiInfo.getApiId() + " v" + defaultApiInfo.getVersion();
log.debug("For service '{}' found default API version '{}'", serviceId, defaultVersion);
return defaultVersion;
}

/**
Expand All @@ -118,6 +126,7 @@ public String retrieveDefaultApiVersion(@NonNull String serviceId) {
* @throws ApiDocNotFoundException if the response is error
*/
public ApiDocInfo retrieveApiDoc(@NonNull String serviceId, String apiVersion) {
log.debug("Retrieving API doc for '{} {}'", serviceId, apiVersion);
InstanceInfo instanceInfo = getInstanceInfo(serviceId);

List<ApiInfo> apiInfoList = metadataParser.parseApiInfo(instanceInfo.getMetadata());
Expand All @@ -131,6 +140,7 @@ private ApiDocInfo buildApiDocInfo(String serviceId, ApiInfo apiInfo, InstanceIn
String apiDocUrl = getApiDocUrl(apiInfo, instanceInfo, routes);

if (apiDocUrl == null) {
log.warn("No api doc URL for '{} {} {}'", serviceId, apiInfo.getApiId(), apiInfo.getVersion());
return getApiDocInfoBySubstituteSwagger(instanceInfo, routes, apiInfo);
}

Expand All @@ -144,6 +154,7 @@ private ApiDocInfo buildApiDocInfo(String serviceId, ApiInfo apiInfo, InstanceIn
}
return new ApiDocInfo(apiInfo, apiDocContent, routes);
} catch (Exception e) {
log.error("Error retrieving api doc for '{}'", serviceId, e);
return getApiDocInfoBySubstituteSwagger(instanceInfo, routes, apiInfo);
}
}
Expand All @@ -165,6 +176,7 @@ private ApiDocInfo buildApiDocInfo(String serviceId, ApiInfo apiInfo, InstanceIn
* @throws ApiDocNotFoundException if the response is error
*/
public ApiDocInfo retrieveDefaultApiDoc(@NonNull String serviceId) {
log.debug("Retrieving default API doc for service '{}'", serviceId);
InstanceInfo instanceInfo = getInstanceInfo(serviceId);

List<ApiInfo> apiInfoList = metadataParser.parseApiInfo(instanceInfo.getMetadata());
Expand All @@ -177,6 +189,7 @@ private ApiInfo getDefaultApiInfo(List<ApiInfo> apiInfoList) {
ApiInfo defaultApiInfo = getApiInfoSetAsDefault(apiInfoList);

if (defaultApiInfo == null) {
log.debug("No API set as default, will use highest major version as default");
defaultApiInfo = getHighestApiVersion(apiInfoList);
}

Expand All @@ -188,7 +201,10 @@ private ApiInfo getApiInfoSetAsDefault(List<ApiInfo> apiInfoList) {
for (ApiInfo apiInfo : apiInfoList) {
if (apiInfo.isDefaultApi()) {
if (defaultApiInfo != null) {
// multiple APIs set as default, can't handle conflict so stop looking for set default
log.warn("Multiple API are set as default: '{} {}' and '{} {}'. Neither will be treated as the default.",
defaultApiInfo.getApiId(), apiInfo.getVersion(),
apiInfo.getApiId(), apiInfo.getVersion()
);
return null;
} else {
defaultApiInfo = apiInfo;
Expand Down Expand Up @@ -271,16 +287,21 @@ private String getApiDocContentByUrl(@NonNull String serviceId, String apiDocUrl
HttpHeaders headers = new HttpHeaders();
headers.setAccept(Collections.singletonList(MediaType.APPLICATION_JSON));

ResponseEntity<String> response = restTemplate.exchange(
apiDocUrl,
HttpMethod.GET,
new HttpEntity<>(headers),
String.class);

if (response.getStatusCode().isError()) {
throw new ApiDocNotFoundException("No API Documentation was retrieved due to " + serviceId + " server error: '" + response.getBody() + "'.");
try {
ResponseEntity<String> response = restTemplate.exchange(
apiDocUrl,
HttpMethod.GET,
new HttpEntity<>(headers),
String.class);

if (response.getStatusCode().isError()) {
throw new ApiDocNotFoundException("No API Documentation was retrieved due to " + serviceId + " server error: '" + response.getBody() + "'.");
}
return response.getBody();
} catch (Exception e) {
log.error("Error retrieving api doc by url for {} at {}", serviceId, apiDocUrl, e);
throw e;
}
return response.getBody();
}

/**
Expand Down Expand Up @@ -316,7 +337,7 @@ private ApiInfo findApi(List<ApiInfo> apiInfos, String apiVersion) {
}

if (apiVersion == null) {
return apiInfos.get(0);
return apiInfos.get(0);
}

String[] api = apiVersion.split(" ");
Expand All @@ -330,7 +351,9 @@ private ApiInfo findApi(List<ApiInfo> apiInfos, String apiVersion) {
.findFirst();

if (!result.isPresent()) {
throw new ApiDocNotFoundException("There is no api doc for combination of apiId and version. " + apiId + " " + version);
String errMessage = String.format("Error finding api doc: there is no api doc for '%s %s'.", apiId, version);
log.error(errMessage);
throw new ApiDocNotFoundException(errMessage);
} else {
return result.get();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -116,8 +116,8 @@ public ResponseEntity<String> getApiDiffInfo(@NonNull String serviceId, String a
result = result.replace("<link rel=\"stylesheet\" href=\"http://deepoove.com/swagger-diff/stylesheets/demo.css\">", "");
return new ResponseEntity<>(result, createHeaders(), HttpStatus.OK);
} catch (Exception e) {
String errorMessage = String.format("Error retrieving API diff for %s and versions %s and %s", serviceId, apiVersion1, apiVersion2);
log.debug("{}. Cause: {}", errorMessage, e.getMessage());
String errorMessage = String.format("Error retrieving API diff for '%s' with versions '%s' and '%s'", serviceId, apiVersion1, apiVersion2);
log.error(errorMessage, e);
throw new ApiDiffNotAvailableException(errorMessage);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
*/
package org.zowe.apiml.apicatalog.swagger;

import lombok.extern.slf4j.Slf4j;
import org.zowe.apiml.config.ApiInfo;
import com.netflix.appinfo.InstanceInfo;
import org.apache.velocity.Template;
Expand All @@ -22,6 +23,7 @@
import static org.zowe.apiml.constants.EurekaMetadataDefinition.SERVICE_DESCRIPTION;
import static org.zowe.apiml.constants.EurekaMetadataDefinition.SERVICE_TITLE;

@Slf4j
public class SubstituteSwaggerGenerator {
private final VelocityEngine ve = new VelocityEngine();

Expand All @@ -34,6 +36,7 @@ public SubstituteSwaggerGenerator() {
public String generateSubstituteSwaggerForService(InstanceInfo service,
ApiInfo api,
String gatewayScheme, String gatewayHost) {
log.warn("Generating substitute swagger for service instance '{}' API '{} {}'", service.getInstanceId(), api.getApiId(), api.getVersion());
String title = service.getMetadata().get(SERVICE_TITLE);
String description = service.getMetadata().get(SERVICE_DESCRIPTION);
String basePath = (api.getGatewayUrl().startsWith("/") ? "" : "/") + service.getAppName().toLowerCase()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ public String transformApiDoc(String serviceId, ApiDocInfo apiDocInfo) {
try {
return initializeObjectMapper().writeValueAsString(openAPI);
} catch (JsonProcessingException e) {
log.debug("Could not convert Swagger to JSON", e);
log.debug("Could not convert OpenAPI to JSON", e);
throw new ApiDocTransformationException("Could not convert Swagger to JSON");
}
}
Expand Down
Loading

0 comments on commit 7b0455d

Please sign in to comment.